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

Changeset 173551 in webkit


Ignore:
Timestamp:
Sep 11, 2014, 6:50:24 PM (12 years ago)
Author:
commit-queue@webkit.org
Message:

Web Inspector: Occasional ASSERT closing web inspector
​https://bugs.webkit.org/show_bug.cgi?id=136762

Patch by Joseph Pecoraro <Joseph Pecoraro> on 2014-09-11
Reviewed by Timothy Hatcher.

Source/JavaScriptCore:

It is harmless, and indeed possible to have an empty set of listeners
now that each Page gets its own PageDebugServer instead of a shared
global. So we should replace the null checks with isEmpty checks.
Since nobody was ever returning null, convert to references as well.

  • inspector/JSGlobalObjectScriptDebugServer.h:
  • inspector/ScriptDebugServer.cpp:

(Inspector::ScriptDebugServer::dispatchBreakpointActionLog):
(Inspector::ScriptDebugServer::dispatchBreakpointActionSound):
(Inspector::ScriptDebugServer::dispatchBreakpointActionProbe):
(Inspector::ScriptDebugServer::sourceParsed):
(Inspector::ScriptDebugServer::dispatchFunctionToListeners):
(Inspector::ScriptDebugServer::notifyDoneProcessingDebuggerEvents):
(Inspector::ScriptDebugServer::handlePause):
(Inspector::ScriptDebugServer::needPauseHandling): Deleted.

  • inspector/ScriptDebugServer.h:

Source/WebCore:

  • bindings/js/WorkerScriptDebugServer.h:
  • inspector/PageScriptDebugServer.h:
