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

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

 /*
  * Copyright (C) 2009 Google Inc. 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
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 24 matching lines @@
 #include "core/frame/LocalDOMWindow.h"
 #include "core/frame/LocalFrame.h"
 #include "core/frame/Location.h"
 #include "core/frame/Settings.h"
 #include "core/html/HTMLFrameElementBase.h"
 #include "core/workers/MainThreadWorkletGlobalScope.h"
 #include "platform/weborigin/SecurityOrigin.h"
 
 namespace blink {
 
-static bool isOriginAccessibleFromDOMWindow(const SecurityOrigin* targetOrigin, const LocalDOMWindow* accessingWindow)
+namespace {
+
+bool canAccessFrameInternal(const LocalDOMWindow* accessingWindow, const SecurityOrigin* targetFrameOrigin, const DOMWindow* targetWindow)
 {
-    return accessingWindow && accessingWindow->document()->getSecurityOrigin()->canAccessCheckSuborigins(targetOrigin);
-}
-
-static bool canAccessFrame(v8::Isolate* isolate, const LocalDOMWindow* accessingWindow, const SecurityOrigin* targetFrameOrigin, const DOMWindow* targetWindow, ExceptionState& exceptionState)
-{
-    ASSERT_WITH_SECURITY_IMPLICATION(!(targetWindow && targetWindow->frame()) || targetWindow == targetWindow->frame()->domWindow());
+    SECURITY_DCHECK(!(targetWindow && targetWindow->frame())

[haraken, 2016/08/13 02:29:36]

I think it's worth changing this to SECURITY_CHECK (in a follow-up CL).

[Yuki, 2016/08/15 03:42:58]

Done.

+        || 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()))

[haraken, 2016/08/13 02:29:36]

!accessingWindow || !targetWindow || ! targetWindow->isLocalDOMWindow()

[Yuki, 2016/08/15 03:42:58]

I'd prefer this (old) style to yours.  My execuses are:
1)
  if (!( complex okay condition ))
    early exit (return, throw, exit, etc.);
is a common pattern (as a substitute of unless(...)).
2)
  if (x && x->foo())
is also a common short-circuit pattern.
  if (!x || !x->foo())
is a bit less easy to read (for me).

+        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)
         exceptionState.throwSecurityError(targetWindow->sanitizedCrossDomainAccessErrorMessage(accessingWindow), targetWindow->crossDomainAccessErrorMessage(accessingWindow));
     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)
 {
     ASSERT(target);
     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)
 {
     ASSERT(target);
     const DOMWindow* window = target->toDOMWindow();
     if (!window) {
         // We only need to check the access to Window objects which are
         // cross-origin accessible.  If it's not a Window, the object's
         // origin must always be the same origin (or it already leaked).
         return true;
     }
     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)
 {
     ASSERT(target);
     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)
 {
     ASSERT(target);
     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)
 {
     DCHECK(executionContext);
     DOMWindow* domWindow = toDOMWindow(context);
     if (executionContext->isMainThreadWorkletGlobalScope()) {
         Frame* callingFrame = toMainThreadWorkletGlobalScope(executionContext)->frame();
         domWindow = callingFrame ? callingFrame->domWindow() : nullptr;
     }
 
     DCHECK(workletGlobalScope);
     const Frame* workletGlobalScopeFrame = workletGlobalScope->frame();
     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)
 {
     ExecutionContext* targetExecutionContext = toExecutionContext(target);
     DCHECK(targetExecutionContext);
 
     ExecutionContext* callingExecutionContext = toExecutionContext(calling);
     DCHECK(callingExecutionContext);
 
     if (targetExecutionContext->isMainThreadWorkletGlobalScope())
         return shouldAllowAccessTo(isolate, calling, callingExecutionContext, toMainThreadWorkletGlobalScope(targetExecutionContext), DoNotReportSecurityError);
 
     if (callingExecutionContext->isMainThreadWorkletGlobalScope())
         return shouldAllowAccessTo(isolate, target, targetExecutionContext, toMainThreadWorkletGlobalScope(callingExecutionContext), DoNotReportSecurityError);
 
     DOMWindow* window = toDOMWindow(target);
     return window && shouldAllowAccessTo(isolate, toLocalDOMWindow(toDOMWindow(calling)), window, DoNotReportSecurityError);
 }
 
 bool BindingSecurity::shouldAllowAccessTo(v8::Isolate* isolate, const LocalDOMWindow* accessingWindow, const Node* target, ExceptionState& exceptionState)
 {
     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