Web Inspector: Canvas Tab: main WebGL canvas on acko.net has no reported size
authorwebkit@devinrousso.com <webkit@devinrousso.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 16 Mar 2018 07:01:01 +0000 (07:01 +0000)
committerwebkit@devinrousso.com <webkit@devinrousso.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 16 Mar 2018 07:01:01 +0000 (07:01 +0000)
https://bugs.webkit.org/show_bug.cgi?id=178798
<rdar://problem/35175740>

Reviewed by Brian Burg.

When the main frame navigates, the DOMAgent would recieve two InspectorInstrumentation calls,
one when the frame begins its navigation and the other when the document for that frame has
loaded. Both of these would discard the DOMAgent's bindings, which included the map of
`nodeId`s. This was an issue for canvases, as the frontend would be notified whenever any
canvas is created, which would usually occur before the `DOMContentLoaded` event is fired.
As a result, the canvases would attempt to retrieve their associated node, only to have the
DOMAgent discard those bindings quickly thereafter.

This patch removes DOMAgent's and DOMDebuggerAgent's (parity) instrumentation hooks for the
latter event, ensuring that the bindings are only discarded once.

* inspector/agents/InspectorDOMAgent.h:
* inspector/agents/InspectorDOMAgent.cpp:
(WebCore::InspectorDOMAgent::mainFrameDOMContentLoaded): Deleted.

* inspector/agents/InspectorDOMDebuggerAgent.h:
* inspector/agents/InspectorDOMDebuggerAgent.cpp:
(WebCore::InspectorDOMDebuggerAgent::frameDocumentUpdated): Added.
(WebCore::InspectorDOMDebuggerAgent::mainFrameDOMContentLoaded): Deleted.

* inspector/InspectorInstrumentation.cpp:
(WebCore::InspectorInstrumentation::domContentLoadedEventFiredImpl):
(WebCore::InspectorInstrumentation::frameDocumentUpdatedImpl):

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

Source/WebCore/ChangeLog
Source/WebCore/inspector/InspectorInstrumentation.cpp
Source/WebCore/inspector/agents/InspectorDOMAgent.cpp
Source/WebCore/inspector/agents/InspectorDOMAgent.h
Source/WebCore/inspector/agents/InspectorDOMDebuggerAgent.cpp
Source/WebCore/inspector/agents/InspectorDOMDebuggerAgent.h

index 647b7f5..786450c 100644 (file)
@@ -1,3 +1,35 @@
+2018-03-16  Devin Rousso  <webkit@devinrousso.com>
+
+        Web Inspector: Canvas Tab: main WebGL canvas on acko.net has no reported size
+        https://bugs.webkit.org/show_bug.cgi?id=178798
+        <rdar://problem/35175740>
+
+        Reviewed by Brian Burg.
+
+        When the main frame navigates, the DOMAgent would recieve two InspectorInstrumentation calls,
+        one when the frame begins its navigation and the other when the document for that frame has
+        loaded. Both of these would discard the DOMAgent's bindings, which included the map of
+        `nodeId`s. This was an issue for canvases, as the frontend would be notified whenever any
+        canvas is created, which would usually occur before the `DOMContentLoaded` event is fired.
+        As a result, the canvases would attempt to retrieve their associated node, only to have the
+        DOMAgent discard those bindings quickly thereafter.
+
+        This patch removes DOMAgent's and DOMDebuggerAgent's (parity) instrumentation hooks for the
+        latter event, ensuring that the bindings are only discarded once.
+
+        * inspector/agents/InspectorDOMAgent.h:
+        * inspector/agents/InspectorDOMAgent.cpp:
+        (WebCore::InspectorDOMAgent::mainFrameDOMContentLoaded): Deleted.
+
+        * inspector/agents/InspectorDOMDebuggerAgent.h:
+        * inspector/agents/InspectorDOMDebuggerAgent.cpp:
+        (WebCore::InspectorDOMDebuggerAgent::frameDocumentUpdated): Added.
+        (WebCore::InspectorDOMDebuggerAgent::mainFrameDOMContentLoaded): Deleted.
+
+        * inspector/InspectorInstrumentation.cpp:
+        (WebCore::InspectorInstrumentation::domContentLoadedEventFiredImpl):
+        (WebCore::InspectorInstrumentation::frameDocumentUpdatedImpl):
+
 2018-03-15  Tim Horton  <timothy_horton@apple.com>
 
         Include CADisplayLink explicitly where needed, instead of all of CA
