Only do security checks on javascript: URLs for frames for loading

third_party/WebKit/Source/core/html/HTMLFrameElementBase.cpp

Index: third_party/WebKit/Source/core/html/HTMLFrameElementBase.cpp
diff --git a/third_party/WebKit/Source/core/html/HTMLFrameElementBase.cpp b/third_party/WebKit/Source/core/html/HTMLFrameElementBase.cpp
index 0c5029c6cb5d1b06cf5593ce1729964e2dd6cfc7..fcb970c22d7d9b60dca94b6d29cc5b8bfcd5ee98 100644
--- a/third_party/WebKit/Source/core/html/HTMLFrameElementBase.cpp
+++ b/third_party/WebKit/Source/core/html/HTMLFrameElementBase.cpp
@@ -50,16 +50,20 @@ HTMLFrameElementBase::HTMLFrameElementBase(const QualifiedName& tagName, Documen
 {
 }
 
-bool HTMLFrameElementBase::isURLAllowed() const
+bool HTMLFrameElementBase::isURLAllowed(ReasonForCallingURLAllowed reason) const
 {
     if (m_URL.isEmpty())
         return true;
 
     const KURL& completeURL = document().completeURL(m_URL);
 
-    if (protocolIsJavaScript(completeURL)) {
-        if (contentFrame() && !ScriptController::canAccessFromCurrentOrigin(toIsolate(&document()), contentFrame()))
-            return false;
+    if (reason == WillLoadURL && protocolIsJavaScript(completeURL)) {

[dcheng, 2016/07/27 15:15:58]

I wonder if we should just cache the result of this and have the various checks
consult the saved bit? It's not super intuitive (IMO) to callers what the right
enum value to pass should be, and I worry that if we add new calls, it'd be easy
to get it wrong.

[jochen (gone - plz use gerrit), 2016/07/27 15:19:37]

well, if you want to do layout and get it wrong, you'll crash in the
SECURITY_CHECK below.

If you want to load and use the layout check instead, we might be toast anyways,
if there is no cached value yet :-/

[dcheng, 2016/07/27 15:39:06]

I'm kind of thinking we should just reset this to false in insertedInto and
removedFrom: that would solve that issue right?

+        if (contentFrame()) {
+            v8::Isolate* isolate = toIsolate(&document());
+            SECURITY_CHECK(isolate->InContext());
+            if (!ScriptController::canAccessFromCurrentOrigin(toIsolate(&document()), contentFrame()))

[dcheng, 2016/07/27 15:15:58]

I wonder if we should remove the isolate->InContext() branch in
canAccessFromCurrentOrigin()?

[jochen (gone - plz use gerrit), 2016/07/27 15:19:37]

done

+                return false;
+        }
     }
 
     LocalFrame* parentFrame = document().frame();
@@ -71,7 +75,7 @@ bool HTMLFrameElementBase::isURLAllowed() const
 
 void HTMLFrameElementBase::openURL(bool replaceCurrentItem)
 {
-    if (!isURLAllowed())
+    if (!isURLAllowed(WillLoadURL))
         return;
 
     if (m_URL.isEmpty())