Move securityCheck out of V8WrapperInstantiationScope

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

Index: third_party/WebKit/Source/bindings/core/v8/V8DOMWrapper.cpp
diff --git a/third_party/WebKit/Source/bindings/core/v8/V8DOMWrapper.cpp b/third_party/WebKit/Source/bindings/core/v8/V8DOMWrapper.cpp
index 249080d68e506246e613dcc04512f5399433f5be..efbe7870b108a9fe920de3a5609e878926891f82 100644
--- a/third_party/WebKit/Source/bindings/core/v8/V8DOMWrapper.cpp
+++ b/third_party/WebKit/Source/bindings/core/v8/V8DOMWrapper.cpp
@@ -31,14 +31,9 @@
 #include "bindings/core/v8/V8DOMWrapper.h"
 
 #include "bindings/core/v8/V8Binding.h"
-#include "bindings/core/v8/V8Location.h"
 #include "bindings/core/v8/V8ObjectConstructor.h"
 #include "bindings/core/v8/V8PerContextData.h"
 #include "bindings/core/v8/V8PerIsolateData.h"
-#include "bindings/core/v8/V8ScriptRunner.h"
-#include "bindings/core/v8/V8Window.h"
-#include "core/dom/Document.h"
-#include "core/frame/LocalDOMWindow.h"
 
 namespace blink {
 
@@ -46,14 +41,10 @@ v8::Local<v8::Object> V8DOMWrapper::CreateWrapper(
     v8::Isolate* isolate,
     v8::Local<v8::Object> creation_context,
     const WrapperTypeInfo* type) {
-  ASSERT(!type->Equals(&V8Window::wrapperTypeInfo));
-  // According to
-  // https://html.spec.whatwg.org/multipage/browsers.html#security-location,
-  // cross-origin script access to a few properties of Location is allowed.
-  // Location already implements the necessary security checks.
-  bool with_security_check = !type->Equals(&V8Location::wrapperTypeInfo);
-  V8WrapperInstantiationScope scope(creation_context, isolate,
-                                    with_security_check);
+  V8WrapperInstantiationScope scope(creation_context, isolate, type);
+  if (scope.AccessCheckFailed()) {

[Yuki, 2017/04/20 09:32:47]

It's okay that this CL is not focusing on this issue, but this is still
problematic.  It's wrong to continue to run having an exception thrown. 
Ideally, the DCHECK in ToV8() should be fixed instead.

At least, we should have a TODO comment here.

[adithyas, 2017/04/20 14:51:51]

Added in a TODO, I'll try fixing this in a follow up CL.

+    return v8::Local<v8::Object>();
+  }
 
   V8PerContextData* per_context_data =
       V8PerContextData::From(scope.GetContext());
@@ -106,54 +97,4 @@ bool V8DOMWrapper::HasInternalFieldsSet(v8::Local<v8::Value> value) {
          untrusted_wrapper_type_info->gin_embedder == gin::kEmbedderBlink;
 }
 
-void V8WrapperInstantiationScope::SecurityCheck(
-    v8::Isolate* isolate,
-    v8::Local<v8::Context> context_for_wrapper) {
-  if (context_.IsEmpty())
-    return;
-  // If the context is different, we need to make sure that the current
-  // context has access to the creation context.
-  LocalFrame* frame = ToLocalFrameIfNotDetached(context_for_wrapper);
-  if (!frame) {
-    // Sandbox detached frames - they can't create cross origin objects.
-    LocalDOMWindow* calling_window = CurrentDOMWindow(isolate);
-    LocalDOMWindow* target_window = ToLocalDOMWindow(context_for_wrapper);
-    // TODO(jochen): Currently, Location is the only object for which we can
-    // reach this code path. Should be generalized.
-    ExceptionState exception_state(
-        isolate, ExceptionState::kConstructionContext, "Location");
-    if (BindingSecurity::ShouldAllowAccessToDetachedWindow(
-            calling_window, target_window, exception_state))
-      return;
-
-    CHECK_EQ(kSecurityError, exception_state.Code());
-    return;
-  }
-  const DOMWrapperWorld& current_world = DOMWrapperWorld::World(context_);
-  RELEASE_ASSERT(current_world.GetWorldId() ==
-                 DOMWrapperWorld::World(context_for_wrapper).GetWorldId());
-  // TODO(jochen): Add the interface name here once this is generalized.
-  ExceptionState exception_state(isolate, ExceptionState::kConstructionContext,
-                                 nullptr);
-  if (current_world.IsMainWorld() &&
-      !BindingSecurity::ShouldAllowAccessToFrame(CurrentDOMWindow(isolate),
-                                                 frame, exception_state)) {
-    CHECK_EQ(kSecurityError, exception_state.Code());
-    return;
-  }
-}
-
-void V8WrapperInstantiationScope::ConvertException() {
-  v8::Isolate* isolate = context_->GetIsolate();
-  // TODO(jochen): Currently, Location is the only object for which we can reach
-  // this code path. Should be generalized.
-  ExceptionState exception_state(isolate, ExceptionState::kConstructionContext,
-                                 "Location");
-  LocalDOMWindow* calling_window = CurrentDOMWindow(isolate);
-  LocalDOMWindow* target_window = ToLocalDOMWindow(context_);
-  exception_state.ThrowSecurityError(
-      target_window->SanitizedCrossDomainAccessErrorMessage(calling_window),
-      target_window->CrossDomainAccessErrorMessage(calling_window));
-}
-
 }  // namespace blink