binding: Moves the check for the first access to the initial document into BindingSecurity.

Checks the access to the initial document and reports it not only at
securityCheck() in V8Window.cpp but also at every call to
BindingSecurity::shouldAllowAccessTo() because V8 only calls back
securityCheck() on property lookups, and not for function invocation.

BindingSecurity::shouldAllowAccessTo() is called with every possible
cross-origin window, which means every possible new window.  Thus,
shouldAllowAccessTo() should be the right place to check the access
to the initial document.

BUG=630662
TBR=benwells@chromium.org

Review-Url: https://codereview.chromium.org/2209303002
Cr-Commit-Position: refs/heads/master@{#412195}
diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/AwTestBase.java b/android_webview/javatests/src/org/chromium/android_webview/test/AwTestBase.java
index f14ba48..cb4e8b0 100644
--- a/android_webview/javatests/src/org/chromium/android_webview/test/AwTestBase.java
+++ b/android_webview/javatests/src/org/chromium/android_webview/test/AwTestBase.java
@@ -659,8 +659,15 @@
         });
 
         OnPageFinishedHelper onPageFinishedHelper = popupContentsClient.getOnPageFinishedHelper();
-        int callCount = onPageFinishedHelper.getCallCount();
-        onPageFinishedHelper.waitForCallback(callCount, 1, WAIT_TIMEOUT_MS, TimeUnit.MILLISECONDS);
+        int finishCallCount = onPageFinishedHelper.getCallCount();
+        TestAwContentsClient.OnReceivedTitleHelper onReceivedTitleHelper =
+                popupContentsClient.getOnReceivedTitleHelper();
+        int titleCallCount = onReceivedTitleHelper.getCallCount();
+
+        onPageFinishedHelper.waitForCallback(finishCallCount, 1, WAIT_TIMEOUT_MS,
+                TimeUnit.MILLISECONDS);
+        onReceivedTitleHelper.waitForCallback(titleCallCount, 1, WAIT_TIMEOUT_MS,
+                TimeUnit.MILLISECONDS);
 
         return new PopupInfo(popupContentsClient, popupContainerView, popupContents);
     }
diff --git a/android_webview/javatests/src/org/chromium/android_webview/test/PopupWindowTest.java b/android_webview/javatests/src/org/chromium/android_webview/test/PopupWindowTest.java
index 3dabeb7..6c5c793 100644
--- a/android_webview/javatests/src/org/chromium/android_webview/test/PopupWindowTest.java
+++ b/android_webview/javatests/src/org/chromium/android_webview/test/PopupWindowTest.java
@@ -77,15 +77,23 @@
                         + "  window.popupWindow.document.body.innerHTML = 'Hello from the parent!';"
                         + "}</script>");
 
+        final String popupPageHtml = CommonResources.makeHtmlPageFrom(
+                "<title>" + POPUP_TITLE + "</title>",
+                "This is a popup window");
+
         triggerPopup(mParentContents, mParentContentsClient, mWebServer, parentPageHtml,
-                null, popupPath, "tryOpenWindow()");
+                popupPageHtml, popupPath, "tryOpenWindow()");
+        PopupInfo popupInfo = connectPendingPopup(mParentContents);
+        assertEquals(POPUP_TITLE, getTitleOnUiThread(popupInfo.popupContents));
+
         TestCallbackHelperContainer.OnPageFinishedHelper onPageFinishedHelper =
-                connectPendingPopup(mParentContents).popupContentsClient.getOnPageFinishedHelper();
+                popupInfo.popupContentsClient.getOnPageFinishedHelper();
         final int onPageFinishedCallCount = onPageFinishedHelper.getCallCount();
+
         executeJavaScriptAndWaitForResult(mParentContents, mParentContentsClient,
                 "modifyDomOfPopup()");
+        // Test that |waitForCallback| does not time out.
         onPageFinishedHelper.waitForCallback(onPageFinishedCallCount);
-        assertEquals("about:blank", onPageFinishedHelper.getUrl());
     }
 
     @SmallTest
diff --git a/chrome/browser/apps/app_browsertest.cc b/chrome/browser/apps/app_browsertest.cc
index 7300ae4..f513a78 100644
--- a/chrome/browser/apps/app_browsertest.cc
+++ b/chrome/browser/apps/app_browsertest.cc
@@ -41,6 +41,7 @@
 #include "content/public/browser/host_zoom_map.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_widget_host_view.h"
+#include "content/public/test/browser_test_utils.h"
 #include "content/public/test/test_utils.h"
 #include "extensions/browser/app_window/app_window.h"
 #include "extensions/browser/app_window/app_window_registry.h"
