69037 2011-09-28 16:00:30 -0700 ASSERT if WebView's UIDelegate does not implement webView:runOpenPanel 2011-09-28 17:44:40 -0700 1 1 1 Unclassified WebKit WebKit API 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 joepeck webkit-unassigned joepeck webkit.review.bot oldest_to_newest 475056 0 joepeck 2011-09-28 16:00:30 -0700 If a WebView's UIDelegate does not implement any of the webView:runOpenPanel delegates then WebChromeClient will ASSERT(!_chooser) when the unused WebOpenPanelResultListener deallocated. WebKit expects an action (-cancel, -chooseFile:, or -chooseFiles:) to be called on the listener before its deallocated. We can either not create the listener if its not needed, or just default to cancel if no delegate is implemented. 475094 1 109096 joepeck 2011-09-28 17:06:01 -0700 Created attachment 109096 [PATCH] Proposed Fix Easiest approach was to just send a -cancel to the listener. 475097 2 joepeck 2011-09-28 17:08:33 -0700 This would also fix a leak for those cases. 475107 3 109096 ap 2011-09-28 17:20:30 -0700 Comment on attachment 109096 [PATCH] Proposed Fix Is it right to -release after -cancel? This looks like an over-release. 475112 4 joepeck 2011-09-28 17:25:11 -0700 In this case, -cancel doesn't release the object, it just sends a message down to the WebCore::FileChooser and clears the FileChooser. -release just deallocates, which expects the chooser to have already been cleared. The relevant code here is: ifndef NDEBUG - (void)dealloc { ASSERT(!_chooser); [super dealloc]; } - (void)finalize { ASSERT(!_chooser); [super finalize]; } #endif - (void)cancel { ASSERT(_chooser); if (!_chooser) return; _chooser->deref(); _chooser = 0; } 475116 5 joepeck 2011-09-28 17:27:16 -0700 (In reply to comment #4) > "it just sends a message down to the WebCore::FileChooser" Actually the -cancel path doesn't send a message. The -chooseFile(s) paths do send a message and clear the chooser. The rest of my comment is still fine. 475124 6 109096 webkit.review.bot 2011-09-28 17:44:36 -0700 Comment on attachment 109096 [PATCH] Proposed Fix Clearing flags on attachment: 109096 Committed r96279: <http://trac.webkit.org/changeset/96279> 475126 7 webkit.review.bot 2011-09-28 17:44:40 -0700 All reviewed patches have been landed. Closing bug. 109096 2011-09-28 17:06:01 -0700 2011-09-28 17:44:36 -0700 [PATCH] Proposed Fix runOpen.patch text/plain 1736 joepeck ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJLaXQvbWFjL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQv bWFjL0NoYW5nZUxvZwppbmRleCBmYzk3Mzk1Li5hOTkxZjYwIDEwMDY0NAotLS0gYS9Tb3VyY2Uv V2ViS2l0L21hYy9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYktpdC9tYWMvQ2hhbmdlTG9nCkBA IC0xLDMgKzEsMTUgQEAKKzIwMTEtMDktMjggIEpvc2VwaCBQZWNvcmFybyAgPGpvZXBlY2tAd2Vi a2l0Lm9yZz4KKworICAgICAgICBBU1NFUlQgaWYgV2ViVmlldydzIFVJRGVsZWdhdGUgZG9lcyBu b3QgaW1wbGVtZW50IHdlYlZpZXc6cnVuT3BlblBhbmVsCisgICAgICAgIGh0dHBzOi8vYnVncy53 ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD02OTAzNworCisgICAgICAgIFJldmlld2VkIGJ5IE5P Qk9EWSAoT09QUyEpLgorCisgICAgICAgICogV2ViQ29yZVN1cHBvcnQvV2ViQ2hyb21lQ2xpZW50 Lm1tOgorICAgICAgICAoV2ViQ2hyb21lQ2xpZW50OjpydW5PcGVuUGFuZWwpOgorICAgICAgICBJ ZiBubyBkZWxlZ2F0ZSBpcyBpbXBsZW1lbnRlZCwgZGlzbWlzcyB0aGUgbGlzdGVuZXIKKyAgICAg ICAgd2l0aCBhIGNhbmNlbC4gSXQgZXhwZWN0cyBzb21lIGFjdGlvbi4KKwogMjAxMS0wOS0yOCAg QW50b24gRCdBdXJpYSAgPGFkYXVyaWFAYXBwbGUuY29tPgogCiAgICAgICAgIEV4cG9zZSArW1dl YlN0b3JhZ2VNYW5hZ2VyIF9zdG9yYWdlRGlyZWN0b3J5UGF0aF0gYXMgU1BJCmRpZmYgLS1naXQg YS9Tb3VyY2UvV2ViS2l0L21hYy9XZWJDb3JlU3VwcG9ydC9XZWJDaHJvbWVDbGllbnQubW0gYi9T b3VyY2UvV2ViS2l0L21hYy9XZWJDb3JlU3VwcG9ydC9XZWJDaHJvbWVDbGllbnQubW0KaW5kZXgg NGYyZTVlNy4uYTc3MzA3NyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdC9tYWMvV2ViQ29yZVN1 cHBvcnQvV2ViQ2hyb21lQ2xpZW50Lm1tCisrKyBiL1NvdXJjZS9XZWJLaXQvbWFjL1dlYkNvcmVT dXBwb3J0L1dlYkNocm9tZUNsaWVudC5tbQpAQCAtNzQwLDggKzc0MCwxMCBAQCB2b2lkIFdlYkNo cm9tZUNsaWVudDo6cnVuT3BlblBhbmVsKEZyYW1lKiwgUGFzc1JlZlB0cjxGaWxlQ2hvb3Nlcj4g Y2hvb3NlcikKICAgICBpZCBkZWxlZ2F0ZSA9IFttX3dlYlZpZXcgVUlEZWxlZ2F0ZV07CiAgICAg aWYgKFtkZWxlZ2F0ZSByZXNwb25kc1RvU2VsZWN0b3I6QHNlbGVjdG9yKHdlYlZpZXc6cnVuT3Bl blBhbmVsRm9yRmlsZUJ1dHRvbldpdGhSZXN1bHRMaXN0ZW5lcjphbGxvd011bHRpcGxlRmlsZXM6 KV0pCiAgICAgICAgIENhbGxVSURlbGVnYXRlKG1fd2ViVmlldywgQHNlbGVjdG9yKHdlYlZpZXc6 cnVuT3BlblBhbmVsRm9yRmlsZUJ1dHRvbldpdGhSZXN1bHRMaXN0ZW5lcjphbGxvd011bHRpcGxl RmlsZXM6KSwgbGlzdGVuZXIsIGFsbG93TXVsdGlwbGVGaWxlcyk7Ci0gICAgZWxzZQorICAgIGVs c2UgaWYgKFtkZWxlZ2F0ZSByZXNwb25kc1RvU2VsZWN0b3I6QHNlbGVjdG9yKHdlYlZpZXc6cnVu T3BlblBhbmVsRm9yRmlsZUJ1dHRvbldpdGhSZXN1bHRMaXN0ZW5lcjopXSkKICAgICAgICAgQ2Fs bFVJRGVsZWdhdGUobV93ZWJWaWV3LCBAc2VsZWN0b3Iod2ViVmlldzpydW5PcGVuUGFuZWxGb3JG aWxlQnV0dG9uV2l0aFJlc3VsdExpc3RlbmVyOiksIGxpc3RlbmVyKTsKKyAgICBlbHNlCisgICAg ICAgIFtsaXN0ZW5lciBjYW5jZWxdOwogICAgIFtsaXN0ZW5lciByZWxlYXNlXTsKICAgICBFTkRf QkxPQ0tfT0JKQ19FWENFUFRJT05TOwogfQo=