207232 2020-02-04 14:58:10 -0800 Web Inspector: ensure that `didClearWindowObjectInWorld` is dispatched for the main world first 2020-02-07 16:40:39 -0800 1 1 1 Unclassified WebKit Web Inspector WebKit Local Build All All RESOLVED FIXED InRadar P2 Normal --- 206110 1 hi hi commit-queue hi inspector-bugzilla-changes pfeldman timothy webkit-bug-importer oldest_to_newest 1614764 0 hi 2020-02-04 14:58:10 -0800 (In reply to Devin Rousso from comment https://webkit.org/b/206110#c16) > View in context: > https://bugs.webkit.org/attachment.cgi?id=388830&action=review > > >> Source/WebCore/inspector/agents/page/PageRuntimeAgent.cpp:178 > >> + auto injectedScript = injectedScriptManager().injectedScriptFor(globalObject); > > > > I believe there is a subtle issue here - when didClearWindowObjectInWorld is called for the User world before it is called for the Normal world, subsequent call to clear the normal world will erase this injected script from the InjectedScriptManager through m_injectedScriptManager->discardInjectedScripts(). > > > > This can either be patched via the following change to the dispatchDidClearWindowObjectsInAllWorlds (that is somewhat hacky), or via going back to instrumenting only normal worlds and iterating over the remaining worlds within agents explicitly. > > > > void FrameLoader::dispatchDidClearWindowObjectsInAllWorlds() > > { > > Vector<Ref<DOMWrapperWorld>> worlds; > > ScriptController::getAllWorlds(worlds); > > // It is essential that the normal world is cleared first. > > // Various subsystem (InjectedScriptManager) will reset state upon normal > > // world initialization. > > Vector<DOMWrapperWorld*> nonNormalWorlds; > > for (auto& world : worlds) { > > if (world->type() == DOMWrapperWorld::Type::Normal) > > dispatchDidClearWindowObjectInWorld(world); > > else > > nonNormalWorlds.append(&world.get()); > > } > > for (auto* world : nonNormalWorlds) > > dispatchDidClearWindowObjectInWorld(*world); > > } > > Interesting! That is super subtle :P > > I think we'd ideally want to dispatch for the `mainThreadNormalWorld()` > first (as that's really the "signpost" for resetting all the injected > scripts and debugger state), then all other normal worlds, and then all > other worlds. IIUC, there should only ever be one normal world though (and > the current code supports that), so I think we can just do as you suggest. > > @Pavel Feldman, do you want to fix this, or should I? 1614768 1 389716 hi 2020-02-04 15:01:21 -0800 Created attachment 389716 Patch 1615461 2 389716 commit-queue 2020-02-05 18:13:52 -0800 Comment on attachment 389716 Patch Clearing flags on attachment: 389716 Committed r255885: <https://trac.webkit.org/changeset/255885> 1615462 3 commit-queue 2020-02-05 18:13:53 -0800 All reviewed patches have been landed. Closing bug. 1615463 4 webkit-bug-importer 2020-02-05 18:14:17 -0800 <rdar://problem/59210284> 1616459 5 pfeldman 2020-02-07 16:40:39 -0800 *** Bug 207408 has been marked as a duplicate of this bug. *** 389716 2020-02-04 15:01:21 -0800 2020-02-05 18:13:52 -0800 Patch bug-207232-20200204150120.patch text/plain 2906 hi ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJDb3JlL0No YW5nZUxvZwppbmRleCA4YmRlNzViZGNjMDA2YjJkMmUxOTlmMGM0ZWJkMWFjNWUzNjkzZmI1Li4y YjZiN2NhYTBmZmQ3NTkyNWNlNDE1OGMzYjdlNDBjMzc3ODI1YmE1IDEwMDY0NAotLS0gYS9Tb3Vy Y2UvV2ViQ29yZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCkBAIC0x LDMgKzEsMTggQEAKKzIwMjAtMDItMDQgIERldmluIFJvdXNzbyAgPGRyb3Vzc29AYXBwbGUuY29t PgorCisgICAgICAgIFdlYiBJbnNwZWN0b3I6IGVuc3VyZSB0aGF0IGBkaWRDbGVhcldpbmRvd09i amVjdEluV29ybGRgIGlzIGRpc3BhdGNoZWQgZm9yIHRoZSBtYWluIHdvcmxkIGZpcnN0CisgICAg ICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yMDcyMzIKKworICAg ICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICBJdCBpcyBuZWNlc3Nh cnkgdG8gb3JkZXIgdGhlIGBET01XcmFwcGVyV29ybGRgcyBiZWNhdXNlIGNlcnRhaW4gY2FsbGVy cyBleHBlY3QgdGhlIG1haW4KKyAgICAgICAgd29ybGQgdG8gYmUgdGhlIGZpcnN0IGl0ZW0gaW4g dGhlIGxpc3QsIGFzIHRoZXkgdXNlIHRoZSBtYWluIHdvcmxkIGFzIGFuIGluZGljYXRvciBvZiB3 aGVuCisgICAgICAgIHRoZSBwYWdlIGlzIHJlYWR5IHRvIHN0YXJ0IGV2YWx1YXRpbmcgSmF2YVNj cmlwdC4gRm9yIGV4YW1wbGUsIFdlYiBJbnNwZWN0b3Igd2FpdHMgZm9yIHRoZQorICAgICAgICBt YWluIHdvcmxkIGNoYW5nZSB0byBjbGVhciBhbnkgaW5qZWN0ZWQgc2NyaXB0cyBhbmQgZGVidWdn ZXIvYnJlYWtwb2ludCBzdGF0ZS4KKworICAgICAgICAqIGJpbmRpbmdzL2pzL1dlYkNvcmVKU0Ns aWVudERhdGEuY3BwOgorICAgICAgICAoV2ViQ29yZTo6SlNWTUNsaWVudERhdGE6OmdldEFsbFdv cmxkcyk6CisKIDIwMjAtMDItMDQgIHlvdWVubiBmYWJsZXQgIDx5b3Vlbm5AYXBwbGUuY29tPgog CiAgICAgICAgIE5ldHdvcmtQcm9jZXNzIHNob3VsZCBiZSBub3RpZmllZCBieSBVSVByb2Nlc3Mg d2hlbiBpdHMgc2VydmljZSB3b3JrZXIgcHJvY2VzcyBjb25uZWN0aW9uIHNob3VsZCBiZSBvbgpk aWZmIC0tZ2l0IGEvU291cmNlL1dlYkNvcmUvYmluZGluZ3MvanMvV2ViQ29yZUpTQ2xpZW50RGF0 YS5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9iaW5kaW5ncy9qcy9XZWJDb3JlSlNDbGllbnREYXRhLmNw cAppbmRleCBlZmU4YmExMzgzNjUzNmRiN2VjNmFkMTJkNjE2YjA3ZTIwMGRhZjY4Li43Mzc2MjJh OTA2YTNlYTc4M2RkYzA0ZTYyZjdlZDBiNjZiZWMzMDliIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2Vi Q29yZS9iaW5kaW5ncy9qcy9XZWJDb3JlSlNDbGllbnREYXRhLmNwcAorKysgYi9Tb3VyY2UvV2Vi Q29yZS9iaW5kaW5ncy9qcy9XZWJDb3JlSlNDbGllbnREYXRhLmNwcApAQCAtMTA0LDEwICsxMDQs MzUgQEAgSlNWTUNsaWVudERhdGE6On5KU1ZNQ2xpZW50RGF0YSgpCiB2b2lkIEpTVk1DbGllbnRE YXRhOjpnZXRBbGxXb3JsZHMoVmVjdG9yPFJlZjxET01XcmFwcGVyV29ybGQ+PiYgd29ybGRzKQog ewogICAgIEFTU0VSVCh3b3JsZHMuaXNFbXB0eSgpKTsKLSAgICAKKwogICAgIHdvcmxkcy5yZXNl cnZlSW5pdGlhbENhcGFjaXR5KG1fd29ybGRTZXQuc2l6ZSgpKTsKLSAgICBmb3IgKGF1dG8gaXQg PSBtX3dvcmxkU2V0LmJlZ2luKCksIGVuZCA9IG1fd29ybGRTZXQuZW5kKCk7IGl0ICE9IGVuZDsg KytpdCkKLSAgICAgICAgd29ybGRzLnVuY2hlY2tlZEFwcGVuZCgqKCppdCkpOworCisgICAgLy8g SXQgaXMgbmVjZXNzYXJ5IHRvIG9yZGVyIHRoZSBgRE9NV3JhcHBlcldvcmxkYHMgYmVjYXVzZSBj ZXJ0YWluIGNhbGxlcnMgZXhwZWN0IHRoZSBtYWluIHdvcmxkCisgICAgLy8gdG8gYmUgdGhlIGZp cnN0IGl0ZW0gaW4gdGhlIGxpc3QsIGFzIHRoZXkgdXNlIHRoZSBtYWluIHdvcmxkIGFzIGFuIGlu ZGljYXRvciBvZiB3aGVuIHRoZSBwYWdlCisgICAgLy8gaXMgcmVhZHkgdG8gc3RhcnQgZXZhbHVh dGluZyBKYXZhU2NyaXB0LiBGb3IgZXhhbXBsZSwgV2ViIEluc3BlY3RvciB3YWl0cyBmb3IgdGhl IG1haW4gd29ybGQKKyAgICAvLyBjaGFuZ2UgdG8gY2xlYXIgYW55IGluamVjdGVkIHNjcmlwdHMg YW5kIGRlYnVnZ2VyL2JyZWFrcG9pbnQgc3RhdGUuCisKKyAgICBhdXRvJiBtYWluTm9ybWFsV29y bGQgPSBtYWluVGhyZWFkTm9ybWFsV29ybGQoKTsKKworICAgIC8vIEFkZCBtYWluIG5vcm1hbCB3 b3JsZC4KKyAgICBpZiAobV93b3JsZFNldC5jb250YWlucygmbWFpbk5vcm1hbFdvcmxkKSkKKyAg ICAgICAgd29ybGRzLnVuY2hlY2tlZEFwcGVuZChtYWluTm9ybWFsV29ybGQpOworCisgICAgLy8g QWRkIG90aGVyIG5vcm1hbCB3b3JsZHMuCisgICAgZm9yIChhdXRvKiB3b3JsZCA6IG1fd29ybGRT ZXQpIHsKKyAgICAgICAgaWYgKHdvcmxkLT50eXBlKCkgIT0gRE9NV3JhcHBlcldvcmxkOjpUeXBl OjpOb3JtYWwpCisgICAgICAgICAgICBjb250aW51ZTsKKyAgICAgICAgaWYgKHdvcmxkID09ICZt YWluTm9ybWFsV29ybGQpCisgICAgICAgICAgICBjb250aW51ZTsKKyAgICAgICAgd29ybGRzLnVu Y2hlY2tlZEFwcGVuZCgqd29ybGQpOworICAgIH0KKworICAgIC8vIEFkZCBub24tbm9ybWFsIHdv cmxkcy4KKyAgICBmb3IgKGF1dG8qIHdvcmxkIDogbV93b3JsZFNldCkgeworICAgICAgICBpZiAo d29ybGQtPnR5cGUoKSA9PSBET01XcmFwcGVyV29ybGQ6OlR5cGU6Ok5vcm1hbCkKKyAgICAgICAg ICAgIGNvbnRpbnVlOworICAgICAgICB3b3JsZHMudW5jaGVja2VkQXBwZW5kKCp3b3JsZCk7Cisg ICAgfQogfQogCiB2b2lkIEpTVk1DbGllbnREYXRhOjppbml0Tm9ybWFsV29ybGQoVk0qIHZtKQo=