@@ -451,8 +452,10 @@
       message_;
   observer.Wait();
   ASSERT_EQ(kExpectedNumberOfTabs, observer.tabs().size());
+  content::WaitForLoadStop(observer.tabs()[kExpectedNumberOfTabs - 1]);
   EXPECT_EQ(GURL(kChromiumURL),
             observer.tabs()[kExpectedNumberOfTabs - 1]->GetURL());
+  content::WaitForLoadStop(observer.tabs()[kExpectedNumberOfTabs - 2]);
   EXPECT_EQ(GURL(kChromiumURL),
             observer.tabs()[kExpectedNumberOfTabs - 2]->GetURL());
 }
diff --git a/chrome/browser/ssl/ssl_browser_tests.cc b/chrome/browser/ssl/ssl_browser_tests.cc
index 6e95c62..28422f9 100644
--- a/chrome/browser/ssl/ssl_browser_tests.cc
+++ b/chrome/browser/ssl/ssl_browser_tests.cc
@@ -1954,11 +1954,11 @@
   nav_observer.Wait();
   // Since the popup is showing an interstitial, it shouldn't have a last
   // committed entry.
+  content::WaitForInterstitialAttach(popup);
   EXPECT_FALSE(popup->GetController().GetLastCommittedEntry());
   ASSERT_TRUE(popup->GetController().GetVisibleEntry());
   EXPECT_EQ(https_server_expired_.GetURL("/ssl/bad_iframe.html"),
             popup->GetController().GetVisibleEntry()->GetURL());
-  content::WaitForInterstitialAttach(popup);
   EXPECT_TRUE(popup->ShowingInterstitialPage());
 
   // Add another tab to make sure the browser does not exit when we close
