Move GetDocument method from WebFrame to WebLocalFrame.

extensions/renderer/script_context.cc

Index: extensions/renderer/script_context.cc
diff --git a/extensions/renderer/script_context.cc b/extensions/renderer/script_context.cc
index 197e6c0c764c6ee96c514d876b832a2dea46894d..9a60177ad76ebe841bc8da9c7331b4081b9d14e4 100644
--- a/extensions/renderer/script_context.cc
+++ b/extensions/renderer/script_context.cc
@@ -300,7 +300,7 @@ GURL ScriptContext::GetAccessCheckedFrameURL(
 }
 
 // static
-GURL ScriptContext::GetEffectiveDocumentURL(const blink::WebLocalFrame* frame,
+GURL ScriptContext::GetEffectiveDocumentURL(blink::WebLocalFrame* frame,
                                             const GURL& document_url,
                                             bool match_about_blank) {
   // Common scenario. If |match_about_blank| is false (as is the case in most
@@ -312,7 +312,8 @@ GURL ScriptContext::GetEffectiveDocumentURL(const blink::WebLocalFrame* frame,
   // Non-sandboxed about:blank and about:srcdoc pages inherit their security
   // origin from their parent frame/window. So, traverse the frame/window
   // hierarchy to find the closest non-about:-page and return its URL.
-  const blink::WebFrame* parent = frame;
+  blink::WebFrame* parent = frame;
+  blink::WebDocument parent_document;
   do {
     if (parent->Parent())
       parent = parent->Parent();
@@ -320,12 +321,15 @@ GURL ScriptContext::GetEffectiveDocumentURL(const blink::WebLocalFrame* frame,
       parent = parent->Opener();
     else
       parent = nullptr;
-  } while (parent && !parent->GetDocument().IsNull() &&
-           GURL(parent->GetDocument().Url()).SchemeIs(url::kAboutScheme));
 
-  if (parent && !parent->GetDocument().IsNull()) {
+    parent_document = parent && parent->IsWebLocalFrame()

[Devlin, 2017/06/21 00:57:55]

This actually seems like it would be a behavior change for the case of an OOPIF
with an accessible parent document, right?

[Łukasz Anforowicz, 2017/06/21 16:54:35]

I might be missing something, but I still don't think this CL changes the
behavior here.

1. Before and after this CL the |frame| argument is a *local* frame.

2. Before and after this CL the code walks the chain of *local* parents/openers
until it finds a non-kAboutScheme URL.  Before this CL, only local
parents/openers were considered because of |!parent->GetDocument().IsNull()|
check.  Notice how before this CL WebRemoteFrameImpl::GetDocument returns a null
document and how after this CL there is no WebRemoteFrameImpl::GetDocument (to
some extent the parent_document assignment in the loop here "inlines" the old
WebRemoteFrameImpl::GetDocument's behavior).

Walking only *local* frames makes sense here - we want to find the "real"
security origin of a frame (i.e. if foo.com opened about:blank, then we want to
find foo.com) and so we don't have to consider *remote* frames (because remote
frames are always cross-site).

Does that make sense?

[Devlin, 2017/06/21 19:24:50]

Ah, thanks for the explanation!  This is fine then. :)  I missed that
WebRemoteFrameImpl returned an empty document.

+                          ? parent->ToWebLocalFrame()->GetDocument()
+                          : blink::WebDocument();
+  } while (!parent_document.IsNull() &&
+           GURL(parent_document.Url()).SchemeIs(url::kAboutScheme));
+
+  if (!parent_document.IsNull()) {
     // Only return the parent URL if the frame can access it.
-    const blink::WebDocument& parent_document = parent->GetDocument();
     if (frame->GetDocument().GetSecurityOrigin().CanAccess(
             parent_document.GetSecurityOrigin())) {
       return parent_document.Url();