139568 2014-12-11 16:46:01 -0800 WK1: Need new SPI for Webkit clients to override default immediate action 2014-12-11 18:16:01 -0800 1 1 1 Unclassified WebKit WebKit API 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 bdakin webkit-unassigned andersca bdakin commit-queue eric.carlson glenn jer.noble mitz philipj sergio thorton oldest_to_newest 1054687 0 bdakin 2014-12-11 16:46:01 -0800 WK1: Need new SPI for Webkit clients to override default immediate action rdar://problem/19225448 1054707 1 243166 bdakin 2014-12-11 17:22:42 -0800 Created attachment 243166 Patch 1054708 2 commit-queue 2014-12-11 17:25:23 -0800 Attachment 243166 did not pass style-queue: ERROR: Source/WebKit/mac/WebView/WebImmediateActionController.mm:146: The parameter name "]" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style. 1054710 3 243166 mitz 2014-12-11 17:32:17 -0800 Comment on attachment 243166 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=243166&action=review > Source/WebKit/mac/WebView/WebImmediateActionController.mm:140 > + defaultAnimationController = (id<NSImmediateActionAnimationController>)qlPreviewLinkItem.get(); Missing space between id and < > Source/WebKit/mac/WebView/WebImmediateActionController.mm:145 > + WebElementDictionary *webHitTestResult = [[[WebElementDictionary alloc] initWithHitTestResult:_hitTestResult] autorelease]; We should only make this object if we need it (i.e. if the delegate responds to the selector). 1054711 4 243166 mitz 2014-12-11 17:33:28 -0800 Comment on attachment 243166 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=243166&action=review >> Source/WebKit/mac/WebView/WebImmediateActionController.mm:145 >> + WebElementDictionary *webHitTestResult = [[[WebElementDictionary alloc] initWithHitTestResult:_hitTestResult] autorelease]; > > We should only make this object if we need it (i.e. if the delegate responds to the selector). Oh, and instead of autorelease we should RetainPtr it! 1054716 5 bdakin 2014-12-11 17:48:47 -0800 Thank you!! I addressed all of the comments. http://trac.webkit.org/changeset/177199 1054721 6 243169 mitz 2014-12-11 17:59:13 -0800 Created attachment 243169 Follow-up: fix a leak 1054723 7 mitz 2014-12-11 18:00:32 -0800 (In reply to comment #5) > Thank you!! I addressed all of the comments. > > http://trac.webkit.org/changeset/177199 My RetainPtr advice led to a leak. Please review the fix in attachment 243169. 1054726 8 243169 mitz 2014-12-11 18:14:57 -0800 Comment on attachment 243169 Follow-up: fix a leak Anders reviewed this. 1054727 9 mitz 2014-12-11 18:16:01 -0800 Fixed the leak in http://trac.webkit.org/changeset/177201 243166 2014-12-11 17:22:42 -0800 2014-12-11 17:32:17 -0800 Patch for-review.txt text/plain 4587 bdakin SW5kZXg6IFNvdXJjZS9XZWJLaXQvbWFjL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2Uv V2ViS2l0L21hYy9DaGFuZ2VMb2cJKHJldmlzaW9uIDE3NzE5NikKKysrIFNvdXJjZS9XZWJLaXQv bWFjL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE5IEBACisyMDE0LTEyLTEx ICBCZXRoIERha2luICA8YmRha2luQGFwcGxlLmNvbT4KKworICAgICAgICBXSzE6IE5lZWQgbmV3 IFNQSSBmb3IgV2Via2l0IGNsaWVudHMgdG8gb3ZlcnJpZGUgZGVmYXVsdCBpbW1lZGlhdGUgYWN0 aW9uCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xMzk1 NjgKKyAgICAgICAgLWFuZCBjb3JyZXNwb25kaW5nLQorICAgICAgICByZGFyOi8vcHJvYmxlbS8x OTIyNTQ0OAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg IENhbGwgU1BJIGdpdmluZyBjbGllbnRzIGEgY2hhbmNlIHRvIG92ZXJyaWRlIHRoZSBkZWZhdWx0 IGJlaGF2aW9yLgorICAgICAgICAqIFdlYlZpZXcvV2ViSW1tZWRpYXRlQWN0aW9uQ29udHJvbGxl ci5tbToKKyAgICAgICAgKC1bV2ViSW1tZWRpYXRlQWN0aW9uQ29udHJvbGxlciBfdXBkYXRlSW1t ZWRpYXRlQWN0aW9uSXRlbV0pOgorCisgICAgICAgIE5ldyBTUEkuCisgICAgICAgICogV2ViVmll dy9XZWJVSURlbGVnYXRlUHJpdmF0ZS5oOgorCiAyMDE0LTEyLTExICBUaW1vdGh5IEhvcnRvbiAg PHRpbW90aHlfaG9ydG9uQGFwcGxlLmNvbT4KIAogICAgICAgICBNb3ZlIHRoZSBwcmV2aWV3IHBv cG92ZXIgdG8gV2ViSW1tZWRpYXRlQWN0aW9uQ29udHJvbGxlcgpJbmRleDogU291cmNlL1dlYktp dC9tYWMvV2ViVmlldy9XZWJJbW1lZGlhdGVBY3Rpb25Db250cm9sbGVyLm1tCj09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0K LS0tIFNvdXJjZS9XZWJLaXQvbWFjL1dlYlZpZXcvV2ViSW1tZWRpYXRlQWN0aW9uQ29udHJvbGxl ci5tbQkocmV2aXNpb24gMTc3MTkyKQorKysgU291cmNlL1dlYktpdC9tYWMvV2ViVmlldy9XZWJJ bW1lZGlhdGVBY3Rpb25Db250cm9sbGVyLm1tCSh3b3JraW5nIGNvcHkpCkBAIC0yNyw2ICsyNyw3 IEBACiAKICNpZiBQTEFURk9STShNQUMpICYmIF9fTUFDX09TX1hfVkVSU0lPTl9NSU5fUkVRVUlS RUQgPj0gMTAxMDAwCiAKKyNpbXBvcnQgIldlYkVsZW1lbnREaWN0aW9uYXJ5LmgiCiAjaW1wb3J0 ICJXZWJGcmFtZUludGVybmFsLmgiCiAjaW1wb3J0ICJXZWJIVE1MVmlldy5oIgogI2ltcG9ydCAi V2ViSFRNTFZpZXdJbnRlcm5hbC5oIgpAQCAtMTI2LDYgKzEyNyw3IEBAIC0gKHZvaWQpX3VwZGF0 ZUltbWVkaWF0ZUFjdGlvbkl0ZW0KIHsKICAgICBfdHlwZSA9IFdlYkltbWVkaWF0ZUFjdGlvbk5v bmU7CiAgICAgX2ltbWVkaWF0ZUFjdGlvblJlY29nbml6ZXIuYW5pbWF0aW9uQ29udHJvbGxlciA9 IG5pbDsKKyAgICBpZCA8TlNJbW1lZGlhdGVBY3Rpb25BbmltYXRpb25Db250cm9sbGVyPiBkZWZh dWx0QW5pbWF0aW9uQ29udHJvbGxlciA9IG5pbDsKIAogICAgIE5TVVJMICp1cmwgPSBfaGl0VGVz dFJlc3VsdC5hYnNvbHV0ZUxpbmtVUkwoKTsKICAgICBOU1N0cmluZyAqYWJzb2x1dGVVUkxTdHJp bmcgPSBbdXJsIGFic29sdXRlU3RyaW5nXTsKQEAgLTEzNSw5ICsxMzcsMjEgQEAgLSAodm9pZClf dXBkYXRlSW1tZWRpYXRlQWN0aW9uSXRlbQogICAgICAgICBSZXRhaW5QdHI8UUxQcmV2aWV3TWVu dUl0ZW0+IHFsUHJldmlld0xpbmtJdGVtID0gW05TTWVudUl0ZW0gc3RhbmRhcmRRdWlja0xvb2tN ZW51SXRlbV07CiAgICAgICAgIFtxbFByZXZpZXdMaW5rSXRlbSBzZXRQcmV2aWV3U3R5bGU6UUxQ cmV2aWV3U3R5bGVQb3BvdmVyXTsKICAgICAgICAgW3FsUHJldmlld0xpbmtJdGVtIHNldERlbGVn YXRlOnNlbGZdOwotICAgICAgICBfaW1tZWRpYXRlQWN0aW9uUmVjb2duaXplci5hbmltYXRpb25D b250cm9sbGVyID0gKGlkPE5TSW1tZWRpYXRlQWN0aW9uQW5pbWF0aW9uQ29udHJvbGxlcj4pcWxQ cmV2aWV3TGlua0l0ZW0uZ2V0KCk7Ci0gICAgICAgIHJldHVybjsKKyAgICAgICAgZGVmYXVsdEFu aW1hdGlvbkNvbnRyb2xsZXIgPSAoaWQ8TlNJbW1lZGlhdGVBY3Rpb25BbmltYXRpb25Db250cm9s bGVyPilxbFByZXZpZXdMaW5rSXRlbS5nZXQoKTsKICAgICB9CisKKyAgICAvLyBBbGxvdyBjbGll bnRzIHRoZSBvcHBvcnR1bml0eSB0byBvdmVycmlkZSB0aGUgZGVmYXVsdCBpbW1lZGlhdGUgYWN0 aW9uLgorICAgIGlkIGN1c3RvbUNsaWVudEFuaW1hdGlvbkNvbnRyb2xsZXIgPSBuaWw7CisgICAg V2ViRWxlbWVudERpY3Rpb25hcnkgKndlYkhpdFRlc3RSZXN1bHQgPSBbW1tXZWJFbGVtZW50RGlj dGlvbmFyeSBhbGxvY10gaW5pdFdpdGhIaXRUZXN0UmVzdWx0Ol9oaXRUZXN0UmVzdWx0XSBhdXRv cmVsZWFzZV07CisgICAgaWYgKFtbX3dlYlZpZXcgVUlEZWxlZ2F0ZV0gcmVzcG9uZHNUb1NlbGVj dG9yOkBzZWxlY3Rvcihfd2ViVmlldzppbW1lZGlhdGVBY3Rpb25BbmltYXRpb25Db250cm9sbGVy Rm9ySGl0VGVzdFJlc3VsdDp3aXRoVHlwZTopXSkKKyAgICAgICAgY3VzdG9tQ2xpZW50QW5pbWF0 aW9uQ29udHJvbGxlciA9IFtbX3dlYlZpZXcgVUlEZWxlZ2F0ZV0gX3dlYlZpZXc6X3dlYlZpZXcg aW1tZWRpYXRlQWN0aW9uQW5pbWF0aW9uQ29udHJvbGxlckZvckhpdFRlc3RSZXN1bHQ6d2ViSGl0 VGVzdFJlc3VsdCB3aXRoVHlwZTpfdHlwZV07CisKKyAgICBpZiAoY3VzdG9tQ2xpZW50QW5pbWF0 aW9uQ29udHJvbGxlciA9PSBbTlNOdWxsIG51bGxdKQorICAgICAgICByZXR1cm47CisgICAgaWYg KGN1c3RvbUNsaWVudEFuaW1hdGlvbkNvbnRyb2xsZXIgJiYgW2N1c3RvbUNsaWVudEFuaW1hdGlv bkNvbnRyb2xsZXIgY29uZm9ybXNUb1Byb3RvY29sOkBwcm90b2NvbChOU0ltbWVkaWF0ZUFjdGlv bkFuaW1hdGlvbkNvbnRyb2xsZXIpXSkKKyAgICAgICAgX2ltbWVkaWF0ZUFjdGlvblJlY29nbml6 ZXIuYW5pbWF0aW9uQ29udHJvbGxlciA9IChpZCA8TlNJbW1lZGlhdGVBY3Rpb25BbmltYXRpb25D b250cm9sbGVyPiljdXN0b21DbGllbnRBbmltYXRpb25Db250cm9sbGVyOworICAgIGVsc2UKKyAg ICAgICAgX2ltbWVkaWF0ZUFjdGlvblJlY29nbml6ZXIuYW5pbWF0aW9uQ29udHJvbGxlciA9IGRl ZmF1bHRBbmltYXRpb25Db250cm9sbGVyOwogfQogCiAjcHJhZ21hIG1hcmsgUUxQcmV2aWV3TWVu dUl0ZW1EZWxlZ2F0ZSBpbXBsZW1lbnRhdGlvbgpJbmRleDogU291cmNlL1dlYktpdC9tYWMvV2Vi Vmlldy9XZWJVSURlbGVnYXRlUHJpdmF0ZS5oCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNvdXJjZS9XZWJLaXQv bWFjL1dlYlZpZXcvV2ViVUlEZWxlZ2F0ZVByaXZhdGUuaAkocmV2aXNpb24gMTc3MTkyKQorKysg U291cmNlL1dlYktpdC9tYWMvV2ViVmlldy9XZWJVSURlbGVnYXRlUHJpdmF0ZS5oCSh3b3JraW5n IGNvcHkpCkBAIC0yMzQsNiArMjM0LDEwIEBAIEBpbnRlcmZhY2UgTlNPYmplY3QgKFdlYlVJRGVs ZWdhdGVQcml2YXQKIC0gKHZvaWQpd2ViVmlldzooV2ViVmlldyAqKXNlbmRlciBzYXZlRnJhbWVW aWV3OihXZWJGcmFtZVZpZXcgKilmcmFtZVZpZXcgc2hvd2luZ1BhbmVsOihCT09MKXNob3dpbmdQ YW5lbDsKIC0gKE5TQXJyYXkgKilfd2ViVmlldzooV2ViVmlldyAqKXNlbmRlciBhY3Rpb25NZW51 SXRlbXNGb3JIaXRUZXN0UmVzdWx0OihOU0RpY3Rpb25hcnkgKiloaXRUZXN0UmVzdWx0IHdpdGhU eXBlOihXZWJBY3Rpb25NZW51VHlwZSl0eXBlIGRlZmF1bHRBY3Rpb25NZW51SXRlbXM6KE5TQXJy YXkgKilkZWZhdWx0TWVudUl0ZW1zOwogLSAoRERBY3Rpb25Db250ZXh0ICopX3dlYlZpZXc6KFdl YlZpZXcgKilzZW5kZXIgYWN0aW9uQ29udGV4dEZvckhpdFRlc3RSZXN1bHQ6KE5TRGljdGlvbmFy eSAqKWhpdFRlc3RSZXN1bHQgcmFuZ2U6KERPTVJhbmdlICoqKXJhbmdlOworCisvLyBDbGllbnRz IHRoYXQgd2FudCB0byBtYWludGFpbiBkZWZhdWx0IGJlaGF2aW9yIGNhbiByZXR1cm4gbmlsLiBU byBkaXNhYmxlIHRoZSBpbW1lZGlhdGUgYWN0aW9uIGVudGlyZWx5LCByZXR1cm4gTlNOdWxsLiBB bmQgdG8KKy8vIGRvIHNvbWV0aGluZyBjdXN0b20sIHJldHVybiBhbiBvYmplY3QgdGhhdCBjb25m b3JtcyB0byB0aGUgTlNJbW1lZGlhdGVBY3Rpb25BbmltYXRpb25Db250cm9sbGVyIHByb3RvY29s LgorLSAoaWQpX3dlYlZpZXc6KFdlYlZpZXcgKilzZW5kZXIgaW1tZWRpYXRlQWN0aW9uQW5pbWF0 aW9uQ29udHJvbGxlckZvckhpdFRlc3RSZXN1bHQ6KE5TRGljdGlvbmFyeSAqKWhpdFRlc3RSZXN1 bHQgd2l0aFR5cGU6KFdlYkltbWVkaWF0ZUFjdGlvblR5cGUpdHlwZTsKICNlbmRpZgogLSAoQk9P TCl3ZWJWaWV3OihXZWJWaWV3ICopc2VuZGVyIGRpZFByZXNzTWlzc2luZ1BsdWdpbkJ1dHRvbjoo RE9NRWxlbWVudCAqKWVsZW1lbnQ7CiAvKiEK 243169 2014-12-11 17:59:13 -0800 2014-12-11 18:14:57 -0800 Follow-up: fix a leak fix-leak.diff text/plain 1661 mitz SW5kZXg6IFNvdXJjZS9XZWJLaXQvbWFjL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBTb3VyY2Uv V2ViS2l0L21hYy9DaGFuZ2VMb2cJKHJldmlzaW9uIDE3NzE5OSkKKysrIFNvdXJjZS9XZWJLaXQv bWFjL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDEyIEBACisyMDE0LTEyLTEx ICBEYW4gQmVybnN0ZWluICA8bWl0ekBhcHBsZS5jb20+CisKKyAgICAgICAgRml4IGEgbGVhayBp bnRyb2R1Y2VkIGluIHIxNzcxOTkuCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BT ISkuCisKKyAgICAgICAgKiBXZWJWaWV3L1dlYkltbWVkaWF0ZUFjdGlvbkNvbnRyb2xsZXIubW06 CisgICAgICAgICgtW1dlYkltbWVkaWF0ZUFjdGlvbkNvbnRyb2xsZXIgX3VwZGF0ZUltbWVkaWF0 ZUFjdGlvbkl0ZW1dKToKKwogMjAxNC0xMi0xMSAgQmV0aCBEYWtpbiAgPGJkYWtpbkBhcHBsZS5j b20+CiAKICAgICAgICAgV0sxOiBOZWVkIG5ldyBTUEkgZm9yIFdlYmtpdCBjbGllbnRzIHRvIG92 ZXJyaWRlIGRlZmF1bHQgaW1tZWRpYXRlIGFjdGlvbgpJbmRleDogU291cmNlL1dlYktpdC9tYWMv V2ViVmlldy9XZWJJbW1lZGlhdGVBY3Rpb25Db250cm9sbGVyLm1tCj09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFNv dXJjZS9XZWJLaXQvbWFjL1dlYlZpZXcvV2ViSW1tZWRpYXRlQWN0aW9uQ29udHJvbGxlci5tbQko cmV2aXNpb24gMTc3MTk5KQorKysgU291cmNlL1dlYktpdC9tYWMvV2ViVmlldy9XZWJJbW1lZGlh dGVBY3Rpb25Db250cm9sbGVyLm1tCSh3b3JraW5nIGNvcHkpCkBAIC0xNDMsNyArMTQzLDcgQEAg LSAodm9pZClfdXBkYXRlSW1tZWRpYXRlQWN0aW9uSXRlbQogICAgIC8vIEFsbG93IGNsaWVudHMg dGhlIG9wcG9ydHVuaXR5IHRvIG92ZXJyaWRlIHRoZSBkZWZhdWx0IGltbWVkaWF0ZSBhY3Rpb24u CiAgICAgaWQgY3VzdG9tQ2xpZW50QW5pbWF0aW9uQ29udHJvbGxlciA9IG5pbDsKICAgICBpZiAo W1tfd2ViVmlldyBVSURlbGVnYXRlXSByZXNwb25kc1RvU2VsZWN0b3I6QHNlbGVjdG9yKF93ZWJW aWV3OmltbWVkaWF0ZUFjdGlvbkFuaW1hdGlvbkNvbnRyb2xsZXJGb3JIaXRUZXN0UmVzdWx0Ondp dGhUeXBlOildKSB7Ci0gICAgICAgIFJldGFpblB0cjxXZWJFbGVtZW50RGljdGlvbmFyeT4gd2Vi SGl0VGVzdFJlc3VsdCA9IFtbV2ViRWxlbWVudERpY3Rpb25hcnkgYWxsb2NdIGluaXRXaXRoSGl0 VGVzdFJlc3VsdDpfaGl0VGVzdFJlc3VsdF07CisgICAgICAgIFJldGFpblB0cjxXZWJFbGVtZW50 RGljdGlvbmFyeT4gd2ViSGl0VGVzdFJlc3VsdCA9IGFkb3B0TlMoW1tXZWJFbGVtZW50RGljdGlv bmFyeSBhbGxvY10gaW5pdFdpdGhIaXRUZXN0UmVzdWx0Ol9oaXRUZXN0UmVzdWx0XSk7CiAgICAg ICAgIGN1c3RvbUNsaWVudEFuaW1hdGlvbkNvbnRyb2xsZXIgPSBbW193ZWJWaWV3IFVJRGVsZWdh dGVdIF93ZWJWaWV3Ol93ZWJWaWV3IGltbWVkaWF0ZUFjdGlvbkFuaW1hdGlvbkNvbnRyb2xsZXJG b3JIaXRUZXN0UmVzdWx0OndlYkhpdFRlc3RSZXN1bHQuZ2V0KCkgd2l0aFR5cGU6X3R5cGVdOwog ICAgIH0KIAo=