diff --git a/third_party/WebKit/Source/bindings/core/v8/BindingSecurity.cpp b/third_party/WebKit/Source/bindings/core/v8/BindingSecurity.cpp
index d766a75..262a72b 100644
--- a/third_party/WebKit/Source/bindings/core/v8/BindingSecurity.cpp
+++ b/third_party/WebKit/Source/bindings/core/v8/BindingSecurity.cpp
@@ -42,20 +42,36 @@
 
 namespace blink {
 
-static bool isOriginAccessibleFromDOMWindow(const SecurityOrigin* targetOrigin, const LocalDOMWindow* accessingWindow)
-{
-    return accessingWindow && accessingWindow->document()->getSecurityOrigin()->canAccessCheckSuborigins(targetOrigin);
-}
+namespace {
 
-static bool canAccessFrame(v8::Isolate* isolate, const LocalDOMWindow* accessingWindow, const SecurityOrigin* targetFrameOrigin, const DOMWindow* targetWindow, ExceptionState& exceptionState)
+bool canAccessFrameInternal(const LocalDOMWindow* accessingWindow, const SecurityOrigin* targetFrameOrigin, const DOMWindow* targetWindow)
 {
-    ASSERT_WITH_SECURITY_IMPLICATION(!(targetWindow && targetWindow->frame()) || targetWindow == targetWindow->frame()->domWindow());
+    SECURITY_CHECK(!(targetWindow && targetWindow->frame())
+        || targetWindow == targetWindow->frame()->domWindow());
 
     // It's important to check that targetWindow is a LocalDOMWindow: it's
     // possible for a remote frame and local frame to have the same security
     // origin, depending on the model being used to allocate Frames between
     // processes. See https://crbug.com/601629.
-    if (targetWindow && targetWindow->isLocalDOMWindow() && isOriginAccessibleFromDOMWindow(targetFrameOrigin, accessingWindow))
+    if (!(accessingWindow && targetWindow && targetWindow->isLocalDOMWindow()))
+        return false;
+
+    const SecurityOrigin* accessingOrigin =
+        accessingWindow->document()->getSecurityOrigin();
+    if (!accessingOrigin->canAccessCheckSuborigins(targetFrameOrigin))
+        return false;
+
+    // Notify the loader's client if the initial document has been accessed.
+    LocalFrame* targetFrame = toLocalDOMWindow(targetWindow)->frame();
+    if (targetFrame->loader().stateMachine()->isDisplayingInitialEmptyDocument())
+        targetFrame->loader().didAccessInitialDocument();
+
+    return true;
+}
+
+bool canAccessFrame(const LocalDOMWindow* accessingWindow, const SecurityOrigin* targetFrameOrigin, const DOMWindow* targetWindow, ExceptionState& exceptionState)
+{
+    if (canAccessFrameInternal(accessingWindow, targetFrameOrigin, targetWindow))
         return true;
 
     if (targetWindow)
@@ -63,29 +79,25 @@
     return false;
 }
 
-static bool canAccessFrame(v8::Isolate* isolate, const LocalDOMWindow* accessingWindow, SecurityOrigin* targetFrameOrigin, const DOMWindow* targetWindow, SecurityReportingOption reportingOption = ReportSecurityError)
+bool canAccessFrame(const LocalDOMWindow* accessingWindow, SecurityOrigin* targetFrameOrigin, const DOMWindow* targetWindow, SecurityReportingOption reportingOption = ReportSecurityError)
 {
-    ASSERT_WITH_SECURITY_IMPLICATION(!(targetWindow && targetWindow->frame()) || targetWindow == targetWindow->frame()->domWindow());
-
-    // It's important to check that targetWindow is a LocalDOMWindow: it's
-    // possible for a remote frame and local frame to have the same security
-    // origin, depending on the model being used to allocate Frames between
-    // processes. See https://crbug.com/601629.
-    if (targetWindow->isLocalDOMWindow() && isOriginAccessibleFromDOMWindow(targetFrameOrigin, accessingWindow))
+    if (canAccessFrameInternal(accessingWindow, targetFrameOrigin, targetWindow))
         return true;
 
-    if (reportingOption == ReportSecurityError && targetWindow)
+    if (accessingWindow && targetWindow && reportingOption == ReportSecurityError)
         accessingWindow->printErrorMessage(targetWindow->crossDomainAccessErrorMessage(accessingWindow));
     return false;
 }
 
+} // namespace
+
 bool BindingSecurity::shouldAllowAccessTo(v8::Isolate* isolate, const LocalDOMWindow* accessingWindow, const DOMWindow* target, ExceptionState& exceptionState)
 {
     ASSERT(target);
     const Frame* frame = target->frame();
     if (!frame || !frame->securityContext())
         return false;
-    return canAccessFrame(isolate, accessingWindow, frame->securityContext()->getSecurityOrigin(), target, exceptionState);
+    return canAccessFrame(accessingWindow, frame->securityContext()->getSecurityOrigin(), target, exceptionState);
 }
 
 bool BindingSecurity::shouldAllowAccessTo(v8::Isolate* isolate, const LocalDOMWindow* accessingWindow, const DOMWindow* target, SecurityReportingOption reportingOption)
@@ -94,7 +106,7 @@
     const Frame* frame = target->frame();
     if (!frame || !frame->securityContext())
         return false;
-    return canAccessFrame(isolate, accessingWindow, frame->securityContext()->getSecurityOrigin(), target, reportingOption);
+    return canAccessFrame(accessingWindow, frame->securityContext()->getSecurityOrigin(), target, reportingOption);
 }
 
 bool BindingSecurity::shouldAllowAccessTo(v8::Isolate* isolate, const LocalDOMWindow* accessingWindow, const EventTarget* target, ExceptionState& exceptionState)
@@ -110,7 +122,7 @@
     const Frame* frame = window->frame();
     if (!frame || !frame->securityContext())
         return false;
-    return canAccessFrame(isolate, accessingWindow, frame->securityContext()->getSecurityOrigin(), window, exceptionState);
+    return canAccessFrame(accessingWindow, frame->securityContext()->getSecurityOrigin(), window, exceptionState);
 }
 
 bool BindingSecurity::shouldAllowAccessTo(v8::Isolate* isolate, const LocalDOMWindow* accessingWindow, const Location* target, ExceptionState& exceptionState)
@@ -119,7 +131,7 @@
     const Frame* frame = target->frame();
     if (!frame || !frame->securityContext())
         return false;
-    return canAccessFrame(isolate, accessingWindow, frame->securityContext()->getSecurityOrigin(), frame->domWindow(), exceptionState);
+    return canAccessFrame(accessingWindow, frame->securityContext()->getSecurityOrigin(), frame->domWindow(), exceptionState);
 }
 
 bool BindingSecurity::shouldAllowAccessTo(v8::Isolate* isolate, const LocalDOMWindow* accessingWindow, const Location* target, SecurityReportingOption reportingOption)
@@ -128,7 +140,7 @@
     const Frame* frame = target->frame();
     if (!frame || !frame->securityContext())
         return false;
-    return canAccessFrame(isolate, accessingWindow, frame->securityContext()->getSecurityOrigin(), frame->domWindow(), reportingOption);
+    return canAccessFrame(accessingWindow, frame->securityContext()->getSecurityOrigin(), frame->domWindow(), reportingOption);
 }
 
 bool BindingSecurity::shouldAllowAccessTo(v8::Isolate* isolate, v8::Local<v8::Context> context, const ExecutionContext* executionContext, const MainThreadWorkletGlobalScope* workletGlobalScope, SecurityReportingOption reportingOption)
