⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 290671 in webkit


Ignore:
Timestamp:
Mar 1, 2022, 1:19:10 PM (5 years ago)
Author:
Patrick Angle
Message:

Web app fails only when dev tools is open
​https://bugs.webkit.org/show_bug.cgi?id=235017

Reviewed by Devin Rousso.

Using the ScriptExecutionContext from event.target()->scriptExecutionContext() can result the either having a
different script context from the one used when calling willHandleEvent, or the event target's context could be
nullptr. This can occur when handling the event in EventTarget::innerInvokeEventListeners results in a
context change for the event's target, like a MessagePort that has been disentangled, which sets the script
execution context to nullptr. Because we only need the script execution context to get the correct injected
script, and the correct injected script for the action below will always be the same injected script used in
willHandleEvent, we ignore the current script execution context of the event's target and use the context the
event's target had when it began invoking event listeners.

This change protects us both from the reported crash, as well as leaving an injected script in a bad state
because we did not call setEventValue and clearEventValue on matching injected scripts for a single event.

  • inspector/InspectorInstrumentation.cpp:

(WebCore::InspectorInstrumentation::willHandleEventImpl):
(WebCore::InspectorInstrumentation::didHandleEventImpl):

  • inspector/InspectorInstrumentation.h:

(WebCore::InspectorInstrumentation::willHandleEvent):
(WebCore::InspectorInstrumentation::didHandleEvent):

  • inspector/agents/InspectorDOMDebuggerAgent.cpp:

(WebCore::InspectorDOMDebuggerAgent::willHandleEvent):
(WebCore::InspectorDOMDebuggerAgent::didHandleEvent):

  • inspector/agents/InspectorDOMDebuggerAgent.h:
