Make ScriptController::canExecuteScripts redirect to ExecutionContext…

…::canExecuteScripts ScriptController isn't a good place for canExecuteScripts, as its implementation is actually about its frame()->document(). This CL moves the predicate to ExecutionContext, which prepares the Modulator to access the check w/o having to hold onto ScriptController just for the predicate. A follow-up CL is planned to migrate all ScriptController::canExecuteScripts callers. BUG=None Review-Url: https://codereview.chromium.org/2637403010 Cr-Commit-Position: refs/heads/master@{#446860}
qweri0p · Jan 28, 2017 · 647709a · 647709a
1 parent 1ad4baf
commit 647709a
Show file tree

Hide file tree

Showing 5 changed files with 51 additions and 32 deletions.
diff --git a/third_party/WebKit/Source/bindings/core/v8/ScriptController.cpp b/third_party/WebKit/Source/bindings/core/v8/ScriptController.cpp
@@ -249,34 +249,9 @@ void ScriptController::updateDocument() {
 
 bool ScriptController::canExecuteScripts(
     ReasonForCallingCanExecuteScripts reason) {
-
-  if (frame()->document() && frame()->document()->isSandboxed(SandboxScripts)) {
-    // FIXME: This message should be moved off the console once a solution to
-    // https://bugs.webkit.org/show_bug.cgi?id=103274 exists.
-    if (reason == AboutToExecuteScript)
-      frame()->document()->addConsoleMessage(ConsoleMessage::create(
-          SecurityMessageSource, ErrorMessageLevel,
-          "Blocked script execution in '" +
-              frame()->document()->url().elidedString() +
-              "' because the document's frame is sandboxed and the "
-              "'allow-scripts' permission is not set."));
-    return false;
-  }
-
-  if (frame()->document() && frame()->document()->isViewSource()) {
-    ASSERT(frame()->document()->getSecurityOrigin()->isUnique());
-    return true;
-  }
-
-  FrameLoaderClient* client = frame()->loader().client();
-  if (!client)
-    return false;
-  Settings* settings = frame()->settings();
-  const bool allowed =
-      client->allowScript(settings && settings->getScriptEnabled());
-  if (!allowed && reason == AboutToExecuteScript)
-    client->didNotAllowScript();
-  return allowed;
+  Document* document = frame()->document();
+  DCHECK(document);
+  return document->canExecuteScripts(reason);
 }
 
 bool ScriptController::executeScriptIfJavaScriptURL(const KURL& url,

diff --git a/third_party/WebKit/Source/bindings/core/v8/ScriptController.h b/third_party/WebKit/Source/bindings/core/v8/ScriptController.h
@@ -34,6 +34,7 @@
 #include "bindings/core/v8/SharedPersistent.h"
 #include "bindings/core/v8/WindowProxyManager.h"
 #include "core/CoreExport.h"
+#include "core/dom/ExecutionContext.h"
 #include "core/frame/LocalFrame.h"
 #include "platform/heap/Handle.h"
 #include "platform/loader/fetch/AccessControlStatus.h"
@@ -55,10 +56,6 @@ class Widget;
 
 typedef WTF::Vector<v8::Extension*> V8Extensions;
 
-enum ReasonForCallingCanExecuteScripts {
-  AboutToExecuteScript,
-  NotAboutToExecuteScript
-};
 
 class CORE_EXPORT ScriptController final
     : public GarbageCollected<ScriptController> {

diff --git a/third_party/WebKit/Source/core/dom/Document.cpp b/third_party/WebKit/Source/core/dom/Document.cpp
@@ -5529,6 +5529,43 @@ bool Document::isSecureTransitionTo(const KURL& url) const {
   return getSecurityOrigin()->canAccess(other.get());
 }
 
+bool Document::canExecuteScripts(ReasonForCallingCanExecuteScripts reason) {
+  if (isSandboxed(SandboxScripts)) {
+    // FIXME: This message should be moved off the console once a solution to
+    // https://bugs.webkit.org/show_bug.cgi?id=103274 exists.
+    if (reason == AboutToExecuteScript) {
+      addConsoleMessage(ConsoleMessage::create(
+          SecurityMessageSource, ErrorMessageLevel,
+          "Blocked script execution in '" + url().elidedString() +
+              "' because the document's frame is sandboxed and the "
+              "'allow-scripts' permission is not set."));
+    }
+    return false;
+  }
+
+  if (isViewSource()) {
+    DCHECK(getSecurityOrigin()->isUnique());
+    return true;
+  }
+
+  DCHECK(frame())
+      << "you are querying canExecuteScripts on a non contextDocument.";
+
+  FrameLoaderClient* client = frame()->loader().client();
+  if (!client)
+    return false;
+
+  Settings* settings = frame()->settings();
+  if (!client->allowScript(settings && settings->getScriptEnabled())) {
+    if (reason == AboutToExecuteScript)
+      client->didNotAllowScript();
+
+    return false;
+  }
+
+  return true;
+}
+
 bool Document::allowInlineEventHandler(Node* node,
                                        EventListener* listener,
                                        const String& contextURL,

diff --git a/third_party/WebKit/Source/core/dom/Document.h b/third_party/WebKit/Source/core/dom/Document.h
@@ -446,6 +446,7 @@ class CORE_EXPORT Document : public ContainerNode,
     return m_sawElementsInKnownNamespaces;
   }
 
+  bool canExecuteScripts(ReasonForCallingCanExecuteScripts) override;
   bool isRenderingReady() const {
     return haveImportsLoaded() && haveRenderBlockingStylesheetsLoaded();
   }

diff --git a/third_party/WebKit/Source/core/dom/ExecutionContext.h b/third_party/WebKit/Source/core/dom/ExecutionContext.h
@@ -55,6 +55,11 @@ class PublicURLManager;
 class SecurityOrigin;
 enum class TaskType : unsigned;
 
+enum ReasonForCallingCanExecuteScripts {
+  AboutToExecuteScript,
+  NotAboutToExecuteScript
+};
+
 class CORE_EXPORT ExecutionContext : public ContextLifecycleNotifier,
                                      public Supplementable<ExecutionContext> {
   WTF_MAKE_NONCOPYABLE(ExecutionContext);
@@ -113,6 +118,10 @@ class CORE_EXPORT ExecutionContext : public ContextLifecycleNotifier,
     return virtualCompleteURL(url);
   }
 
+  virtual bool canExecuteScripts(ReasonForCallingCanExecuteScripts) {
+    return false;
+  }
+
   bool shouldSanitizeScriptError(const String& sourceURL, AccessControlStatus);
   void dispatchErrorEvent(ErrorEvent*, AccessControlStatus);