Web Inspector: Paused Debugger prevents page reload
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 16 Sep 2015 01:03:35 +0000 (01:03 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 16 Sep 2015 01:03:35 +0000 (01:03 +0000)
https://bugs.webkit.org/show_bug.cgi?id=148174

Patch by Joseph Pecoraro <pecoraro@apple.com> on 2015-09-15
Reviewed by Brian Burg.

Source/JavaScriptCore:

* debugger/Debugger.h:
(JSC::Debugger::suppressAllPauses):
(JSC::Debugger::setSuppressAllPauses):
* debugger/Debugger.cpp:
(JSC::Debugger::Debugger):
(JSC::Debugger::pauseIfNeeded):
* inspector/agents/InspectorDebuggerAgent.h:
* inspector/agents/InspectorDebuggerAgent.cpp:
(Inspector::InspectorDebuggerAgent::setSuppressAllPauses):
Provide a way to suppress pauses.

Source/WebCore:

When navigating the page while paused, suppress any pausing until the page
has completed navigation. If not paused and navigating, you can still pause
in pagehide and unload handlers or other late page events.

Could not write a reliable test for this at the moment.
InspectorTest.reloadPage has multiple issues with the output,
so I'll investigate making reload tests more reliable later.

* inspector/InspectorController.h:
* inspector/InspectorController.cpp:
(WebCore::InspectorController::resume): Deleted.
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::continueLoadAfterNavigationPolicy):
We now use existing InspectorInstrumentation functions instead of a method
on InspectorController during load. In dropping the method InspectorController
can drop a member variable no longer used.

* inspector/InspectorInstrumentation.h:
(WebCore::InspectorInstrumentation::willStartProvisionalLoad):
Add a new instrumentation hook.

* inspector/InspectorInstrumentation.cpp:
(WebCore::InspectorInstrumentation::willStartProvisionalLoadImpl):
(WebCore::InspectorInstrumentation::didCommitLoadImpl):
When starting or completing main frame navigations, let the PageDebuggerAgent do some work.

* inspector/PageDebuggerAgent.h:
* inspector/PageDebuggerAgent.cpp:
(WebCore::PageDebuggerAgent::mainFrameStartedLoading):
(WebCore::PageDebuggerAgent::mainFrameStoppedLoading):
(WebCore::PageDebuggerAgent::mainFrameNavigated):
Suppress pausing if navigating while paused. Otherwise behave as normal.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@189834 268f45cc-cd09-0410-ab3c-d52691b4dbfc

12 files changed:
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/debugger/Debugger.cpp
Source/JavaScriptCore/debugger/Debugger.h
Source/JavaScriptCore/inspector/agents/InspectorDebuggerAgent.cpp
Source/JavaScriptCore/inspector/agents/InspectorDebuggerAgent.h
Source/WebCore/ChangeLog
Source/WebCore/inspector/InspectorController.cpp
Source/WebCore/inspector/InspectorController.h
Source/WebCore/inspector/InspectorInstrumentation.cpp
Source/WebCore/inspector/PageDebuggerAgent.cpp
Source/WebCore/inspector/PageDebuggerAgent.h
Source/WebCore/loader/FrameLoader.cpp

index 65dc781..d456b93 100644 (file)
@@ -1,3 +1,21 @@
+2015-09-15  Joseph Pecoraro  <pecoraro@apple.com>
+
+        Web Inspector: Paused Debugger prevents page reload
+        https://bugs.webkit.org/show_bug.cgi?id=148174
+
+        Reviewed by Brian Burg.
+
+        * debugger/Debugger.h:
+        (JSC::Debugger::suppressAllPauses):
+        (JSC::Debugger::setSuppressAllPauses):
+        * debugger/Debugger.cpp:
+        (JSC::Debugger::Debugger):
+        (JSC::Debugger::pauseIfNeeded):
+        * inspector/agents/InspectorDebuggerAgent.h:
+        * inspector/agents/InspectorDebuggerAgent.cpp:
+        (Inspector::InspectorDebuggerAgent::setSuppressAllPauses):
+        Provide a way to suppress pauses.
+
 2015-09-15  Sukolsak Sakshuwong  <sukolsak@gmail.com>
 
         Implement calls to JavaScript functions in WebAssembly
