v8binding: Minor fix of BindingSecurity

It seemed like that there were cases that BindingSecurity didn't
throw an exception although BindingSecurity::ShouldAllowAccessToXxx
returned false.

This patch makes BindingSecurity always throw an exception when it
returns false.  Also, BindingSecurity reports an error (print an
error message) as best effort.

This patch also supports the case that
remote_object->CreationContext() returns the empty handle.

Change-Id: I155f975af3c5d93f27ccb7e0ca0eb8d5bdc8d403
Reviewed-on: https://chromium-review.googlesource.com/c/1341785Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Yuki Shiino <yukishiino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609624}

third_party/blink/renderer/bindings/core/v8/binding_security.cc

@@ -60,6 +60,35 @@ void BindingSecurity::Init() {
 
 namespace {
 
+void ReportOrThrowSecurityError(const LocalDOMWindow* accessing_window,
+                                const DOMWindow* target_window,
+                                ExceptionState& exception_state) {
+  if (target_window) {
+    exception_state.ThrowSecurityError(
+        target_window->SanitizedCrossDomainAccessErrorMessage(accessing_window),
+        target_window->CrossDomainAccessErrorMessage(accessing_window));
+  } else {
+    exception_state.ThrowSecurityError("Cross origin access was denied.");
+  }
+}
+
+void ReportOrThrowSecurityError(
+    const LocalDOMWindow* accessing_window,
+    const DOMWindow* target_window,
+    BindingSecurity::ErrorReportOption reporting_option) {
+  if (reporting_option == BindingSecurity::ErrorReportOption::kDoNotReport)
+    return;
+
+  if (accessing_window && target_window) {
+    accessing_window->PrintErrorMessage(
+        target_window->CrossDomainAccessErrorMessage(accessing_window));
+  } else if (accessing_window) {
+    accessing_window->PrintErrorMessage("Cross origin access was denied.");
+  } else {
+    // Nowhere to report the error.
+  }
+}
+
 bool CanAccessWindowInternal(const LocalDOMWindow* accessing_window,
                              const DOMWindow* target_window) {
   SECURITY_CHECK(!(target_window && target_window->GetFrame()) ||
@@ -101,29 +130,14 @@ bool CanAccessWindowInternal(const LocalDOMWindow* accessing_window,
   return true;
 }
 
+template <typename ExceptionStateOrErrorReportOption>
 bool CanAccessWindow(const LocalDOMWindow* accessing_window,
                      const DOMWindow* target_window,
-                     ExceptionState& exception_state) {
-  if (CanAccessWindowInternal(accessing_window, target_window))
-    return true;
-
-  if (target_window)
-    exception_state.ThrowSecurityError(
-        target_window->SanitizedCrossDomainAccessErrorMessage(accessing_window),
-        target_window->CrossDomainAccessErrorMessage(accessing_window));
-  return false;
-}
-
-bool CanAccessWindow(const LocalDOMWindow* accessing_window,
-                     const DOMWindow* target_window,
-                     BindingSecurity::ErrorReportOption reporting_option) {
+                     ExceptionStateOrErrorReportOption& error_report) {
   if (CanAccessWindowInternal(accessing_window, target_window))
     return true;
 
-  if (accessing_window && target_window &&
-      reporting_option == BindingSecurity::ErrorReportOption::kReport)
-    accessing_window->PrintErrorMessage(
-        target_window->CrossDomainAccessErrorMessage(accessing_window));
+  ReportOrThrowSecurityError(accessing_window, target_window, error_report);
   return false;
 }
 
@@ -310,6 +324,14 @@ bool ShouldAllowAccessToV8ContextInternal(
   // Workers and worklets do not support multiple contexts, so both of
   // |accessing_context| and |target_context| must be windows at this point.
 
+  // remote_object->CreationContext() returns the empty handle. Remote contexts
+  // are unconditionally treated as cross origin.
+  if (target_context.IsEmpty()) {
+    ReportOrThrowSecurityError(ToLocalDOMWindow(accessing_context), nullptr,
+                               error_report);
+    return false;
+  }
+
   LocalFrame* target_frame = ToLocalFrameIfNotDetached(target_context);
   // TODO(dcheng): Why doesn't this code just use DOMWindows throughout? Can't
   // we just always use ToLocalDOMWindow(context)?