index 8842fe9..1753d1a 100644 (file)
@@ -648,12 +648,6 @@ void InspectorInstrumentation::domContentLoadedEventFiredImpl(InstrumentingAgent
     if (!frame.isMainFrame())
         return;
 
-    if (InspectorDOMAgent* domAgent = instrumentingAgents.inspectorDOMAgent())
-        domAgent->mainFrameDOMContentLoaded();
-
-    if (InspectorDOMDebuggerAgent* domDebuggerAgent = instrumentingAgents.inspectorDOMDebuggerAgent())
-        domDebuggerAgent->mainFrameDOMContentLoaded();
-
     if (InspectorPageAgent* pageAgent = instrumentingAgents.inspectorPageAgent())
         pageAgent->domContentEventFired();
 }
@@ -731,6 +725,9 @@ void InspectorInstrumentation::frameDocumentUpdatedImpl(InstrumentingAgents& ins
 {
     if (InspectorDOMAgent* domAgent = instrumentingAgents.inspectorDOMAgent())
         domAgent->frameDocumentUpdated(frame);
+
+    if (InspectorDOMDebuggerAgent* domDebuggerAgent = instrumentingAgents.inspectorDOMDebuggerAgent())
+        domDebuggerAgent->frameDocumentUpdated(frame);
 }
 
 void InspectorInstrumentation::loaderDetachedFromFrameImpl(InstrumentingAgents& instrumentingAgents, DocumentLoader& loader)
index 03d06d4..13dbbad 100644 (file)
@@ -2023,14 +2023,6 @@ Node* InspectorDOMAgent::innerParentNode(Node* node)
     return node->parentNode();
 }
 
-void InspectorDOMAgent::mainFrameDOMContentLoaded()
-{
-    // Re-push document once it is loaded.
-    discardBindings();
-    if (m_documentRequested)
-        m_frontendDispatcher->documentUpdated();
-}
-
 void InspectorDOMAgent::didCommitLoad(Document* document)
 {
     RefPtr<Element> frameOwner = document->ownerElement();
index 4a76ded..89bd9d1 100644 (file)
@@ -176,7 +176,6 @@ public:
     // Callbacks that don't directly correspond to an instrumentation entry point.
     void setDocument(Document*);
     void releaseDanglingNodes();
-    void mainFrameDOMContentLoaded();
 
     void styleAttributeInvalidated(const Vector<Element*>& elements);
 
index 9e8b70b..d122512 100644 (file)
@@ -32,6 +32,7 @@
 #include "config.h"
 #include "InspectorDOMDebuggerAgent.h"
 
+#include "Frame.h"
 #include "HTMLElement.h"
 #include "InspectorDOMAgent.h"
 #include "InstrumentingAgents.h"
@@ -109,8 +110,11 @@ void InspectorDOMDebuggerAgent::discardAgent()
     m_debuggerAgent = nullptr;
 }
 
-void InspectorDOMDebuggerAgent::mainFrameDOMContentLoaded()
+void InspectorDOMDebuggerAgent::frameDocumentUpdated(Frame& frame)
 {
+    if (!frame.isMainFrame())
+        return;
+
     discardBindings();
 }
 
index 39acf5a..d5581a5 100644 (file)
@@ -41,6 +41,7 @@
 namespace WebCore {
 
 class Element;
+class Frame;
 class InspectorDOMAgent;
 class Node;
 
@@ -72,7 +73,7 @@ public:
     void willModifyDOMAttr(Element&);
     void willSendXMLHttpRequest(const String& url);
     void pauseOnNativeEventIfNeeded(bool isDOMEvent, const String& eventName, bool synchronous);
-    void mainFrameDOMContentLoaded();
+    void frameDocumentUpdated(Frame&);
 
     void didCreateFrontendAndBackend(Inspector::FrontendRouter*, Inspector::BackendDispatcher*) final;
     void willDestroyFrontendAndBackend(Inspector::DisconnectReason) final;