@@ -145,7 +157,7 @@
     if (!workletGlobalScopeFrame || !workletGlobalScopeFrame->securityContext())
         return false;
 
-    return domWindow && canAccessFrame(isolate, toLocalDOMWindow(domWindow), workletGlobalScopeFrame->securityContext()->getSecurityOrigin(), workletGlobalScopeFrame->domWindow(), reportingOption);
+    return domWindow && canAccessFrame(toLocalDOMWindow(domWindow), workletGlobalScopeFrame->securityContext()->getSecurityOrigin(), workletGlobalScopeFrame->domWindow(), reportingOption);
 }
 
 bool BindingSecurity::shouldAllowAccessTo(v8::Isolate* isolate, v8::Local<v8::Context> calling, v8::Local<v8::Context> target, SecurityReportingOption reportingOption)
@@ -170,21 +182,21 @@
 {
     if (!target)
         return false;
-    return canAccessFrame(isolate, accessingWindow, target->document().getSecurityOrigin(), target->document().domWindow(), exceptionState);
+    return canAccessFrame(accessingWindow, target->document().getSecurityOrigin(), target->document().domWindow(), exceptionState);
 }
 
 bool BindingSecurity::shouldAllowAccessTo(v8::Isolate* isolate, const LocalDOMWindow* accessingWindow, const Node* target, SecurityReportingOption reportingOption)
 {
     if (!target)
         return false;
-    return canAccessFrame(isolate, accessingWindow, target->document().getSecurityOrigin(), target->document().domWindow(), reportingOption);
+    return canAccessFrame(accessingWindow, target->document().getSecurityOrigin(), target->document().domWindow(), reportingOption);
 }
 
 bool BindingSecurity::shouldAllowAccessToFrame(v8::Isolate* isolate, const LocalDOMWindow* accessingWindow, const Frame* target, SecurityReportingOption reportingOption)
 {
     if (!target || !target->securityContext())
         return false;
-    return canAccessFrame(isolate, accessingWindow, target->securityContext()->getSecurityOrigin(), target->domWindow(), reportingOption);
+    return canAccessFrame(accessingWindow, target->securityContext()->getSecurityOrigin(), target->domWindow(), reportingOption);
 }
 
 } // namespace blink
diff --git a/third_party/WebKit/Source/bindings/core/v8/BindingSecurity.h b/third_party/WebKit/Source/bindings/core/v8/BindingSecurity.h
index 153818bb..b0746e1 100644
--- a/third_party/WebKit/Source/bindings/core/v8/BindingSecurity.h
+++ b/third_party/WebKit/Source/bindings/core/v8/BindingSecurity.h
@@ -52,6 +52,9 @@
     ReportSecurityError,
 };
 
+// TODO(yukishiino): Remove the first argument of v8::Isolate*.  These functions
+// just check if it's accessible or not, and never create anything.  So there is
+// no need to pass the isolate.
 class CORE_EXPORT BindingSecurity {
     STATIC_ONLY(BindingSecurity);
 public:
diff --git a/third_party/WebKit/Source/bindings/templates/interface_base.cpp b/third_party/WebKit/Source/bindings/templates/interface_base.cpp
index 1ea037f..e4fe0cd 100644
--- a/third_party/WebKit/Source/bindings/templates/interface_base.cpp
+++ b/third_party/WebKit/Source/bindings/templates/interface_base.cpp
@@ -88,19 +88,7 @@
     if (window.IsEmpty())
         return false; // the frame is gone.
 
-    DOMWindow* targetWindow = V8Window::toImpl(window);
-    ASSERT(targetWindow);
-    if (!targetWindow->isLocalDOMWindow())
-        return false;
-
-    LocalFrame* targetFrame = toLocalDOMWindow(targetWindow)->frame();
-    if (!targetFrame)
-        return false;
-
-    // Notify the loader's client if the initial document has been accessed.
-    if (targetFrame->loader().stateMachine()->isDisplayingInitialEmptyDocument())
-        targetFrame->loader().didAccessInitialDocument();
-
+    const DOMWindow* targetWindow = V8Window::toImpl(window);
     return BindingSecurity::shouldAllowAccessTo(isolate, toLocalDOMWindow(toDOMWindow(accessingContext)), targetWindow, DoNotReportSecurityError);
     {% else %}{# if interface_name == 'Window' #}
     {# Not 'Window' means it\'s Location. #}