bindings: Refactors BindingSecurity::shouldAllowAccessToXXX.

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

Index: third_party/WebKit/Source/bindings/core/v8/BindingSecurity.cpp
diff --git a/third_party/WebKit/Source/bindings/core/v8/BindingSecurity.cpp b/third_party/WebKit/Source/bindings/core/v8/BindingSecurity.cpp
index 9654ddf31bceffe9a716bfdc9ef786247a84d733..f142ac0f1e61976054413cbe94aaa69ab5a8b8ad 100644
--- a/third_party/WebKit/Source/bindings/core/v8/BindingSecurity.cpp
+++ b/third_party/WebKit/Source/bindings/core/v8/BindingSecurity.cpp
@@ -41,12 +41,12 @@
 
 namespace blink {
 
-static bool isOriginAccessibleFromDOMWindow(SecurityOrigin* targetOrigin, LocalDOMWindow* accessingWindow)
+static bool isOriginAccessibleFromDOMWindow(const SecurityOrigin* targetOrigin, const LocalDOMWindow* accessingWindow)
 {
     return accessingWindow && accessingWindow->document()->securityOrigin()->canAccessCheckSuborigins(targetOrigin);
 }
 
-static bool canAccessFrame(v8::Isolate* isolate, LocalDOMWindow* accessingWindow, SecurityOrigin* targetFrameOrigin, DOMWindow* targetWindow, ExceptionState& exceptionState)
+static bool canAccessFrame(v8::Isolate* isolate, const LocalDOMWindow* accessingWindow, const SecurityOrigin* targetFrameOrigin, const DOMWindow* targetWindow, ExceptionState& exceptionState)
 {
     if (isOriginAccessibleFromDOMWindow(targetFrameOrigin, accessingWindow))
         return true;
@@ -56,7 +56,7 @@ static bool canAccessFrame(v8::Isolate* isolate, LocalDOMWindow* accessingWindow
     return false;
 }
 
-static bool canAccessFrame(v8::Isolate* isolate, LocalDOMWindow* accessingWindow, SecurityOrigin* targetFrameOrigin, DOMWindow* targetWindow, SecurityReportingOption reportingOption = ReportSecurityError)
+static bool canAccessFrame(v8::Isolate* isolate, const LocalDOMWindow* accessingWindow, SecurityOrigin* targetFrameOrigin, const DOMWindow* targetWindow, SecurityReportingOption reportingOption = ReportSecurityError)
 {
     if (isOriginAccessibleFromDOMWindow(targetFrameOrigin, accessingWindow))
         return true;
@@ -66,28 +66,77 @@ static bool canAccessFrame(v8::Isolate* isolate, LocalDOMWindow* accessingWindow
     return false;
 }
 
-bool BindingSecurity::shouldAllowAccessToFrame(v8::Isolate* isolate, LocalDOMWindow* accessingWindow, Frame* target, SecurityReportingOption reportingOption)
+bool BindingSecurity::shouldAllowAccessTo(v8::Isolate* isolate, const LocalDOMWindow* accessingWindow, const DOMWindow* target, ExceptionState& exceptionState)
 {
-    if (!target || !target->securityContext())
+    ASSERT(target);
+    const Frame* frame = target->frame();
+    if (!frame || !frame->securityContext())
         return false;
-    return canAccessFrame(isolate, accessingWindow, target->securityContext()->securityOrigin(), target->domWindow(), reportingOption);
+    return canAccessFrame(isolate, accessingWindow, frame->securityContext()->securityOrigin(), target, exceptionState);

[haraken, 2015/11/16 11:34:20]

Can we add ASSERT(target == target->frame()->domWindow()) to canAccessFrame()?

[Yuki, 2015/11/20 12:27:52]

Done.

 }
 
-bool BindingSecurity::shouldAllowAccessToFrame(v8::Isolate* isolate, LocalDOMWindow* accessingWindow, Frame* target, ExceptionState& exceptionState)
+bool BindingSecurity::shouldAllowAccessTo(v8::Isolate* isolate, const LocalDOMWindow* accessingWindow, const DOMWindow* target, SecurityReportingOption reportingOption)
 {
-    if (!target || !target->securityContext())
+    ASSERT(target);
+    const Frame* frame = target->frame();
+    if (!frame || !frame->securityContext())
+        return false;
+    return canAccessFrame(isolate, accessingWindow, frame->securityContext()->securityOrigin(), 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()->securityOrigin(), 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, target->securityContext()->securityOrigin(), target->domWindow(), exceptionState);
+    return canAccessFrame(isolate, accessingWindow, frame->securityContext()->securityOrigin(), frame->domWindow(), exceptionState);
 }
 
-bool BindingSecurity::shouldAllowAccessToNode(v8::Isolate* isolate, LocalDOMWindow* accessingWindow, Node* target, SecurityReportingOption reportingOption)
+bool BindingSecurity::shouldAllowAccessTo(v8::Isolate* isolate, const LocalDOMWindow* accessingWindow, const Location* target, SecurityReportingOption reportingOption)
 {
-    return target && canAccessFrame(isolate, accessingWindow, target->document().securityOrigin(), target->document().domWindow(), reportingOption);
+    ASSERT(target);
+    const Frame* frame = target->frame();
+    if (!frame || !frame->securityContext())
+        return false;
+    return canAccessFrame(isolate, accessingWindow, frame->securityContext()->securityOrigin(), frame->domWindow(), reportingOption);
+}
+
+bool BindingSecurity::shouldAllowAccessTo(v8::Isolate* isolate, const LocalDOMWindow* accessingWindow, const Node* target, ExceptionState& exceptionState)
+{
+    if (!target)
+        return false;
+    return canAccessFrame(isolate, accessingWindow, target->document().securityOrigin(), 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().securityOrigin(), target->document().domWindow(), reportingOption);
 }
 
-bool BindingSecurity::shouldAllowAccessToNode(v8::Isolate* isolate, LocalDOMWindow* accessingWindow, Node* target, ExceptionState& exceptionState)
+bool BindingSecurity::shouldAllowAccessToFrame(v8::Isolate* isolate, const LocalDOMWindow* accessingWindow, const Frame* target, SecurityReportingOption reportingOption)
 {
-    return target && canAccessFrame(isolate, accessingWindow, target->document().securityOrigin(), target->document().domWindow(), exceptionState);
+    if (!target || !target->securityContext())

[dcheng, 2015/11/17 01:56:59]

It seems like we make these checks for every Frame*. Can we move these checks
into canAccessFrame() and just pass |frame| directly?

[Yuki, 2015/11/20 12:27:52]

Hmm, we can do it for DOMWindow* and EventTarget*.

For Location, we're passing target->frame()->domWindow() as the target window to
canAccessFrame().  So we need to check if a frame exists or not beforehand.

For Node, we're not using a frame to extract the security origin and the target
window.

I think it's okay as is.  WDYT?

+        return false;
+    return canAccessFrame(isolate, accessingWindow, target->securityContext()->securityOrigin(), target->domWindow(), reportingOption);
 }
 
 } // namespace blink