1 files changed, 69 insertions, 52 deletions

diff --git a/src/core/renderer/user_resource_controller.cpp b/src/core/renderer/user_resource_controller.cpp
index 860f94a52..920fda72e 100644
--- a/src/core/renderer/user_resource_controller.cpp
+++ b/src/core/renderer/user_resource_controller.cpp
@@ -47,10 +47,10 @@
 #include "content/public/renderer/render_frame_observer.h"
 #include "content/public/renderer/render_view_observer.h"
 #include "extensions/common/url_pattern.h"
-#include "third_party/WebKit/public/web/WebDocument.h"
-#include "third_party/WebKit/public/web/WebLocalFrame.h"
-#include "third_party/WebKit/public/web/WebScriptSource.h"
-#include "third_party/WebKit/public/web/WebView.h"
+#include "third_party/blink/public/web/web_document.h"
+#include "third_party/blink/public/web/web_local_frame.h"
+#include "third_party/blink/public/web/web_script_source.h"
+#include "third_party/blink/public/web/web_view.h"
 #include "v8/include/v8.h"
 
 #include "common/qt_messages.h"
@@ -60,6 +60,8 @@
 
 #include <QRegularExpression>
 
+#include <bitset>
+
 Q_GLOBAL_STATIC(UserResourceController, qt_webengine_userResourceController)
 
 static content::RenderView * const globalScriptsIndex = 0;
@@ -134,12 +136,11 @@ public:
     RenderFrameObserverHelper(content::RenderFrame* render_frame);
 
 private:
-    ~RenderFrameObserverHelper() override;
-
     // RenderFrameObserver implementation.
+    void DidCommitProvisionalLoad(bool is_new_navigation, bool is_same_document_navigation) override;
+    void DidClearWindowObject() override;
     void DidFinishDocumentLoad() override;
     void DidFinishLoad() override;
-    void DidStartProvisionalLoad(blink::WebDocumentLoader *document_loader) override;
     void FrameDetached() override;
     void OnDestruct() override;
     bool OnMessageReceived(const IPC::Message& message) override;
@@ -148,12 +149,31 @@ private:
     void onUserScriptRemoved(const UserScriptData &);
     void onScriptsCleared();
 
-    void runScripts(UserScriptData::InjectionPoint, blink::WebLocalFrame *);
+    class Runner;
+    QScopedPointer<Runner> m_runner;
+};
+
+// Helper class to create WeakPtrs so the AfterLoad tasks can be canceled and to
+// avoid running scripts more than once per injection point.
+class UserResourceController::RenderFrameObserverHelper::Runner : public base::SupportsWeakPtr<Runner> {
+public:
+    explicit Runner(blink::WebLocalFrame *frame)
+        : m_frame(frame)
+    {
+    }
 
-    // Set of frames which are pending to get an AfterLoad invocation of runScripts, if they
-    // haven't gotten it already.
-    QSet<blink::WebLocalFrame *> m_pendingFrames;
-    base::WeakPtrFactory<RenderFrameObserverHelper> m_weakPtrFactory;
+    void run(UserScriptData::InjectionPoint p)
+    {
+        DCHECK_LT(p, m_ran.size());
+        if (!m_ran[p]) {
+            UserResourceController::instance()->runScripts(p, m_frame);
+            m_ran[p] = true;
+        }
+    }
+
+private:
+    blink::WebLocalFrame *m_frame;
+    std::bitset<3> m_ran;
 };
 
 // Used only for script cleanup on RenderView destruction.
@@ -166,14 +186,6 @@ private:
     void OnDestruct() override;
 };
 
