From 44303861fd116b3a279d26300147e89a0bf8121c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=BCri=20Valdmann?= <juri.valdmann@qt.io>
Date: Tue, 2 Apr 2019 16:24:50 +0200
Subject: [PATCH] Run non-MainWorld DocumentCreation scripts even if JS
disabled
Fixes regression, introduced by the fix for QTBUG-66011, where setting
JavascriptEnabled to false stops all scripts from running instead of only
MainWorld scripts (as documented). Only the DocumentCreation injection point is
affected.
The original change which introduced the regression consisted of moving the
DocumentCreation injection point from
ContentRendererClient::RunScriptsAtDocumentStart
to
RenderFrameObserver::DidClearWindowObject.
The problem of scripts not working on view-source URLs was fixed by this move,
but it turns out that the call to DidClearWindowObject happens to be conditional
on Document::CanExecuteScripts and this is, of course, false if JS is disabled.
Hence the regression.
This new patch moves the injection point again to a task launched from
RenderFrameObserver::DidCommitProvisionalLoad.
DidCommitProvisionalLoad and DidClearWindowObject are both indirectly called
from DocumentLoader::InstallNewDocument, however the former is called before the
Document is opened and is therefore too early for script execution. As such, the
execution is delayed by posting a task which, in theory, should be scheduled
very soon after the normal call to DidClearWindowObject.
Fixes: QTBUG-74304
Change-Id: Iac8714bcc5651c287b73181811af26996d955af5
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
---
.../renderer/user_resource_controller.cpp | 11 ++---
.../qwebenginescript/tst_qwebenginescript.cpp | 47 +++++++++++++++++++
2 files changed, 50 insertions(+), 8 deletions(-)
diff --git a/src/core/renderer/user_resource_controller.cpp b/src/core/renderer/user_resource_controller.cpp
index 920fda72e..9c03cd651 100644
--- a/src/core/renderer/user_resource_controller.cpp
+++ b/src/core/renderer/user_resource_controller.cpp
@@ -138,7 +138,6 @@ public:
private:
// RenderFrameObserver implementation.
void DidCommitProvisionalLoad(bool is_new_navigation, bool is_same_document_navigation) override;
- void DidClearWindowObject() override;
void DidFinishDocumentLoad() override;
void DidFinishLoad() override;
void FrameDetached() override;
@@ -241,14 +240,10 @@ void UserResourceController::RenderFrameObserverHelper::DidCommitProvisionalLoad
// that the WebChannelTransportHost is ready to receive messages.
m_runner.reset(new Runner(render_frame()->GetWebFrame()));
-}
-void UserResourceController::RenderFrameObserverHelper::DidClearWindowObject()
-{
- // This is called both before and after DidCommitProvisionalLoad, non-null
- // m_runner means it's after.
- if (m_runner)
- m_runner->run(UserScriptData::DocumentElementCreation);
+ base::ThreadTaskRunnerHandle::Get()->PostTask(
+ FROM_HERE,
+ base::BindOnce(&Runner::run, m_runner->AsWeakPtr(), UserScriptData::DocumentElementCreation));
}
void UserResourceController::RenderFrameObserverHelper::DidFinishDocumentLoad()
diff --git a/tests/auto/widgets/qwebenginescript/tst_qwebenginescript.cpp b/tests/auto/widgets/qwebenginescript/tst_qwebenginescript.cpp
index 0fe0ec6cf..be9e59b8c 100644
--- a/tests/auto/widgets/qwebenginescript/tst_qwebenginescript.cpp
+++ b/tests/auto/widgets/qwebenginescript/tst_qwebenginescript.cpp
@@ -23,6 +23,7 @@
#include <qwebengineprofile.h>
#include <qwebenginescript.h>
#include <qwebenginescriptcollection.h>
+#include <qwebenginesettings.h>
#include <qwebengineview.h>
#include "../util.h"
#if QT_CONFIG(webengine_webchannel)
@@ -37,6 +38,8 @@ private Q_SLOTS:
void loadEvents();
void scriptWorld_data();
void scriptWorld();
+ void scriptDisabled();
+ void viewSource();
void scriptModifications();
#if QT_CONFIG(webengine_webchannel)
void webChannel_data();
@@ -218,6 +221,50 @@ void tst_QWebEngineScript::scriptWorld()
QCOMPARE(evaluateJavaScriptSyncInWorld(&page, "typeof(userScriptTest) != \"undefined\" && userScriptTest == 1;", worldId), QVariant::fromValue(true));
}
+// Based on QTBUG-74304
+void tst_QWebEngineScript::scriptDisabled()
+{
+ QWebEnginePage page;
+ page.settings()->setAttribute(QWebEngineSettings::JavascriptEnabled, false);
+ QWebEngineScript script;
+ script.setInjectionPoint(QWebEngineScript::DocumentCreation);
+ script.setWorldId(QWebEngineScript::MainWorld);
+ script.setSourceCode("var foo = 42");
+ page.scripts().insert(script);
+ page.load(QUrl("about:blank"));
+ QSignalSpy spy(&page, &QWebEnginePage::loadFinished);
+ QTRY_COMPARE(spy.count(), 1);
+ QCOMPARE(spy.takeFirst().value(0).toBool(), true);
+ // MainWorld scripts are disabled by the setting...
+ QCOMPARE(evaluateJavaScriptSyncInWorld(&page, "foo", QWebEngineScript::MainWorld), QVariant());
+ QCOMPARE(evaluateJavaScriptSyncInWorld(&page, "foo", QWebEngineScript::ApplicationWorld), QVariant());
+ script.setWorldId(QWebEngineScript::ApplicationWorld);
+ page.scripts().clear();
+ page.scripts().insert(script);
+ page.load(QUrl("about:blank"));
+ QTRY_COMPARE(spy.count(), 1);
+ QCOMPARE(spy.takeFirst().value(0).toBool(), true);
+ // ...but ApplicationWorld scripts should still work
+ QCOMPARE(evaluateJavaScriptSyncInWorld(&page, "foo", QWebEngineScript::MainWorld), QVariant());
+ QCOMPARE(evaluateJavaScriptSyncInWorld(&page, "foo", QWebEngineScript::ApplicationWorld), QVariant(42));
+}
+
+// Based on QTBUG-66011
+void tst_QWebEngineScript::viewSource()
+{
+ QWebEnginePage page;
+ QWebEngineScript script;
+ script.setInjectionPoint(QWebEngineScript::DocumentCreation);
+ script.setWorldId(QWebEngineScript::MainWorld);
+ script.setSourceCode("var foo = 42");
+ page.scripts().insert(script);
+ page.load(QUrl("view-source:about:blank"));
+ QSignalSpy spy(&page, &QWebEnginePage::loadFinished);
+ QTRY_COMPARE(spy.count(), 1);
+ QCOMPARE(spy.takeFirst().value(0).toBool(), true);
+ QCOMPARE(evaluateJavaScriptSync(&page, "foo"), QVariant(42));
+}
+
void tst_QWebEngineScript::scriptModifications()
{
QWebEnginePage page;
--
GitLab