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 65dc781c03cc6b678398d5f1b9f81d51457c6dc5..d456b93e34429db8faf80e45c1ab656be2ee2c2d 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 c6f14a941580bb4bfc53579ab5386058f39cb820..ffe3fd1ab2ee88e93214a548e8dc41d6652c1b7d 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 b753d23f3917cbc7ff64aa1f621013659b776f02..292b147eb2149482206c030c5118369c99e873eb 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 ec27b11b6598176fab4974e2642b46335728e208..bc4912becca75e4d135ae964277673e3bd70f9d4 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 afa286c4787043710eff9484e8297df48a964d87..7adfebb2e0b78af96544eea9dff282333e6078f6 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 e2319adc021a71370fd5d127ac16d9759a68cbf1..f7c3b8b81771966a776da7dd344db649980c3847 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 99ffd8ec03b5df5a2e3b3ed42dbe28b37c972cff..b6b06c5c33343a8920bf8083cb0d80918b3b79a3 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 6e15cedd7b5782f398af345436cc61186e777984..cc45d8adc23797f45e4cdd7893faa7ee90f15142 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 dbd247fba10335792c15d7dfedeba33bffa09e91..68ff17db99d0110578d5a5132832d4a0656909ee 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 1892ae377dabe9772908c42e7f9a3b4680a88f45..50f67b5cdb87df5e8d65c0c029a500c1008890a5 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 baee2baa933418effcf10df596bc4a816f625462..2d38c3394644c77b0d06da55eea9852f0421924f 100644 (file)
@@ -51,6 +51,10 @@ public:
 
     void didClearMainFrameWindowObject();
 
+    void mainFrameStartedLoading();
+    void mainFrameStoppedLoading();
+    void mainFrameNavigated();
+
     virtual PageScriptDebugServer& scriptDebugServer() override;
 
 protected:
index 8deacf47b981ae43607218f5a68450fa75667392..0e44f07295eb0f17c7b66a976904c09b752c67c9 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);