index c6f14a9..ffe3fd1 100644 (file)
@@ -118,6 +118,7 @@ Debugger::Debugger(VM& vm, bool isInWorkerThread)
     , m_breakpointsActivated(true)
     , m_hasHandlerForExceptionCallback(false)
     , m_isInWorkerThread(isInWorkerThread)
+    , m_suppressAllPauses(false)
     , m_steppingMode(SteppingModeDisabled)
     , m_reasonForPause(NotPaused)
     , m_pauseOnCallFrame(0)
@@ -585,6 +586,9 @@ void Debugger::pauseIfNeeded(CallFrame* callFrame)
     if (m_isPaused)
         return;
 
+    if (m_suppressAllPauses)
+        return;
+
     JSGlobalObject* vmEntryGlobalObject = callFrame->vmEntryGlobalObject();
     if (!needPauseHandling(vmEntryGlobalObject))
         return;
index b753d23..292b147 100644 (file)
@@ -110,6 +110,9 @@ public:
     bool isPaused() const { return m_isPaused; }
     bool isStepping() const { return m_steppingMode == SteppingModeEnabled; }
 
+    bool suppressAllPauses() const { return m_suppressAllPauses; }
+    void setSuppressAllPauses(bool suppress) { m_suppressAllPauses = suppress; }
+
     virtual void sourceParsed(ExecState*, SourceProvider*, int errorLineNumber, const WTF::String& errorMessage) = 0;
 
     void exception(CallFrame*, JSValue exceptionValue, bool hasCatchHandler);
@@ -196,6 +199,7 @@ private:
     bool m_breakpointsActivated : 1;
     bool m_hasHandlerForExceptionCallback : 1;
     bool m_isInWorkerThread : 1;
+    bool m_suppressAllPauses : 1;
     unsigned m_steppingMode : 1; // SteppingMode
 
     ReasonForPause m_reasonForPause;
index ec27b11..bc4912b 100644 (file)
@@ -134,6 +134,11 @@ bool InspectorDebuggerAgent::isPaused()
     return scriptDebugServer().isPaused();
 }
 
+void InspectorDebuggerAgent::setSuppressAllPauses(bool suppress)
+{
+    scriptDebugServer().setSuppressAllPauses(suppress);
+}
+
 static RefPtr<InspectorObject> buildAssertPauseReason(const String& message)
 {
     auto reason = Inspector::Protocol::Debugger::AssertPauseReason::create().release();
index afa286c..7adfebb 100644 (file)
@@ -88,7 +88,9 @@ public:
     virtual void setOverlayMessage(ErrorString&, const String*) override;
 
     bool isPaused();
-    
+
+    void setSuppressAllPauses(bool);
+
     void handleConsoleAssert(const String& message);
 
     void schedulePauseOnNextStatement(DebuggerFrontendDispatcher::Reason breakReason, RefPtr<InspectorObject>&& data);
index e2319ad..f7c3b8b 100644 (file)
@@ -1,3 +1,43 @@
+2015-09-15  Joseph Pecoraro  <pecoraro@apple.com>
+
+        Web Inspector: Paused Debugger prevents page reload
+        https://bugs.webkit.org/show_bug.cgi?id=148174
+
+        Reviewed by Brian Burg.
+
+        When navigating the page while paused, suppress any pausing until the page
+        has completed navigation. If not paused and navigating, you can still pause
+        in pagehide and unload handlers or other late page events.
+
+        Could not write a reliable test for this at the moment.
+        InspectorTest.reloadPage has multiple issues with the output,
+        so I'll investigate making reload tests more reliable later.
+
+        * inspector/InspectorController.h:
+        * inspector/InspectorController.cpp:
+        (WebCore::InspectorController::resume): Deleted.
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::continueLoadAfterNavigationPolicy):
+        We now use existing InspectorInstrumentation functions instead of a method
+        on InspectorController during load. In dropping the method InspectorController
+        can drop a member variable no longer used.
+
+        * inspector/InspectorInstrumentation.h:
+        (WebCore::InspectorInstrumentation::willStartProvisionalLoad):
+        Add a new instrumentation hook.
+
+        * inspector/InspectorInstrumentation.cpp:
+        (WebCore::InspectorInstrumentation::willStartProvisionalLoadImpl):
+        (WebCore::InspectorInstrumentation::didCommitLoadImpl):
+        When starting or completing main frame navigations, let the PageDebuggerAgent do some work.
+
+        * inspector/PageDebuggerAgent.h:
+        * inspector/PageDebuggerAgent.cpp:
+        (WebCore::PageDebuggerAgent::mainFrameStartedLoading):
+        (WebCore::PageDebuggerAgent::mainFrameStoppedLoading):
+        (WebCore::PageDebuggerAgent::mainFrameNavigated):
+        Suppress pausing if navigating while paused. Otherwise behave as normal.
+
 2015-09-15  Brent Fulgham  <bfulgham@apple.com>
 
         [Win] Provide a means for viewing the layer tree
