Fix extension bindings injection for iframes (reland)

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 cf6432f8951ca42f6edbfbc58d6663c485318c85..e12de37d304dd1862ad92e5d31b16ee3330744e5 100644
--- a/extensions/renderer/script_context.cc
+++ b/extensions/renderer/script_context.cc
@@ -109,12 +109,24 @@ ScriptContext::ScriptContext(const v8::Local<v8::Context>& v8_context,
       effective_context_type_(effective_context_type),
       safe_builtins_(this),
       isolate_(v8_context->GetIsolate()),
-      url_(web_frame_ ? GetDataSourceURLForFrame(web_frame_) : GURL()),
       runner_(new Runner(this)) {
   VLOG(1) << "Created context:\n" << GetDebugString();
   gin::PerContextData* gin_data = gin::PerContextData::From(v8_context);
   CHECK(gin_data);
   gin_data->set_runner(runner_.get());
+  if (web_frame_) {
+    const blink::WebURL& weburl = web_frame_->document().url();
+    if (weburl.isEmpty()) {
+      blink::WebDataSource* data_source = web_frame_->provisionalDataSource()
+                                          ? web_frame_->provisionalDataSource()
+                                          : web_frame_->dataSource();

[asargent_no_longer_on_chrome, 2016/08/03 00:22:24]

Note that there's a little duplication here with the code in
GetDataSourceURLForFrame - I didn't want to just re-use GetDataSourceURLForFrame
as-is since this code runs on every frame creation and I wanted to avoid the
overhead of taking a blink::WebURL, converting it to a GURL to return from
GetDataSourceURLForFrame, and then immediately converting it back to a WebURL to
do the security origin test. I suppose I could refactor things to have a common
method that returns a blink::WebURL and then use that in both places. Let me
know what you think. 

[Devlin, 2016/08/03 16:16:49]

This also looks very similar to the new logic added in ScriptContextSet.  In
both cases we:
Get the document url
if that's empty, check the data source url
if the data source url is non-empty and the frame can access it, use the data
source url

This makes me think we should probably have a method at least to do that -
something like GetAccessCheckedFrameURL() (strawman name) that we can use in
both places.  There may also be some uses of GetDataSourceURL that should be
converted to that, but we can do that separately.

[asargent_no_longer_on_chrome, 2016/08/04 05:47:18]

Done.

+      if (data_source && web_frame_->getSecurityOrigin().canAccess(
+              blink::WebSecurityOrigin::create(data_source->request().url())))
+        url_ = GURL(data_source->request().url());
+    }
+    if (url_.is_empty())

[Devlin, 2016/08/03 16:16:49]

This can be an else, right?  Because we'll only set url_ above if weburl is
empty.

[asargent_no_longer_on_chrome, 2016/08/04 05:47:18]

Good point - I guess it doesn't help to assign to url_ from an empty weburl.
But, obsoleted by switching to a method. 

+      url_ = GURL(weburl);
+  }
 }
 
 ScriptContext::~ScriptContext() {