Throw an exception when denying access to 'Frame's 'location' setter.

Source/bindings/v8/BindingSecurity.cpp

Index: Source/bindings/v8/BindingSecurity.cpp
diff --git a/Source/bindings/v8/BindingSecurity.cpp b/Source/bindings/v8/BindingSecurity.cpp
index ab23d1ff06709172f59e474a3e78a7ec55fee759..4bcb0f23eb9fef37e709462c5ec56352a341eb6d 100644
--- a/Source/bindings/v8/BindingSecurity.cpp
+++ b/Source/bindings/v8/BindingSecurity.cpp
@@ -34,7 +34,6 @@
 #include "bindings/v8/V8Binding.h"
 #include "core/dom/Document.h"
 #include "core/html/HTMLFrameElementBase.h"
-#include "core/html/parser/HTMLParserIdioms.h"
 #include "core/page/DOMWindow.h"
 #include "core/page/Frame.h"
 #include "core/page/Settings.h"
@@ -42,7 +41,7 @@
 
 namespace WebCore {
 
-static bool canAccessDocument(Document* targetDocument, SecurityReportingOption reportingOption = ReportSecurityError)
+static bool isDocumentAccessibleFromActiveDOMWindow(Document* targetDocument)
 {
     if (!targetDocument)
         return false;
@@ -54,9 +53,26 @@ static bool canAccessDocument(Document* targetDocument, SecurityReportingOption
     if (active->document()->securityOrigin()->canAccess(targetDocument->securityOrigin()))
         return true;
 
+    return false;
+}
+
+static bool canAccessDocument(Document* targetDocument, ExceptionState& es)

[abarth-chromium, 2013/08/12 19:57:03]

What's the point of having this be a separate function?  There's exactly one
caller.

[Mike West, 2013/08/13 08:30:51]

This is the first patch that throws an exception on access violation, but my
goal is to move all the current usage over to the exception-throwing version.
So, there's exactly one caller _for now_. :)

+{
+    if (isDocumentAccessibleFromActiveDOMWindow(targetDocument))
+        return true;
+
+    es.throwDOMException(SecurityError, targetDocument->domWindow()->crossDomainAccessErrorMessage(activeDOMWindow()));

[abarth-chromium, 2013/08/12 19:58:26]

Wait a minute.  not lgtm.  This leaks the current URL of the frame, right?

+    return false;
+}
+
+static bool canAccessDocument(Document* targetDocument, SecurityReportingOption reportingOption = ReportSecurityError)
+{
+    if (isDocumentAccessibleFromActiveDOMWindow(targetDocument))
+        return true;
+
     if (reportingOption == ReportSecurityError) {
         if (Frame* frame = targetDocument->frame())
-            frame->domWindow()->printErrorMessage(targetDocument->domWindow()->crossDomainAccessErrorMessage(active));
+            frame->domWindow()->printErrorMessage(targetDocument->domWindow()->crossDomainAccessErrorMessage(activeDOMWindow()));

[abarth-chromium, 2013/08/12 19:57:03]

It's kine of lame that we call activeDOMWindow twice now.

[Mike West, 2013/08/13 08:30:51]

Good point. Fixed.

     }
 
     return false;
@@ -67,14 +83,14 @@ bool BindingSecurity::shouldAllowAccessToFrame(Frame* target, SecurityReportingO
     return target && canAccessDocument(target->document(), reportingOption);
 }
 
-bool BindingSecurity::shouldAllowAccessToNode(Node* target)
+bool BindingSecurity::shouldAllowAccessToFrame(Frame* target, ExceptionState& es)
 {
-    return target && canAccessDocument(target->document());
+    return target && canAccessDocument(target->document(), es);
 }
 
-bool BindingSecurity::allowSettingFrameSrcToJavascriptUrl(HTMLFrameElementBase* frame, const String& value)
+bool BindingSecurity::shouldAllowAccessToNode(Node* target)
 {
-    return !protocolIsJavaScript(stripLeadingAndTrailingHTMLSpaces(value)) || canAccessDocument(frame->contentDocument());
+    return target && canAccessDocument(target->document());
 }
 
 }