56445 2011-03-15 21:47:42 -0700 Logic error in -[WebHTMLView close] 2011-03-16 23:39:39 -0700 1 1 1 Unclassified WebKit WebKit Misc. 528+ (Nightly build) Mac (Intel) OS X 10.6 RESOLVED FIXED https://trac.webkit.org/browser/trunk/Source/WebKit/mac/WebView/WebHTMLView.mm P2 Normal --- 1 opendarwin webkit-unassigned ap commit-queue mjs oliver oldest_to_newest 368176 0 opendarwin 2011-03-15 21:47:42 -0700 The following code is in -[WebHTMLView close]: [_private clear]; Page* page = core([self _webView]); if (page) page->dragController()->setDraggingImageURL(KURL()); However, -[WebHTMLViewPrivate clear] has this code: dataSource = nil; And -[WebHTMLView _webView] is as follows: - (WebView *)_webView { return [_private->dataSource _webView]; } Thus, the call to [self _webView] always returns nil after [_private clear], and page will always be nil too. The "page" code in-[WebHTMLView close] was introduced in <https://trac.webkit.org/changeset/19104>. I'd say it's hard to image that this section of code was ever tested. I discovered this logic error while investigating https://bugs.webkit.org/show_bug.cgi?id=26667 368219 1 ap 2011-03-15 23:46:58 -0700 Would you be willing to make a patch removing the dead code? 368241 2 opendarwin 2011-03-16 00:02:19 -0700 I suppose. We know it hasn't done anything for 4 years. But shouldn't someone figure out what it was supposed to do in the first place? ;-) 368243 3 ap 2011-03-16 00:17:17 -0700 That would certainly be nice, but it's not really a prerequisite for removing old dead code. 368789 4 86023 opendarwin 2011-03-16 20:35:29 -0700 Created attachment 86023 Patch 368805 5 86023 ap 2011-03-16 21:34:10 -0700 Comment on attachment 86023 Patch Also marking cq+ for commit queue to land this patch. 368888 6 86023 commit-queue 2011-03-16 23:39:35 -0700 Comment on attachment 86023 Patch Clearing flags on attachment: 86023 Committed r81330: <http://trac.webkit.org/changeset/81330> 368889 7 commit-queue 2011-03-16 23:39:39 -0700 All reviewed patches have been landed. Closing bug. 86023 2011-03-16 20:35:29 -0700 2011-03-16 23:39:35 -0700 Patch bug-56445-20110316223527.patch text/plain 2907 opendarwin U3VidmVyc2lvbiBSZXZpc2lvbjogODEzMTEKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZwppbmRleCBmMTFkNWY3MjM1ZTlhYTZh ZjgxOTdiMDc1MzYwYWFhM2FlOTU5YmIwLi5iN2ZhNjRiZDUzNDZhMWNlZDNkNzYzZjRhYjA3YjRm NDdiYWZjZjA1IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvU291 cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTYgQEAKKzIwMTEtMDMtMTYgIEplZmYg Sm9obnNvbiAgPGdpdGh1YkBsYXBjYXRzb2Z0d2FyZS5jb20+CisKKyAgICAgICAgUmV2aWV3ZWQg YnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgTG9naWMgZXJyb3IgaW4gLVtXZWJIVE1MVmll dyBjbG9zZV0KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lk PTU2NDQ1CisKKyAgICAgICAgVGhlIGZ1bmN0aW9uIHNldERyYWdnaW5nSW1hZ2VVUkwoKSBpcyBu byBsb25nZXIgY2FsbGVkIGFuZCBjYW4gYmUgZGVsZXRlZC4KKworICAgICAgICBObyBuZXcgdGVz dHMuIERlbGV0aW5nIGRlYWQgY29kZS4KKworICAgICAgICAqIHBhZ2UvRHJhZ0NvbnRyb2xsZXIu aDoKKwogMjAxMS0wMy0xNiAgSm9zZXBoIFBlY29yYXJvICA8am9lcGVja0B3ZWJraXQub3JnPgog CiAgICAgICAgIFJldmlld2VkIGJ5IEtlbm5ldGggUm9oZGUgQ2hyaXN0aWFuc2VuLgpkaWZmIC0t Z2l0IGEvU291cmNlL1dlYkNvcmUvcGFnZS9EcmFnQ29udHJvbGxlci5oIGIvU291cmNlL1dlYkNv cmUvcGFnZS9EcmFnQ29udHJvbGxlci5oCmluZGV4IDBmMTc2YjFhOTAxOWZiMDMxMGQ1YjhkM2Mz NTg0MTE1OTEwMDhjYmYuLjY5MzE4OWE2YTQyN2Q4ZTM5NGE0OGQwZDljOTMzMmFiZDJiNGRjOGEg MTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL3BhZ2UvRHJhZ0NvbnRyb2xsZXIuaAorKysgYi9T b3VyY2UvV2ViQ29yZS9wYWdlL0RyYWdDb250cm9sbGVyLmgKQEAgLTY2LDcgKzY2LDYgQEAgbmFt ZXNwYWNlIFdlYkNvcmUgewogICAgICAgICB2b2lkIHNldElzSGFuZGxpbmdEcmFnKGJvb2wgaGFu ZGxpbmcpIHsgbV9pc0hhbmRsaW5nRHJhZyA9IGhhbmRsaW5nOyB9CiAgICAgICAgIGJvb2wgaXNI YW5kbGluZ0RyYWcoKSBjb25zdCB7IHJldHVybiBtX2lzSGFuZGxpbmdEcmFnOyB9CiAgICAgICAg IERyYWdPcGVyYXRpb24gc291cmNlRHJhZ09wZXJhdGlvbigpIGNvbnN0IHsgcmV0dXJuIG1fc291 cmNlRHJhZ09wZXJhdGlvbjsgfQotICAgICAgICB2b2lkIHNldERyYWdnaW5nSW1hZ2VVUkwoY29u c3QgS1VSTCYgdXJsKSB7IG1fZHJhZ2dpbmdJbWFnZVVSTCA9IHVybDsgfQogICAgICAgICBjb25z dCBLVVJMJiBkcmFnZ2luZ0ltYWdlVVJMKCkgY29uc3QgeyByZXR1cm4gbV9kcmFnZ2luZ0ltYWdl VVJMOyB9CiAgICAgICAgIHZvaWQgc2V0RHJhZ09mZnNldChjb25zdCBJbnRQb2ludCYgb2Zmc2V0 KSB7IG1fZHJhZ09mZnNldCA9IG9mZnNldDsgfQogICAgICAgICBjb25zdCBJbnRQb2ludCYgZHJh Z09mZnNldCgpIGNvbnN0IHsgcmV0dXJuIG1fZHJhZ09mZnNldDsgfQpkaWZmIC0tZ2l0IGEvU291 cmNlL1dlYktpdC9tYWMvQ2hhbmdlTG9nIGIvU291cmNlL1dlYktpdC9tYWMvQ2hhbmdlTG9nCmlu ZGV4IGYyOWJlODE2NGUzZjY0N2EzZDAwYmU5MDg0MzEyMmZjMjhjYTRmZTIuLmM0NTE4ODIwMWFi MTUxNzU2NzI5YjQxMTk3ODA1ZTIyYWMzYTM4MjkgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQv bWFjL0NoYW5nZUxvZworKysgYi9Tb3VyY2UvV2ViS2l0L21hYy9DaGFuZ2VMb2cKQEAgLTEsMyAr MSwxNiBAQAorMjAxMS0wMy0xNiAgSmVmZiBKb2huc29uICA8Z2l0aHViQGxhcGNhdHNvZnR3YXJl LmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBM b2dpYyBlcnJvciBpbiAtW1dlYkhUTUxWaWV3IGNsb3NlXQorICAgICAgICBodHRwczovL2J1Z3Mu d2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NTY0NDUKKworICAgICAgICBQYWdlIGlzIGFsd2F5 cyBuaWwgYXQgdGhpcyBwb2ludCwgYmVjYXVzZSBbc2VsZiBfd2ViVmlld10gaXMgbmlsLAorICAg ICAgICBzbyB0aGUgY29uZGl0aW9uYWwgY29kZSBpcyBuZXZlciBydW4uCisKKyAgICAgICAgKiBX ZWJWaWV3L1dlYkhUTUxWaWV3Lm1tOgorICAgICAgICAoLVtXZWJIVE1MVmlldyBjbG9zZV0pOgor CiAyMDExLTAzLTE2ICBDaHJpcyBGbGVpemFjaCAgPGNmbGVpemFjaEBhcHBsZS5jb20+CiAKICAg ICAgICAgUmV2aWV3ZWQgYnkgRGFyaW4gQWRsZXIuCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0 L21hYy9XZWJWaWV3L1dlYkhUTUxWaWV3Lm1tIGIvU291cmNlL1dlYktpdC9tYWMvV2ViVmlldy9X ZWJIVE1MVmlldy5tbQppbmRleCAwMGY2NWJkMDZmODc3M2E1Y2M5N2QzZjczYzZiNDQ4M2U0M2Fm ZDNjLi44MjA5NjA1MGJmOWRkN2JlNmNhYTJhYzY1NzgzNWUxZWMwZDk4MDdhIDEwMDY0NAotLS0g YS9Tb3VyY2UvV2ViS2l0L21hYy9XZWJWaWV3L1dlYkhUTUxWaWV3Lm1tCisrKyBiL1NvdXJjZS9X ZWJLaXQvbWFjL1dlYlZpZXcvV2ViSFRNTFZpZXcubW0KQEAgLTE5ODUsMTAgKzE5ODUsNiBAQCBz dGF0aWMgdm9pZCBfdXBkYXRlTW91c2VvdmVyVGltZXJDYWxsYmFjayhDRlJ1bkxvb3BUaW1lclJl ZiB0aW1lciwgdm9pZCAqaW5mbykKICAgICBbc2VsZiByZW1vdmVBbGxUb29sVGlwc107CiAKICAg ICBbX3ByaXZhdGUgY2xlYXJdOwotCi0gICAgUGFnZSogcGFnZSA9IGNvcmUoW3NlbGYgX3dlYlZp ZXddKTsKLSAgICBpZiAocGFnZSkKLSAgICAgICAgcGFnZS0+ZHJhZ0NvbnRyb2xsZXIoKS0+c2V0 RHJhZ2dpbmdJbWFnZVVSTChLVVJMKCkpOwogfQogCiAtIChCT09MKV9oYXNIVE1MRG9jdW1lbnQK