168329 2017-02-14 13:08:49 -0800 webkitpy: Memoize app_identifier_from_bundle for efficiency 2017-02-15 09:59:54 -0800 1 1 1 Unclassified WebKit Tools / Tests WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 jbedard jbedard ap commit-queue dbates glenn lforschler webkit-bug-importer oldest_to_newest 1276884 0 jbedard 2017-02-14 13:08:49 -0800 The DarwinPort class in webkitpy calls PlistBuddy to extract the app identifier from the bundle of an app. This function should be memoized to prevent unneeded calls to PlistBuddy. 1276885 1 webkit-bug-importer 2017-02-14 13:10:03 -0800 <rdar://problem/30518832> 1276894 2 301538 jbedard 2017-02-14 13:22:27 -0800 Created attachment 301538 Patch 1277085 3 301538 dbates 2017-02-14 23:22:07 -0800 Comment on attachment 301538 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=301538&action=review > Tools/ChangeLog:7 > + Please explain why we are making these changes. I assume we are making the memorization change because we instantiate SimulatorProcess frequently and SimulatorProcess.__init__() computes the app bundle identifier. I am assuming DarwinPort._get_crash_log() was just wrong? Regardless, please clarify. The purpose of the ChangeLog is to explain why a change was made. The only time it is acceptable to omit a description is when the title sufficiently explains the reason for a change. The title of this bug is insuffient as it only explains what we are doing and not why. On another note, we generally prefer one bug fix per patch. Among other reasons this limits the risk of causing a regression that could result in a full rollout of the patch. When the patch fixes multiple bugs a rollout in response to a regressiion due to one of the bug fixes results in the undoing of the unrelated bug fixes (since they are in the same patch and assuming the person that performs rollout does a full revert). Using the one bug fix per patch approach at most one of the bug fixes would be reverted. This patch fixes three bugs: 1. Memorize a function 2. Fix _get_crash_log() and 3. Remove an unnecessary import of a Python module. 1277086 4 301538 dbates 2017-02-14 23:23:02 -0800 Comment on attachment 301538 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=301538&action=review > Tools/ChangeLog:4 > + https://bugs.webkit.org/show_bug.cgi?id=168329 Please add the Radar bug URL under this line before landing. 1277191 5 jbedard 2017-02-15 08:38:13 -0800 I will split this into the three requested bugs. This bug will continue to track the memoization change, I will include better descriptions of the rationale of the changes in the respective change logs. 1277201 6 301622 jbedard 2017-02-15 08:53:06 -0800 Created attachment 301622 Patch 1277207 7 jbedard 2017-02-15 09:04:05 -0800 Remove time import: https://bugs.webkit.org/show_bug.cgi?id=168371 Fix DarwinPort._get_crash_log(): https://bugs.webkit.org/show_bug.cgi?id=168372 1277238 8 301622 commit-queue 2017-02-15 09:59:45 -0800 Comment on attachment 301622 Patch Clearing flags on attachment: 301622 Committed r212372: <http://trac.webkit.org/changeset/212372> 1277239 9 commit-queue 2017-02-15 09:59:54 -0800 All reviewed patches have been landed. Closing bug. 301538 2017-02-14 13:22:27 -0800 2017-02-15 08:53:03 -0800 Patch bug-168329-20170214131953.patch text/plain 2194 jbedard SW5kZXg6IFRvb2xzL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBUb29scy9DaGFuZ2VMb2cJKHJl dmlzaW9uIDIxMjMxNikKKysrIFRvb2xzL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwz ICsxLDE2IEBACisyMDE3LTAyLTE0ICBKb25hdGhhbiBCZWRhcmQgIDxqYmVkYXJkQGFwcGxlLmNv bT4KKworICAgICAgICB3ZWJraXRweTogTWVtb2l6ZSBhcHBfaWRlbnRpZmllcl9mcm9tX2J1bmRs ZSBmb3IgZWZmaWNpZW5jeSwgY2FsbCBwYXJlbnQgY2xhc3MgZm9yIF9nZXRfY3Jhc2hfbG9nCisg ICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNjgzMjkKKwor ICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIFNjcmlwdHMv d2Via2l0cHkvcG9ydC9kYXJ3aW4ucHk6IFJlbW92ZSB0aW1lIGltcG9ydCwgYWRkIG1lbW9yaXpl ZCBpbXBvcnQuCisgICAgICAgIChEYXJ3aW5Qb3J0KToKKyAgICAgICAgKERhcndpblBvcnQuX2dl dF9jcmFzaF9sb2cpOiBDYWxsIHBhcmVudCBjbGFzcyBpbXBsZW1lbnRhdGlvbi4KKyAgICAgICAg KERhcndpblBvcnQuYXBwX2lkZW50aWZpZXJfZnJvbV9idW5kbGUpOiBNZW1vaXplIHRvIGF2b2lk IGV4dHJhIGV4ZWN1dGlvbnMgb2YgUGxpc3RCdWRkeS4KKyAgICAgICAgCisKIDIwMTctMDItMTQg IEpvbmF0aGFuIEJlZGFyZCAgPGpiZWRhcmRAYXBwbGUuY29tPgogCiAgICAgICAgIFVucmV2aWV3 ZWQgYnVpbGQtZml4IGFmdGVyIHIyMTIyOTcuCkluZGV4OiBUb29scy9TY3JpcHRzL3dlYmtpdHB5 L3BvcnQvZGFyd2luLnB5Cj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFRvb2xzL1NjcmlwdHMvd2Via2l0cHkvcG9y dC9kYXJ3aW4ucHkJKHJldmlzaW9uIDIxMjMwNSkKKysrIFRvb2xzL1NjcmlwdHMvd2Via2l0cHkv cG9ydC9kYXJ3aW4ucHkJKHdvcmtpbmcgY29weSkKQEAgLTIyLDggKzIyLDggQEAKIAogaW1wb3J0 IGxvZ2dpbmcKIGltcG9ydCBvcwotaW1wb3J0IHRpbWUKIAorZnJvbSB3ZWJraXRweS5jb21tb24u bWVtb2l6ZWQgaW1wb3J0IG1lbW9pemVkCiBmcm9tIHdlYmtpdHB5LmNvbW1vbi5zeXN0ZW0uY3Jh c2hsb2dzIGltcG9ydCBDcmFzaExvZ3MKIGZyb20gd2Via2l0cHkuY29tbW9uLnN5c3RlbS5leGVj dXRpdmUgaW1wb3J0IFNjcmlwdEVycm9yCiBmcm9tIHdlYmtpdHB5LnBvcnQuYXBwbGUgaW1wb3J0 IEFwcGxlUG9ydApAQCAtMTAxLDcgKzEwMSw3IEBAIGNsYXNzIERhcndpblBvcnQoQXBwbGVQb3J0 KToKICAgICAgICAgcmV0dXJuIGNyYXNoX2xvZy5maW5kX2FsbF9sb2dzKGluY2x1ZGVfZXJyb3Jz PVRydWUsIG5ld2VyX3RoYW49bmV3ZXJfdGhhbikKIAogICAgIGRlZiBfZ2V0X2NyYXNoX2xvZyhz ZWxmLCBuYW1lLCBwaWQsIHN0ZG91dCwgc3RkZXJyLCBuZXdlcl90aGFuLCB0aW1lX2ZuPU5vbmUs IHNsZWVwX2ZuPU5vbmUsIHdhaXRfZm9yX2xvZz1UcnVlKToKLSAgICAgICAgcmV0dXJuIE5vbmUK KyAgICAgICAgcmV0dXJuIHN1cGVyKERhcndpblBvcnQsIHNlbGYpLl9nZXRfY3Jhc2hfbG9nKG5h bWUsIHBpZCwgc3Rkb3V0LCBzdGRlcnIsIG5ld2VyX3RoYW4pCiAKICAgICBkZWYgbG9va19mb3Jf bmV3X2NyYXNoX2xvZ3Moc2VsZiwgY3Jhc2hlZF9wcm9jZXNzZXMsIHN0YXJ0X3RpbWUpOgogICAg ICAgICAiIiJTaW5jZSBjcmFzaCBsb2dzIGNhbiB0YWtlIGEgbG9uZyB0aW1lIHRvIGJlIHdyaXR0 ZW4gb3V0IGlmIHRoZSBzeXN0ZW0gaXMKQEAgLTE3NSw2ICsxNzUsNyBAQCBjbGFzcyBEYXJ3aW5Q b3J0KEFwcGxlUG9ydCk6CiAgICAgICAgICAgICBfbG9nLndhcm4oInhjcnVuIGZhaWxlZDsgZmFs bGluZyBiYWNrIHRvICclcycuIiAlIGZhbGxiYWNrKQogICAgICAgICAgICAgcmV0dXJuIGZhbGxi YWNrCiAKKyAgICBAbWVtb2l6ZWQKICAgICBkZWYgYXBwX2lkZW50aWZpZXJfZnJvbV9idW5kbGUo c2VsZiwgYXBwX2J1bmRsZSk6CiAgICAgICAgIHBsaXN0X3BhdGggPSBzZWxmLl9maWxlc3lzdGVt LmpvaW4oYXBwX2J1bmRsZSwgJ0luZm8ucGxpc3QnKQogICAgICAgICBpZiBub3Qgc2VsZi5fZmls ZXN5c3RlbS5leGlzdHMocGxpc3RfcGF0aCk6Cg== 301622 2017-02-15 08:53:06 -0800 2017-02-15 09:59:45 -0800 Patch bug-168329-20170215085031.patch text/plain 1879 jbedard SW5kZXg6IFRvb2xzL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBUb29scy9DaGFuZ2VMb2cJKHJl dmlzaW9uIDIxMjM2NCkKKysrIFRvb2xzL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwz ICsxLDE5IEBACisyMDE3LTAyLTE1ICBKb25hdGhhbiBCZWRhcmQgIDxqYmVkYXJkQGFwcGxlLmNv bT4KKworICAgICAgICB3ZWJraXRweTogTWVtb2l6ZSBhcHBfaWRlbnRpZmllcl9mcm9tX2J1bmRs ZSBmb3IgZWZmaWNpZW5jeSwgY2FsbCBwYXJlbnQgY2xhc3MgZm9yIF9nZXRfY3Jhc2hfbG9nCisg ICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNjgzMjkKKyAg ICAgICAgPHJkYXI6Ly9wcm9ibGVtLzMwNTE4ODMyPgorCisgICAgICAgIFJldmlld2VkIGJ5IERh bmllbCBCYXRlcy4KKworICAgICAgICBXaGVuIHRlc3Rpbmcgb24gZGV2aWNlLCBhcHBfaWRlbnRp Zmllcl9mcm9tX2J1bmRsZSBpcyByZXBlYXRlZGx5IGNhbGxlZCBidXQgdGhlIHJldHVybiB2YWx1 ZSB3aWxsCisgICAgICAgIG5ldmVyIGNoYW5nZSBnaXZlbiB0aGUgc2FtZSBpbnB1dCBhcmd1bWVu dHMuICBNZW1vaXplIGZ1bmN0aW9uIGZvciBlZmZpY2llbmN5LgorCisgICAgICAgICogU2NyaXB0 cy93ZWJraXRweS9wb3J0L2Rhcndpbi5weTogQWRkIG1lbW9pemVkIGltcG9ydC4KKyAgICAgICAg KERhcndpblBvcnQpOgorICAgICAgICAoRGFyd2luUG9ydC5hcHBfaWRlbnRpZmllcl9mcm9tX2J1 bmRsZSk6IE1lbW9pemUgdG8gYXZvaWQgZXh0cmEgZXhlY3V0aW9ucyBvZiBQbGlzdEJ1ZGR5Lgor ICAgICAgICAKKwogMjAxNy0wMi0xNSAgQ2FybG9zIEdhcmNpYSBDYW1wb3MgIDxjZ2FyY2lhQGln YWxpYS5jb20+CiAKICAgICAgICAgW1NPVVBdIENyZWRlbnRpYWxzIHN0b3JlZCBieSBsaWJzb3Vw IGFyZSB1c2VkIGV2ZW4gU3RvcmVkQ3JlZGVudGlhbHMgcG9saWN5IGlzIERvTm90QWxsb3dTdG9y ZWRDcmVkZW50aWFscwpJbmRleDogVG9vbHMvU2NyaXB0cy93ZWJraXRweS9wb3J0L2Rhcndpbi5w eQo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09Ci0tLSBUb29scy9TY3JpcHRzL3dlYmtpdHB5L3BvcnQvZGFyd2luLnB5CShy ZXZpc2lvbiAyMTIzNjQpCisrKyBUb29scy9TY3JpcHRzL3dlYmtpdHB5L3BvcnQvZGFyd2luLnB5 CSh3b3JraW5nIGNvcHkpCkBAIC0yNCw2ICsyNCw3IEBAIGltcG9ydCBsb2dnaW5nCiBpbXBvcnQg b3MKIGltcG9ydCB0aW1lCiAKK2Zyb20gd2Via2l0cHkuY29tbW9uLm1lbW9pemVkIGltcG9ydCBt ZW1vaXplZAogZnJvbSB3ZWJraXRweS5jb21tb24uc3lzdGVtLmNyYXNobG9ncyBpbXBvcnQgQ3Jh c2hMb2dzCiBmcm9tIHdlYmtpdHB5LmNvbW1vbi5zeXN0ZW0uZXhlY3V0aXZlIGltcG9ydCBTY3Jp cHRFcnJvcgogZnJvbSB3ZWJraXRweS5wb3J0LmFwcGxlIGltcG9ydCBBcHBsZVBvcnQKQEAgLTE3 NSw2ICsxNzYsNyBAQCBjbGFzcyBEYXJ3aW5Qb3J0KEFwcGxlUG9ydCk6CiAgICAgICAgICAgICBf bG9nLndhcm4oInhjcnVuIGZhaWxlZDsgZmFsbGluZyBiYWNrIHRvICclcycuIiAlIGZhbGxiYWNr KQogICAgICAgICAgICAgcmV0dXJuIGZhbGxiYWNrCiAKKyAgICBAbWVtb2l6ZWQKICAgICBkZWYg YXBwX2lkZW50aWZpZXJfZnJvbV9idW5kbGUoc2VsZiwgYXBwX2J1bmRsZSk6CiAgICAgICAgIHBs aXN0X3BhdGggPSBzZWxmLl9maWxlc3lzdGVtLmpvaW4oYXBwX2J1bmRsZSwgJ0luZm8ucGxpc3Qn KQogICAgICAgICBpZiBub3Qgc2VsZi5fZmlsZXN5c3RlbS5leGlzdHMocGxpc3RfcGF0aCk6Cg==