index 99ffd8e..b6b06c5 100644 (file)
@@ -163,10 +163,10 @@ InspectorController::InspectorController(Page& page, InspectorClient* inspectorC
     m_agents.append(WTF::move(consoleAgentPtr));
 
     auto debuggerAgentPtr = std::make_unique<PageDebuggerAgent>(pageContext, pageAgent, m_overlay.get());
-    m_debuggerAgent = debuggerAgentPtr.get();
+    PageDebuggerAgent* debuggerAgent = debuggerAgentPtr.get();
     m_agents.append(WTF::move(debuggerAgentPtr));
 
-    auto domDebuggerAgentPtr = std::make_unique<InspectorDOMDebuggerAgent>(pageContext, m_domAgent, m_debuggerAgent);
+    auto domDebuggerAgentPtr = std::make_unique<InspectorDOMDebuggerAgent>(pageContext, m_domAgent, debuggerAgent);
     m_domDebuggerAgent = domDebuggerAgentPtr.get();
     m_agents.append(WTF::move(domDebuggerAgentPtr));
 
@@ -184,8 +184,8 @@ InspectorController::InspectorController(Page& page, InspectorClient* inspectorC
         );
     }
 
-    runtimeAgent->setScriptDebugServer(&m_debuggerAgent->scriptDebugServer());
-    m_timelineAgent->setPageScriptDebugServer(&m_debuggerAgent->scriptDebugServer());
+    runtimeAgent->setScriptDebugServer(&debuggerAgent->scriptDebugServer());
+    m_timelineAgent->setPageScriptDebugServer(&debuggerAgent->scriptDebugServer());
 }
 
 InspectorController::~InspectorController()
@@ -423,14 +423,6 @@ void InspectorController::setProfilerEnabled(bool enable)
     }
 }
 