-void UserResourceController::RenderFrameObserverHelper::runScripts(UserScriptData::InjectionPoint p, blink::WebLocalFrame *frame)
-{
-    if (p == UserScriptData::AfterLoad && !m_pendingFrames.remove(frame))
-        return;
-
-    UserResourceController::instance()->runScripts(p, frame);
-}
-
 void UserResourceController::runScripts(UserScriptData::InjectionPoint p, blink::WebLocalFrame *frame)
 {
     content::RenderFrame *renderFrame = content::RenderFrame::FromWebFrame(frame);
@@ -188,7 +200,7 @@ void UserResourceController::runScripts(UserScriptData::InjectionPoint p, blink:
     QList<uint64_t> scriptsToRun = m_viewUserScriptMap.value(0).toList();
     scriptsToRun.append(m_viewUserScriptMap.value(renderView).toList());
 
-    Q_FOREACH (uint64_t id, scriptsToRun) {
+    for (uint64_t id : qAsConst(scriptsToRun)) {
         const UserScriptData &script = m_scripts.value(id);
         if (script.injectionPoint != p
                 || (!script.injectForSubframes && !isMainFrame))
@@ -197,66 +209,71 @@ void UserResourceController::runScripts(UserScriptData::InjectionPoint p, blink:
             continue;
         blink::WebScriptSource source(blink::WebString::FromUTF8(script.source), script.url);
         if (script.worldId)
-            frame->ExecuteScriptInIsolatedWorld(script.worldId, &source, /*numSources = */1);
+            frame->ExecuteScriptInIsolatedWorld(script.worldId, source);
         else
             frame->ExecuteScript(source);
     }
 }
 
-void UserResourceController::RunScriptsAtDocumentStart(content::RenderFrame *render_frame)
-{
-    runScripts(UserScriptData::DocumentElementCreation, render_frame->GetWebFrame());
-}
-
 void UserResourceController::RunScriptsAtDocumentEnd(content::RenderFrame *render_frame)
 {
     runScripts(UserScriptData::DocumentLoadFinished, render_frame->GetWebFrame());
 }
 
 UserResourceController::RenderFrameObserverHelper::RenderFrameObserverHelper(content::RenderFrame *render_frame)
-    : content::RenderFrameObserver(render_frame), m_weakPtrFactory(this)
+    : content::RenderFrameObserver(render_frame)
 {
 }
 
-UserResourceController::RenderFrameObserverHelper::~RenderFrameObserverHelper()
+UserResourceController::RenderViewObserverHelper::RenderViewObserverHelper(content::RenderView *render_view)
+    : content::RenderViewObserver(render_view)
 {
-    m_weakPtrFactory.InvalidateWeakPtrs();
 }
 
-UserResourceController::RenderViewObserverHelper::RenderViewObserverHelper(content::RenderView *render_view)
-    : content::RenderViewObserver(render_view)
+void UserResourceController::RenderFrameObserverHelper::DidCommitProvisionalLoad(bool /* is_new_navigation */,
+                                                                                 bool is_same_document_navigation)
 {
+    if (is_same_document_navigation)
+        return;
+
+    // We are almost ready to run scripts. We still have to wait until the host
+    // process has been notified of the DidCommitProvisionalLoad event to ensure
+    // that the WebChannelTransportHost is ready to receive messages.
+
+    m_runner.reset(new Runner(render_frame()->GetWebFrame()));
 }
 
-void UserResourceController::RenderFrameObserverHelper::DidFinishDocumentLoad()
+void UserResourceController::RenderFrameObserverHelper::DidClearWindowObject()
 {
-    blink::WebLocalFrame *frame = render_frame()->GetWebFrame();
-    m_pendingFrames.insert(frame);
-    base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(FROM_HERE, base::Bind(&UserResourceController::RenderFrameObserverHelper::runScripts,
-                                                                               m_weakPtrFactory.GetWeakPtr(), UserScriptData::AfterLoad, frame),
-                                                         base::TimeDelta::FromMilliseconds(afterLoadTimeout));
+    // This is called both before and after DidCommitProvisionalLoad, non-null
+    // m_runner means it's after.
+    if (m_runner)
+        m_runner->run(UserScriptData::DocumentElementCreation);
 }
 
-void UserResourceController::RenderFrameObserverHelper::DidFinishLoad()
+void UserResourceController::RenderFrameObserverHelper::DidFinishDocumentLoad()
 {
-    blink::WebLocalFrame *frame = render_frame()->GetWebFrame();
+    // Don't run scripts if provisional load failed (DidFailProvisionalLoad
+    // called instead of DidCommitProvisionalLoad).
+    if (m_runner)
+        base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+            FROM_HERE,
+            base::BindOnce(&Runner::run, m_runner->AsWeakPtr(), UserScriptData::AfterLoad),
+            base::TimeDelta::FromMilliseconds(afterLoadTimeout));
 
-    // DidFinishDocumentLoad always comes before this, so frame has already been marked as pending.
-    base::ThreadTaskRunnerHandle::Get()->PostTask(FROM_HERE, base::Bind(&UserResourceController::RenderFrameObserverHelper::runScripts,
-                                                                        m_weakPtrFactory.GetWeakPtr(), UserScriptData::AfterLoad, frame));
 }
 
-void UserResourceController::RenderFrameObserverHelper::DidStartProvisionalLoad(blink::WebDocumentLoader *document_loader)
+void UserResourceController::RenderFrameObserverHelper::DidFinishLoad()
 {
-    Q_UNUSED(document_loader);
-    blink::WebLocalFrame *frame = render_frame()->GetWebFrame();
-    m_pendingFrames.remove(frame);
+    if (m_runner)
+        base::ThreadTaskRunnerHandle::Get()->PostTask(
+            FROM_HERE,
+            base::BindOnce(&Runner::run, m_runner->AsWeakPtr(), UserScriptData::AfterLoad));
 }
 
 void UserResourceController::RenderFrameObserverHelper::FrameDetached()
 {
-    blink::WebLocalFrame *frame = render_frame()->GetWebFrame();
-    m_pendingFrames.remove(frame);
+    m_runner.reset();
 }
 
 void UserResourceController::RenderFrameObserverHelper::OnDestruct()
@@ -326,7 +343,7 @@ UserResourceController::UserResourceController()
 {
 #if !defined(QT_NO_DEBUG) || defined(QT_FORCE_ASSERTS)
     static bool onlyCalledOnce = true;
-    Q_ASSERT(onlyCalledOnce);
+    DCHECK(onlyCalledOnce);
     onlyCalledOnce = false;
 #endif // !defined(QT_NO_DEBUG) || defined(QT_FORCE_ASSERTS)
 }
@@ -348,7 +365,7 @@ void UserResourceController::renderViewDestroyed(content::RenderView *renderView
     ViewUserScriptMap::iterator it = m_viewUserScriptMap.find(renderView);
     if (it == m_viewUserScriptMap.end()) // ASSERT maybe?
         return;
-    Q_FOREACH (uint64_t id, it.value()) {
+    for (uint64_t id : qAsConst(it.value())) {
         m_scripts.remove(id);
     }
     m_viewUserScriptMap.remove(renderView);
@@ -379,7 +396,7 @@ void UserResourceController::clearScriptsForView(content::RenderView *view)
     ViewUserScriptMap::iterator it = m_viewUserScriptMap.find(view);
     if (it == m_viewUserScriptMap.end())
         return;
-    Q_FOREACH (uint64_t id, it.value())
+    for (uint64_t id : qAsConst(it.value()))
         m_scripts.remove(id);
 
     m_viewUserScriptMap.remove(view);