[Webkit-unassigned] [Bug 207232] New: Web Inspector: ensure that `didClearWindowObjectInWorld` is dispatched for the main world first

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=207232

            Bug ID: 207232
           Summary: Web Inspector: ensure that
                    `didClearWindowObjectInWorld` is dispatched for the
                    main world first
           Product: WebKit
           Version: WebKit Local Build
          Hardware: All
                OS: All
            Status: NEW
          Severity: Normal
          Priority: P2
         Component: Web Inspector
          Assignee: webkit-unassigned at lists.webkit.org
          Reporter: drousso at apple.com
                CC: inspector-bugzilla-changes at group.apple.com

(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?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20200204/4dddf940/attachment.htm>