Make error messages for cross-domain access OOPIF-friendly.

Source/bindings/core/v8/BindingSecurity.cpp

Index: Source/bindings/core/v8/BindingSecurity.cpp
diff --git a/Source/bindings/core/v8/BindingSecurity.cpp b/Source/bindings/core/v8/BindingSecurity.cpp
index 3f9c7eeae381754d04ed17c45ca0fd4befb6945d..6cf718f525d152abe43584f5d8645001c4fc288e 100644
--- a/Source/bindings/core/v8/BindingSecurity.cpp
+++ b/Source/bindings/core/v8/BindingSecurity.cpp
@@ -41,60 +41,50 @@
 
 namespace blink {
 
-static bool isDocumentAccessibleFromDOMWindow(Document* targetDocument, LocalDOMWindow* callingWindow)
+static bool isOriginAccessibleFromDOMWindow(SecurityOrigin* targetOrigin, LocalDOMWindow* callingWindow)
 {
-    if (!targetDocument)
-        return false;
-
-    if (!callingWindow)
-        return false;
-
-    if (callingWindow->document()->securityOrigin()->canAccess(targetDocument->securityOrigin()))
-        return true;
-
-    return false;
+    return callingWindow && callingWindow->document()->securityOrigin()->canAccess(targetOrigin);
 }
 
-static bool canAccessDocument(v8::Isolate* isolate, Document* targetDocument, ExceptionState& exceptionState)
+static bool canAccessFrame(v8::Isolate* isolate, SecurityOrigin* targetFrameOrigin, DOMWindow* targetWindow, ExceptionState& exceptionState)
 {
     LocalDOMWindow* callingWindow = callingDOMWindow(isolate);
-    if (isDocumentAccessibleFromDOMWindow(targetDocument, callingWindow))
+    if (isOriginAccessibleFromDOMWindow(targetFrameOrigin, callingWindow))
         return true;
 
-    if (targetDocument->domWindow())
-        exceptionState.throwSecurityError(targetDocument->domWindow()->sanitizedCrossDomainAccessErrorMessage(callingWindow), targetDocument->domWindow()->crossDomainAccessErrorMessage(callingWindow));
+    if (targetWindow)
+        exceptionState.throwSecurityError(targetWindow->sanitizedCrossDomainAccessErrorMessage(callingWindow), targetWindow->crossDomainAccessErrorMessage(callingWindow));
     return false;
 }
 
-static bool canAccessDocument(v8::Isolate* isolate, Document* targetDocument, SecurityReportingOption reportingOption = ReportSecurityError)
+static bool canAccessFrame(v8::Isolate* isolate, SecurityOrigin* targetFrameOrigin, DOMWindow* targetWindow, SecurityReportingOption reportingOption = ReportSecurityError)
 {
     LocalDOMWindow* callingWindow = callingDOMWindow(isolate);
-    if (isDocumentAccessibleFromDOMWindow(targetDocument, callingWindow))
+    if (isOriginAccessibleFromDOMWindow(targetFrameOrigin, callingWindow))
         return true;
 
-    if (reportingOption == ReportSecurityError && targetDocument->domWindow())
-        callingWindow->printErrorMessage(targetDocument->domWindow()->crossDomainAccessErrorMessage(callingWindow));
-
+    if (reportingOption == ReportSecurityError && targetWindow)
+        callingWindow->printErrorMessage(targetWindow->crossDomainAccessErrorMessage(callingWindow));
     return false;
 }
 
 bool BindingSecurity::shouldAllowAccessToFrame(v8::Isolate* isolate, Frame* target, SecurityReportingOption reportingOption)
 {
-    if (!target || !target->isLocalFrame())
+    if (!target || !target->securityContext())
         return false;
-    return canAccessDocument(isolate, toLocalFrame(target)->document(), reportingOption);
+    return canAccessFrame(isolate, target->securityContext()->securityOrigin(), target->domWindow(), reportingOption);
 }
 
 bool BindingSecurity::shouldAllowAccessToFrame(v8::Isolate* isolate, Frame* target, ExceptionState& exceptionState)
 {
-    if (!target || !target->isLocalFrame())
+    if (!target || !target->securityContext())
         return false;
-    return canAccessDocument(isolate, toLocalFrame(target)->document(), exceptionState);
+    return canAccessFrame(isolate, target->securityContext()->securityOrigin(), target->domWindow(), exceptionState);
 }
 
 bool BindingSecurity::shouldAllowAccessToNode(v8::Isolate* isolate, Node* target, ExceptionState& exceptionState)
 {
-    return target && canAccessDocument(isolate, &target->document(), exceptionState);
+    return target && canAccessFrame(isolate, target->document().securityOrigin(), target->document().domWindow(), exceptionState);

[dcheng, 2015/04/23 17:39:57]

Would it make sense to have canAccessFrame() take a Frame*, and then we just
pass the target frame to it? That way, we don't have to pass the
SecurityOrigin/DOMWindow together like this.

[alexmos, 2015/04/23 17:45:28]

That was the first thing I tried, and unfortunately that didn't work, as
shouldAllowAccessToNode can be called with a target that has a Document with a
null frame() (This was exercised by
http/tests/inspector/template-content-inspect-crash.html).

[dcheng, 2015/04/23 18:01:24]

Do we actually return a true value in that case? Or would it return false
similar to lines 73-74 and 80-81?

[alexmos, 2015/04/23 21:25:23]

That particular test returns false - I think it has a DocumentFragment with a
unique origin and compares it to the test page's 127.0.0.1 origin.  But I'm not
sure that will always be false -- it seems DocumentFragments would pass their
creator Document's origin, which could be anything.  

That said, I'm also not sure under which circumstances shouldAllowAccessToNode
could get called for DocumentFragments.  Here's the call stack from the
inspector test, in case it's useful: 

#1  blink::BindingSecurity::shouldAllowAccessToNode (isolate=, target=,
exceptionState=...)
    at ../../third_party/WebKit/Source/bindings/core/v8/BindingSecurity.cpp:87
#2  blink::InjectedScriptHost::nodeAsScriptValue (scriptState=, node=)
    at
../../third_party/WebKit/Source/bindings/core/v8/custom/V8InjectedScriptHostCustom.cpp:79
#3  blink::InspectableNode::get (this=, state=)
    at ../../third_party/WebKit/Source/core/inspector/InspectorDOMAgent.cpp:2160
#4  blink::V8InjectedScriptHost::inspectedObjectMethodCustom (info=...)
    at
../../third_party/WebKit/Source/bindings/core/v8/custom/V8InjectedScriptHostCustom.cpp:115
#5  blink::InjectedScriptHostV8Internal::inspectedObjectMethodCallback
(info=...)
    at gen/blink/bindings/core/v8/V8InjectedScriptHost.cpp:56
#6  v8::internal::FunctionCallbackArguments::Call (this=, 
   
f=<blink::InjectedScriptHostV8Internal::inspectedObjectMethodCallback(v8::FunctionCallbackInfo<v8::Value>
const&)>) at ../../v8/src/arguments.cc:33

 }
 
 }