Location:
trunk/Source
Files:
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r173541 r173551  
     12014-09-11  Joseph Pecoraro  <pecoraro@apple.com>
     2
     3        Web Inspector: Occasional ASSERT closing web inspector
     4        https://bugs.webkit.org/show_bug.cgi?id=136762
     5
     6        Reviewed by Timothy Hatcher.
     7
     8        It is harmless, and indeed possible to have an empty set of listeners
     9        now that each Page gets its own PageDebugServer instead of a shared
     10        global. So we should replace the null checks with isEmpty checks.
     11        Since nobody was ever returning null, convert to references as well.
     12
     13        * inspector/JSGlobalObjectScriptDebugServer.h:
     14        * inspector/ScriptDebugServer.cpp:
     15        (Inspector::ScriptDebugServer::dispatchBreakpointActionLog):
     16        (Inspector::ScriptDebugServer::dispatchBreakpointActionSound):
     17        (Inspector::ScriptDebugServer::dispatchBreakpointActionProbe):
     18        (Inspector::ScriptDebugServer::sourceParsed):
     19        (Inspector::ScriptDebugServer::dispatchFunctionToListeners):
     20        (Inspector::ScriptDebugServer::notifyDoneProcessingDebuggerEvents):
     21        (Inspector::ScriptDebugServer::handlePause):
     22        (Inspector::ScriptDebugServer::needPauseHandling): Deleted.
     23        * inspector/ScriptDebugServer.h:
     24
    1252014-09-10  Michael Saboff  <msaboff@apple.com>
    226

  • trunk/Source/JavaScriptCore/inspector/JSGlobalObjectScriptDebugServer.h

    r166971 r173551  
    4848
    4949private:
    50     virtual ListenerSet* getListenersForGlobalObject(JSC::JSGlobalObject*) override { return &m_listeners; }
     50    virtual ListenerSet& getListeners() override { return m_listeners; }
    5151    virtual void didPause(JSC::JSGlobalObject*) override { }
    5252    virtual void didContinue(JSC::JSGlobalObject*) override { }

  • trunk/Source/JavaScriptCore/inspector/ScriptDebugServer.cpp

    r173410 r173551  
    148148        return;
    149149
    150     ListenerSet* listeners = getListenersForGlobalObject(exec->lexicalGlobalObject());
    151     if (!listeners)
    152         return;
    153     ASSERT(!listeners->isEmpty());
     150    ListenerSet& listeners = getListeners();
     151    if (listeners.isEmpty())
     152        return;
    154153
    155154    TemporaryChange<bool> change(m_callingListeners, true);
    156155
    157156    Vector<ScriptDebugListener*> listenersCopy;
    158     copyToVector(*listeners, listenersCopy);
     157    copyToVector(listeners, listenersCopy);
    159158    for (auto* listener : listenersCopy)
    160159        listener->breakpointActionLog(exec, message);
    161160}
    162161
    163 void ScriptDebugServer::dispatchBreakpointActionSound(ExecState* exec, int breakpointActionIdentifier)
    164 {
    165     if (m_callingListeners)
    166         return;
    167 
    168     ListenerSet* listeners = getListenersForGlobalObject(exec->lexicalGlobalObject());
    169     if (!listeners)
    170         return;
    171     ASSERT(!listeners->isEmpty());
     162void ScriptDebugServer::dispatchBreakpointActionSound(ExecState*, int breakpointActionIdentifier)
     163{
     164    if (m_callingListeners)
     165        return;
     166
     167    ListenerSet& listeners = getListeners();
     168    if (listeners.isEmpty())
     169        return;
    172170
    173171    TemporaryChange<bool> change(m_callingListeners, true);
    174172
    175173    Vector<ScriptDebugListener*> listenersCopy;
    176     copyToVector(*listeners, listenersCopy);
     174    copyToVector(listeners, listenersCopy);
    177175    for (auto* listener : listenersCopy)
    178176        listener->breakpointActionSound(breakpointActionIdentifier);
    … …  
    184182        return;
    185183
    186     ListenerSet* listeners = getListenersForGlobalObject(exec->lexicalGlobalObject());
    187     if (!listeners)
    188         return;
    189     ASSERT(!listeners->isEmpty());
     184    ListenerSet& listeners = getListeners();
     185    if (listeners.isEmpty())
     186        return;
    190187
    191188    TemporaryChange<bool> change(m_callingListeners, true);
    192189
    193190    Vector<ScriptDebugListener*> listenersCopy;
    194     copyToVector(*listeners, listenersCopy);
     191    copyToVector(listeners, listenersCopy);
    195192    for (auto* listener : listenersCopy)
    196193        listener->breakpointActionProbe(exec, action, m_hitCount, sample);
    … …  
    252249        return;
    253250
    254     ListenerSet* listeners = getListenersForGlobalObject(exec->lexicalGlobalObject());
    255     if (!listeners)
    256         return;
    257     ASSERT(!listeners->isEmpty());
     251    ListenerSet& listeners = getListeners();
     252    if (listeners.isEmpty())
     253        return;
    258254
    259255    TemporaryChange<bool> change(m_callingListeners, true);
    … …  
    261257    bool isError = errorLine != -1;
    262258    if (isError)
    263         dispatchFailedToParseSource(*listeners, sourceProvider, errorLine, errorMessage);
     259        dispatchFailedToParseSource(listeners, sourceProvider, errorLine, errorMessage);
    264260    else
    265         dispatchDidParseSource(*listeners, sourceProvider, isContentScript(exec));
     261        dispatchDidParseSource(listeners, sourceProvider, isContentScript(exec));
     262}
     263
     264void ScriptDebugServer::dispatchFunctionToListeners(JavaScriptExecutionCallback callback)
     265{
     266    if (m_callingListeners)
     267        return;
     268
     269    TemporaryChange<bool> change(m_callingListeners, true);
     270
     271    ListenerSet& listeners = getListeners();
     272    if (!listeners.isEmpty())
     273        dispatchFunctionToListeners(listeners, callback);
    266274}
    267275
    … …  
    274282}
    275283
    276 void ScriptDebugServer::dispatchFunctionToListeners(JavaScriptExecutionCallback callback, JSGlobalObject* globalObject)
    277 {
    278     if (m_callingListeners)
    279         return;
    280 
    281     TemporaryChange<bool> change(m_callingListeners, true);
    282 
    283     if (ListenerSet* listeners = getListenersForGlobalObject(globalObject)) {
    284         if (!listeners->isEmpty())
    285             dispatchFunctionToListeners(*listeners, callback);
    286     }
    287 }
    288 
    289284void ScriptDebugServer::notifyDoneProcessingDebuggerEvents()
    290285{
    291286    m_doneProcessingDebuggerEvents = true;
    292 }
    293 
    294 bool ScriptDebugServer::needPauseHandling(JSGlobalObject* globalObject)
    295 {
    296     return !!getListenersForGlobalObject(globalObject);
    297287}
    298288
    … …  
    317307void ScriptDebugServer::handlePause(Debugger::ReasonForPause, JSGlobalObject* vmEntryGlobalObject)
    318308{
    319     dispatchFunctionToListeners(&ScriptDebugServer::dispatchDidPause, vmEntryGlobalObject);
     309    dispatchFunctionToListeners(&ScriptDebugServer::dispatchDidPause);
    320310    LegacyProfiler::profiler()->didPause(currentDebuggerCallFrame());
    321311    didPause(vmEntryGlobalObject);
    … …  
    326316    didContinue(vmEntryGlobalObject);
    327317    LegacyProfiler::profiler()->didContinue(currentDebuggerCallFrame());
    328     dispatchFunctionToListeners(&ScriptDebugServer::dispatchDidContinue, vmEntryGlobalObject);
     318    dispatchFunctionToListeners(&ScriptDebugServer::dispatchDidContinue);
    329319}
    330320

  • trunk/Source/JavaScriptCore/inspector/ScriptDebugServer.h

    r167816 r173551  
    7575    ~ScriptDebugServer();
    7676
    77     virtual ListenerSet* getListenersForGlobalObject(JSC::JSGlobalObject*) = 0;
     77    virtual ListenerSet& getListeners() = 0;
    7878    virtual void didPause(JSC::JSGlobalObject*) = 0;
    7979    virtual void didContinue(JSC::JSGlobalObject*) = 0;
    … …  
    8484    bool evaluateBreakpointAction(const ScriptBreakpointAction&);
    8585
    86     void dispatchFunctionToListeners(JavaScriptExecutionCallback, JSC::JSGlobalObject*);
     86    void dispatchFunctionToListeners(JavaScriptExecutionCallback);
    8787    void dispatchFunctionToListeners(const ListenerSet& listeners, JavaScriptExecutionCallback);
    8888    void dispatchDidPause(ScriptDebugListener*);
    … …  
    100100
    101101    virtual void sourceParsed(JSC::ExecState*, JSC::SourceProvider*, int errorLine, const String& errorMsg) override final;
    102     virtual bool needPauseHandling(JSC::JSGlobalObject*) override final;
     102    virtual bool needPauseHandling(JSC::JSGlobalObject*) override final { return true; }
    103103    virtual void handleBreakpointHit(const JSC::Breakpoint&) override final;
    104104    virtual void handleExceptionInBreakpointCondition(JSC::ExecState*, JSC::JSValue exception) const override final;

  • trunk/Source/WebCore/ChangeLog

    r173549 r173551  
     12014-09-11  Joseph Pecoraro  <pecoraro@apple.com>
     2
     3        Web Inspector: Occasional ASSERT closing web inspector
     4        https://bugs.webkit.org/show_bug.cgi?id=136762
     5
     6        Reviewed by Timothy Hatcher.
     7
     8        * bindings/js/WorkerScriptDebugServer.h:
     9        * inspector/PageScriptDebugServer.h:
     10
    1112014-09-11  Chris Dumez  <cdumez@apple.com>
    212

  • trunk/Source/WebCore/bindings/js/WorkerScriptDebugServer.h

    r163777 r173551  
    5454
    5555private:
    56     virtual ListenerSet* getListenersForGlobalObject(JSC::JSGlobalObject*) override { return &m_listeners; }
     56    virtual ListenerSet& getListeners() override { return m_listeners; }
    5757    virtual void didPause(JSC::JSGlobalObject*) override { }
    5858    virtual void didContinue(JSC::JSGlobalObject*) override { }

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

    r167133 r173551  
    5252
    5353private:
    54     virtual ListenerSet* getListenersForGlobalObject(JSC::JSGlobalObject*) override { return &m_listeners; }
     54    virtual ListenerSet& getListeners() override { return m_listeners; }
    5555    virtual void didPause(JSC::JSGlobalObject*) override;
    5656    virtual void didContinue(JSC::JSGlobalObject*) override;
Note: See TracChangeset for help on using the changeset viewer.