-void InspectorController::resume()
-{
-    if (m_debuggerAgent) {
-        ErrorString unused;
-        m_debuggerAgent->resume(unused);
-    }
-}
-
 bool InspectorController::developerExtrasEnabled() const
 {
     return m_page.settings().developerExtrasEnabled();
index 6e15ced..cc45d8a 100644 (file)
@@ -121,8 +121,6 @@ public:
     WEBCORE_EXPORT bool profilerEnabled() const;
     WEBCORE_EXPORT void setProfilerEnabled(bool);
 
-    void resume();
-
     InspectorClient* inspectorClient() const { return m_inspectorClient; }
     InspectorPageAgent* pageAgent() const { return m_pageAgent; }
 
@@ -156,7 +154,6 @@ private:
     InspectorDOMAgent* m_domAgent { nullptr };
     InspectorResourceAgent* m_resourceAgent { nullptr };
     InspectorPageAgent* m_pageAgent { nullptr };
-    PageDebuggerAgent* m_debuggerAgent { nullptr };
     InspectorDOMDebuggerAgent* m_domDebuggerAgent { nullptr };
     InspectorTimelineAgent* m_timelineAgent { nullptr };
 
index dbd247f..68ff17d 100644 (file)
@@ -798,6 +798,9 @@ void InspectorInstrumentation::didCommitLoadImpl(InstrumentingAgents& instrument
 
         if (InspectorLayerTreeAgent* layerTreeAgent = instrumentingAgents.inspectorLayerTreeAgent())
             layerTreeAgent->reset();
+
+        if (PageDebuggerAgent* pageDebuggerAgent = instrumentingAgents.pageDebuggerAgent())
+            pageDebuggerAgent->mainFrameNavigated();
     }
 
     if (InspectorDOMAgent* domAgent = instrumentingAgents.inspectorDOMAgent())
@@ -828,12 +831,22 @@ void InspectorInstrumentation::loaderDetachedFromFrameImpl(InstrumentingAgents&
 
 void InspectorInstrumentation::frameStartedLoadingImpl(InstrumentingAgents& instrumentingAgents, Frame& frame)
 {
+    if (frame.isMainFrame()) {
+        if (PageDebuggerAgent* pageDebuggerAgent = instrumentingAgents.pageDebuggerAgent())
+            pageDebuggerAgent->mainFrameStartedLoading();
+    }
+
     if (InspectorPageAgent* inspectorPageAgent = instrumentingAgents.inspectorPageAgent())
         inspectorPageAgent->frameStartedLoading(frame);
 }
 
 void InspectorInstrumentation::frameStoppedLoadingImpl(InstrumentingAgents& instrumentingAgents, Frame& frame)
 {
+    if (frame.isMainFrame()) {
+        if (PageDebuggerAgent* pageDebuggerAgent = instrumentingAgents.pageDebuggerAgent())
+            pageDebuggerAgent->mainFrameStoppedLoading();
+    }
+
     if (InspectorPageAgent* inspectorPageAgent = instrumentingAgents.inspectorPageAgent())
         inspectorPageAgent->frameStoppedLoading(frame);
 }
index 1892ae3..50f67b5 100644 (file)
@@ -147,4 +147,23 @@ void PageDebuggerAgent::didClearMainFrameWindowObject()
     didClearGlobalObject();
 }
 
+void PageDebuggerAgent::mainFrameStartedLoading()
+{
+    if (isPaused()) {
+        setSuppressAllPauses(true);
+        ErrorString unused;
+        resume(unused);
+    }
+}
+
+void PageDebuggerAgent::mainFrameStoppedLoading()
+{
+    setSuppressAllPauses(false);
+}
+
+void PageDebuggerAgent::mainFrameNavigated()
+{
+    setSuppressAllPauses(false);
+}
+
 } // namespace WebCore
index baee2ba..2d38c33 100644 (file)
@@ -51,6 +51,10 @@ public:
 
     void didClearMainFrameWindowObject();
 
+    void mainFrameStartedLoading();
+    void mainFrameStoppedLoading();
+    void mainFrameNavigated();
+
     virtual PageScriptDebugServer& scriptDebugServer() override;
 
 protected:
index 8deacf4..0e44f07 100644 (file)
@@ -2983,11 +2983,6 @@ void FrameLoader::continueLoadAfterNavigationPolicy(const ResourceRequest& reque
     if (!m_frame.page())
         return;
 
-    if (Page* page = m_frame.page()) {
-        if (m_frame.isMainFrame())
-            page->inspectorController().resume();
-    }
-
     setProvisionalDocumentLoader(m_policyDocumentLoader.get());
     m_loadType = type;
     setState(FrameStateProvisional);