Make ScriptController inherit LocalWindowProxyManager

third_party/WebKit/Source/bindings/core/v8/ScriptController.cpp

 /*
  * Copyright (C) 2008, 2009 Google Inc. All rights reserved.
  * Copyright (C) 2009 Apple Inc. All rights reserved.
  * Copyright (C) 2014 Opera Software ASA. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
@@ skipping 60 matching lines @@
 #include "public/platform/Platform.h"
 #include "wtf/CurrentTime.h"
 #include "wtf/StdLibExtras.h"
 #include "wtf/StringExtras.h"
 #include "wtf/text/CString.h"
 #include "wtf/text/StringBuilder.h"
 #include "wtf/text/TextPosition.h"
 
 namespace blink {
 
-ScriptController::ScriptController(LocalFrame* frame)
-    : m_windowProxyManager(LocalWindowProxyManager::create(*frame)) {}
-
-DEFINE_TRACE(ScriptController) {
-  visitor->trace(m_windowProxyManager);
-}
+ScriptController::ScriptController(LocalFrame& frame)
+    : LocalWindowProxyManager(frame) {}
 
 void ScriptController::clearForClose() {
-  m_windowProxyManager->clearForClose();
+  LocalWindowProxyManager::clearForClose();
   MainThreadDebugger::instance()->didClearContextsForFrame(frame());
 }
 
 void ScriptController::updateSecurityOrigin(SecurityOrigin* securityOrigin) {
-  m_windowProxyManager->updateSecurityOrigin(securityOrigin);
+  // This is a little hacky, but a LocalWindowProxyManager can only hold
+  // LocalWindowProxy objects, so the downcast to LocalWindowProxy is safe.
+  static_cast<LocalWindowProxy*>(mainWorldProxy())

[Yuki, 2017/01/13 10:41:59]

Did you make WindowProxyManagerImplHelper::mainWorldProxy() return a ProxyType,
didn't you?

Probably, you may need
  using WindowProxyManagerImplHelper<F, P>::mainWorldProxy;
in LocalWindowProxyManager.

[dcheng, 2017/01/13 18:57:30]

Originally, I wrote it this way for consistency originally (since the loop still
needs to downcast), but I guess it's fine to use mainWorldProxy() here.

+      ->updateSecurityOrigin(securityOrigin);
+  for (auto& entry : isolatedWorlds()) {
+    auto* isolatedWindowProxy =
+        static_cast<LocalWindowProxy*>(entry.value.get());
+    SecurityOrigin* isolatedSecurityOrigin =
+        isolatedWindowProxy->world().isolatedWorldSecurityOrigin();
+    isolatedWindowProxy->updateSecurityOrigin(isolatedSecurityOrigin);
+  }
 }
 
 namespace {
 
 V8CacheOptions cacheOptions(const ScriptResource* resource,
                             const Settings* settings) {
   V8CacheOptions v8CacheOptions(V8CacheOptionsDefault);
   if (settings)
     v8CacheOptions = settings->getV8CacheOptions();
   if (resource && !resource->response().cacheStorageCacheName().isNull()) {
@@ skipping 97 matching lines @@
           V8ScriptRunner::runCompiledScript(
               isolate(), compiledScript.script(isolate()), frame()->document()),
           result, tryCatch))
     return;
 
   TRACE_EVENT_INSTANT1(TRACE_DISABLED_BY_DEFAULT("devtools.timeline"),
                        "UpdateCounters", TRACE_EVENT_SCOPE_THREAD, "data",
                        InspectorUpdateCountersEvent::data());
 }
 
-LocalWindowProxy* ScriptController::windowProxy(DOMWrapperWorld& world) {
-  LocalWindowProxy* windowProxy = m_windowProxyManager->windowProxy(world);
-  windowProxy->initializeIfNeeded();
-  return windowProxy;
-}
-
 bool ScriptController::shouldBypassMainWorldCSP() {
   v8::HandleScope handleScope(isolate());
   v8::Local<v8::Context> context = isolate()->GetCurrentContext();
   if (context.IsEmpty() || !toDOMWindow(context))
     return false;
   DOMWrapperWorld& world = DOMWrapperWorld::current(isolate());
   return world.isIsolatedWorld() ? world.isolatedWorldHasContentSecurityPolicy()
                                  : false;
 }
 
 TextPosition ScriptController::eventHandlerPosition() const {
   ScriptableDocumentParser* parser =
       frame()->document()->scriptableDocumentParser();
   if (parser)
     return parser->textPosition();
   return TextPosition::minimumPosition();
 }
 
 void ScriptController::enableEval() {
   v8::HandleScope handleScope(isolate());
-  v8::Local<v8::Context> v8Context =
-      m_windowProxyManager->mainWorldProxy()->contextIfInitialized();
+  v8::Local<v8::Context> v8Context = mainWorldProxy()->contextIfInitialized();
   if (v8Context.IsEmpty())
     return;
   v8Context->AllowCodeGenerationFromStrings(true);
 }
 
 void ScriptController::disableEval(const String& errorMessage) {
   v8::HandleScope handleScope(isolate());
-  v8::Local<v8::Context> v8Context =
-      m_windowProxyManager->mainWorldProxy()->contextIfInitialized();
+  v8::Local<v8::Context> v8Context = mainWorldProxy()->contextIfInitialized();
   if (v8Context.IsEmpty())
     return;
   v8Context->AllowCodeGenerationFromStrings(false);
   v8Context->SetErrorMessageForCodeGenerationFromStrings(
       v8String(isolate(), errorMessage));
 }
 
 PassRefPtr<SharedPersistent<v8::Object>> ScriptController::createPluginWrapper(
     Widget* widget) {
   ASSERT(widget);
@@ skipping 19 matching lines @@
 void ScriptController::registerExtensionIfNeeded(v8::Extension* extension) {
   const V8Extensions& extensions = registeredExtensions();
   for (size_t i = 0; i < extensions.size(); ++i) {
     if (extensions[i] == extension)
       return;
   }
   v8::RegisterExtension(extension);
   registeredExtensions().push_back(extension);
 }
 
-void ScriptController::clearWindowProxy() {
-  // V8 binding expects ScriptController::clearWindowProxy only be called when a
-  // frame is loading a new page. This creates a new context for the new page.
-  m_windowProxyManager->clearForNavigation();
+void ScriptController::clearForNavigation() {

[Yuki, 2017/01/13 10:42:00]

nit: Could you put clearForClose and clearForNavigation close to each other?

[dcheng, 2017/01/13 18:57:30]

Done.

+  // V8 binding expects ScriptController::clearForNavigation() only be called
+  // when a frame is loading a new page.
+  WindowProxyManagerBase::clearForNavigation();

[Yuki, 2017/01/13 10:42:00]

Given LocalWindowProxyManager is a direct base class, this should be
LocalWindowProxyManager::clearForNavigation(), I think.

[dcheng, 2017/01/13 18:57:30]

Done.

   MainThreadDebugger::instance()->didClearContextsForFrame(frame());
 }
 
 void ScriptController::updateDocument() {
-  m_windowProxyManager->mainWorldProxy()->updateDocument();
+  mainWorldProxy()->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(
@@ skipping 156 matching lines @@
     for (size_t i = 0; i < resultArray->Length(); ++i) {
       v8::Local<v8::Value> value;
       if (!resultArray->Get(scriptState->context(), i).ToLocal(&value))
         return;
       results->push_back(value);
     }
   }
 }
 
 }  // namespace blink