Fix crash when trying to execute script on a detached RenderFrame

Task-number: QTBUG-56661
Change-Id: I546222dde64c54955c62d2c30df79d4773b9973c
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

6 files changed, 54 insertions, 18 deletions

diff --git a/src/core/renderer/content_renderer_client_qt.cpp b/src/core/renderer/content_renderer_client_qt.cpp
index a6e35f4be..69f5bfefe 100644
--- a/src/core/renderer/content_renderer_client_qt.cpp
+++ b/src/core/renderer/content_renderer_client_qt.cpp
@@ -135,6 +135,12 @@ void ContentRendererClientQt::RenderFrameCreated(content::RenderFrame* render_fr
 
 void ContentRendererClientQt::RunScriptsAtDocumentStart(content::RenderFrame* render_frame)
 {
+    // Check whether the render_frame has been created and has not been detached yet.
+    // Otherwise the WebFrame is not available.
+    RenderFrameObserverQt *render_frame_observer = RenderFrameObserverQt::Get(render_frame);
+    if (!render_frame_observer || render_frame_observer->isFrameDetached())
+        return; // The frame is invisible to scripts.
+
     if (WebChannelIPCTransport *transport = WebChannelIPCTransport::Get(render_frame->GetRenderView()))
         transport->RunScriptsAtDocumentStart(render_frame);
     UserResourceController::instance()->RunScriptsAtDocumentStart(render_frame);
@@ -142,6 +148,12 @@ void ContentRendererClientQt::RunScriptsAtDocumentStart(content::RenderFrame* re
 
 void ContentRendererClientQt::RunScriptsAtDocumentEnd(content::RenderFrame* render_frame)
 {
+    // Check whether the render_frame has been created and has not been detached yet.
+    // Otherwise the WebFrame is not available.
+    RenderFrameObserverQt *render_frame_observer = RenderFrameObserverQt::Get(render_frame);
+    if (!render_frame_observer || render_frame_observer->isFrameDetached())
+        return; // The frame is invisible to scripts.
+
     UserResourceController::instance()->RunScriptsAtDocumentEnd(render_frame);
 }
 
diff --git a/src/core/renderer/render_frame_observer_qt.cpp b/src/core/renderer/render_frame_observer_qt.cpp
index 77bc89f67..53e9407db 100644
--- a/src/core/renderer/render_frame_observer_qt.cpp
+++ b/src/core/renderer/render_frame_observer_qt.cpp
@@ -55,6 +55,8 @@ namespace QtWebEngineCore {
 
 RenderFrameObserverQt::RenderFrameObserverQt(content::RenderFrame* render_frame)
     : RenderFrameObserver(render_frame)
+    , RenderFrameObserverTracker<RenderFrameObserverQt>(render_frame)
+    , m_isFrameDetached(false)
 {
 }
 
@@ -72,4 +74,14 @@ void RenderFrameObserverQt::DidCreatePepperPlugin(content::RendererPpapiHost* ho
 }
 #endif
 
+void RenderFrameObserverQt::FrameDetached()
+{
+    m_isFrameDetached = true;
+}
+
+bool RenderFrameObserverQt::isFrameDetached() const
+{
+    return m_isFrameDetached;
+}
+
 } // namespace QtWebEngineCore
diff --git a/src/core/renderer/render_frame_observer_qt.h b/src/core/renderer/render_frame_observer_qt.h
index c59b7babc..b74be5cd1 100644
--- a/src/core/renderer/render_frame_observer_qt.h
+++ b/src/core/renderer/render_frame_observer_qt.h
@@ -42,6 +42,7 @@
 
 #include "base/compiler_specific.h"
 #include "content/public/renderer/render_frame_observer.h"
+#include "content/public/renderer/render_frame_observer_tracker.h"
 
 
 namespace content {
@@ -50,7 +51,10 @@ class RenderFrame;
 
 namespace QtWebEngineCore {
 
-class RenderFrameObserverQt : public content::RenderFrameObserver {
+class RenderFrameObserverQt
+        : public content::RenderFrameObserver
+        , public content::RenderFrameObserverTracker<RenderFrameObserverQt>
+{
 public:
     explicit RenderFrameObserverQt(content::RenderFrame* render_frame);
     ~RenderFrameObserverQt();
@@ -59,9 +63,14 @@ public:
     void DidCreatePepperPlugin(content::RendererPpapiHost* host) override;
 #endif
     void OnDestruct() override { }
+    void FrameDetached() override;
+
+    bool isFrameDetached() const;
 
 private:
     DISALLOW_COPY_AND_ASSIGN(RenderFrameObserverQt);
+
+    bool m_isFrameDetached;
 };
 
 } // namespace QtWebEngineCore
diff --git a/src/core/renderer/web_channel_ipc_transport.cpp b/src/core/renderer/web_channel_ipc_transport.cpp
index 3a44bb182..2ca6640c6 100644
--- a/src/core/renderer/web_channel_ipc_transport.cpp
+++ b/src/core/renderer/web_channel_ipc_transport.cpp
@@ -164,9 +164,8 @@ void WebChannelIPCTransport::RunScriptsAtDocumentStart(content::RenderFrame *ren
 {
     // JavaScript run before this point doesn't stick, and needs to be redone.
     // ### FIXME: we should try no even installing before
-    blink::WebLocalFrame *frame = render_frame->GetWebFrame();
     if (m_installed && render_frame->IsMainFrame())
-        WebChannelTransport::Install(frame, m_installedWorldId);
+        WebChannelTransport::Install(render_frame->GetWebFrame(), m_installedWorldId);
 }
 
 
diff --git a/tests/auto/quick/qmltests/BLACKLIST b/tests/auto/quick/qmltests/BLACKLIST
index a993fcd32..dea9f3ca1 100644
--- a/tests/auto/quick/qmltests/BLACKLIST
+++ b/tests/auto/quick/qmltests/BLACKLIST
@@ -9,6 +9,3 @@ osx
 
 [WebViewGeopermission::test_geoPermissionRequest]
 osx
-
-[WebEngineViewLoadUrl::test_loadDataUrl]
-windows
diff --git a/tests/auto/quick/qmltests/data/tst_loadUrl.qml b/tests/auto/quick/qmltests/data/tst_loadUrl.qml
index 49db85a3e..2a43e1577 100644
--- a/tests/auto/quick/qmltests/data/tst_loadUrl.qml
+++ b/tests/auto/quick/qmltests/data/tst_loadUrl.qml
@@ -189,13 +189,6 @@ TestWebEngineView {
             var loadRequest = null;
 
             // Test load of a data URL
-            // QTBUG-56661: Loading an URL before the data URL load improves the probability
-            // of the render process crash after the data URL load.
-            var url = Qt.resolvedUrl("test1.html");
-            webEngineView.url = url;
-            verify(webEngineView.waitForLoadSucceeded());
-            webEngineView.clear();
-
             var dataUrl = "data:text/html,foo";
             webEngineView.url = dataUrl;
             tryCompare(loadRequestArray, "length", 2);
@@ -208,11 +201,6 @@ TestWebEngineView {
             compare(loadRequest.activeUrl, dataUrl);
             webEngineView.clear();
 
-            // QTBUG-56661: This load might fail on Windows
-            webEngineView.url = url;
-            verify(webEngineView.waitForLoadSucceeded());
-            webEngineView.clear();
-
             // Test loadHtml after a failed load
             var aboutBlank = "about:blank";
             webEngineView.url = aboutBlank; // Reset from previous test
@@ -249,6 +237,25 @@ TestWebEngineView {
             webEngineView.clear();
         }
 
+        function test_QTBUG_56661() {
+            var url = Qt.resolvedUrl("test1.html");
+
+            // Warm up phase
+            webEngineView.url = url;
+            verify(webEngineView.waitForLoadSucceeded());
+
+            // Load data URL
+            var dataUrl = "data:text/html,foo";
+            webEngineView.url = dataUrl;
+            verify(webEngineView.waitForLoadSucceeded());
+
+            // WebEngine should not try to execute user scripts in the
+            // render frame of the warm up phase otherwise the renderer
+            // crashes.
+            webEngineView.url = url;
+            verify(webEngineView.waitForLoadSucceeded());
+        }
+
         function test_stopStatus() {
             var loadRequest = null;