Location:
trunk/Source/WebCore
Files:
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r290667 r290671  
     12022-03-01  Patrick Angle  <pangle@apple.com>
     2
     3        Web app fails only when dev tools is open
     4        https://bugs.webkit.org/show_bug.cgi?id=235017
     5
     6        Reviewed by Devin Rousso.
     7
     8        Using the `ScriptExecutionContext` from `event.target()->scriptExecutionContext()` can result the either having a
     9        different script context from the one used when calling `willHandleEvent`, or the event target's context could be
     10        `nullptr`. This can occur when handling the event in `EventTarget::innerInvokeEventListeners` results in a
     11        context change for the event's target, like a MessagePort that has been `disentangle`d, which sets the script
     12        execution context to `nullptr`. Because we only need the script execution context to get the correct injected
     13        script, and the correct injected script for the action below will always be the same injected script used in
     14        `willHandleEvent`, we ignore the current script execution context of the event's target and use the context the
     15        event's target had when it began invoking event listeners.
     16
     17        This change protects us both from the reported crash, as well as leaving an injected script in a bad state
     18        because we did not call `setEventValue` and `clearEventValue` on matching injected scripts for a single event.
     19
     20        * inspector/InspectorInstrumentation.cpp:
     21        (WebCore::InspectorInstrumentation::willHandleEventImpl):
     22        (WebCore::InspectorInstrumentation::didHandleEventImpl):
     23        * inspector/InspectorInstrumentation.h:
     24        (WebCore::InspectorInstrumentation::willHandleEvent):
     25        (WebCore::InspectorInstrumentation::didHandleEvent):
     26        * inspector/agents/InspectorDOMDebuggerAgent.cpp:
     27        (WebCore::InspectorDOMDebuggerAgent::willHandleEvent):
     28        (WebCore::InspectorDOMDebuggerAgent::didHandleEvent):
     29        * inspector/agents/InspectorDOMDebuggerAgent.h:
     30
    1312022-03-01  Martin Robinson  <mrobinson@webkit.org>
    232

  • trunk/Source/WebCore/inspector/InspectorInstrumentation.cpp

    r290223 r290671  
    420420}
    421421
    422 void InspectorInstrumentation::willHandleEventImpl(InstrumentingAgents& instrumentingAgents, Event& event, const RegisteredEventListener& listener)
     422void InspectorInstrumentation::willHandleEventImpl(InstrumentingAgents& instrumentingAgents, ScriptExecutionContext& context, Event& event, const RegisteredEventListener& listener)
    423423{
    424424    if (auto* webDebuggerAgent = instrumentingAgents.enabledWebDebuggerAgent())
    … …  
    426426
    427427    if (auto* domDebuggerAgent = instrumentingAgents.enabledDOMDebuggerAgent())
    428         domDebuggerAgent->willHandleEvent(event, listener);
    429 }
    430 
    431 void InspectorInstrumentation::didHandleEventImpl(InstrumentingAgents& instrumentingAgents, Event& event, const RegisteredEventListener& listener)
     428        domDebuggerAgent->willHandleEvent(context, event, listener);
     429}
     430
     431void InspectorInstrumentation::didHandleEventImpl(InstrumentingAgents& instrumentingAgents, ScriptExecutionContext& context, Event& event, const RegisteredEventListener& listener)
    432432{
    433433    if (auto* webDebuggerAgent = instrumentingAgents.enabledWebDebuggerAgent())
    … …  
    435435
    436436    if (auto* domDebuggerAgent = instrumentingAgents.enabledDOMDebuggerAgent())
    437         domDebuggerAgent->didHandleEvent(event, listener);
     437        domDebuggerAgent->didHandleEvent(context, event, listener);
    438438}
    439439

  • trunk/Source/WebCore/inspector/InspectorInstrumentation.h

    r290223 r290671  
    390390    static bool isEventListenerDisabledImpl(InstrumentingAgents&, EventTarget&, const AtomString& eventType, EventListener&, bool capture);
    391391    static void willDispatchEventImpl(InstrumentingAgents&, Document&, const Event&);
    392     static void willHandleEventImpl(InstrumentingAgents&, Event&, const RegisteredEventListener&);
    393     static void didHandleEventImpl(InstrumentingAgents&, Event&, const RegisteredEventListener&);
     392    static void willHandleEventImpl(InstrumentingAgents&, ScriptExecutionContext&, Event&, const RegisteredEventListener&);
     393    static void didHandleEventImpl(InstrumentingAgents&, ScriptExecutionContext&, Event&, const RegisteredEventListener&);
    394394    static void didDispatchEventImpl(InstrumentingAgents&, const Event&);
    395395    static void willDispatchEventOnWindowImpl(InstrumentingAgents&, const Event&, DOMWindow&);
    … …  
    893893    FAST_RETURN_IF_NO_FRONTENDS(void());
    894894    if (auto* agents = instrumentingAgents(context))
    895         return willHandleEventImpl(*agents, event, listener);
     895        return willHandleEventImpl(*agents, context, event, listener);
    896896}
    897897
    … …  
    900900    FAST_RETURN_IF_NO_FRONTENDS(void());
    901901    if (auto* agents = instrumentingAgents(context))
    902         return didHandleEventImpl(*agents, event, listener);
     902        return didHandleEventImpl(*agents, context, event, listener);
    903903}
    904904

  • trunk/Source/WebCore/inspector/agents/InspectorDOMDebuggerAgent.cpp

    r278253 r290671  
    219219}
    220220
    221 void InspectorDOMDebuggerAgent::willHandleEvent(Event& event, const RegisteredEventListener& registeredEventListener)
    222 {
    223     auto state = event.target()->scriptExecutionContext()->globalObject();
     221void InspectorDOMDebuggerAgent::willHandleEvent(ScriptExecutionContext& scriptExecutionContext, Event& event, const RegisteredEventListener& registeredEventListener)
     222{
     223    // `event.target()->scriptExecutionContext()` can change between `willHandleEvent` and `didHandleEvent`. The passed
     224    // `scriptExecutionContext` parameter will always match in companion calls to `willHandleEvent` and
     225    // `didHandleEvent`, and will not be null.
     226    auto state = scriptExecutionContext.globalObject();
    224227    auto injectedScript = m_injectedScriptManager.injectedScriptFor(state);
    225228    if (injectedScript.hasNoValue())
    … …  
    260263}
    261264
    262 void InspectorDOMDebuggerAgent::didHandleEvent(Event& event, const RegisteredEventListener& registeredEventListener)
    263 {
    264     auto state = event.target()->scriptExecutionContext()->globalObject();
     265void InspectorDOMDebuggerAgent::didHandleEvent(ScriptExecutionContext& scriptExecutionContext, Event& event, const RegisteredEventListener& registeredEventListener)
     266{
     267    // `event.target()->scriptExecutionContext()` can change between `willHandleEvent` and `didHandleEvent`. Here it
     268    // could also be nullptr. The passed `scriptExecutionContext` parameter here will always match in companion calls to
     269    // `willHandleEvent` and `didHandleEvent`, and will not be null.
     270    auto state = scriptExecutionContext.globalObject();
    265271    auto injectedScript = m_injectedScriptManager.injectedScriptFor(state);
    266272    if (injectedScript.hasNoValue())

  • trunk/Source/WebCore/inspector/agents/InspectorDOMDebuggerAgent.h

    r278253 r290671  
    7777    void willSendXMLHttpRequest(const String& url);
    7878    void willFetch(const String& url);
    79     void willHandleEvent(Event&, const RegisteredEventListener&);
    80     void didHandleEvent(Event&, const RegisteredEventListener&);
     79    void willHandleEvent(ScriptExecutionContext&, Event&, const RegisteredEventListener&);
     80    void didHandleEvent(ScriptExecutionContext&, Event&, const RegisteredEventListener&);
    8181    void willFireTimer(bool oneShot);
    8282    void didFireTimer(bool oneShot);
Note: See TracChangeset for help on using the changeset viewer.