Plumbing blink::Interface{Provider|Registry} through WebLocalFrame's constructor.

Plumbing blink::Interface{Provider|Registry} through WebLocalFrame's constructor.

This CL helps remove WebFrameClient::interfaceProvider methods which
would have clashed (after the Great Blink Rename) with either
InterfaceProvider type and/or RenderFrameImpl::GetInterfaceProvider
method.  Same for interfaceRegistry method.

BUG=682560
Review-Url: https://codereview.chromium.org/2651583007
Cr-Commit-Position: refs/heads/master@{#445781}
Committed: https://chromium.googlesource.com/chromium/src/+/2d6720b079b98fb9104c7274ef033a08d9e68efc

[Łukasz Anforowicz, 2017-01-23 23:16:54]

The CQ bit was checked by lukasza@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-23 23:17:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Łukasz Anforowicz, 2017-01-23 23:38:15]

lukasza@chromium.org changed reviewers:
+ dcheng@chromium.org

[Łukasz Anforowicz, 2017-01-23 23:38:16]

Daniel, do you want to take a look how your proposal from
https://crbug.com/682560#c1 has turned out?  I think this looks okay, but I've
also tried going with https://crbug.com/682560#c2 (at
https://crrev.com/2650913002) and I think I like it a little bit better.  WDYT?

https://codereview.chromium.org/2651583007/diff/20001/components/plugins/rend...
File components/plugins/renderer/webview_plugin.cc (right):

https://codereview.chromium.org/2651583007/diff/20001/components/plugins/rend...
components/plugins/renderer/webview_plugin.cc:256:
blink::WebTreeScopeType::Document, this, nullptr, nullptr);
WebViewPlugin::WebViewHelper doesn't override interface{Provider|Registry}, so
it just inherits the default implementation from blink::WebFrameClient which
used to return nullptr.

https://codereview.chromium.org/2651583007/diff/20001/content/renderer/render...
File content/renderer/render_frame_impl.cc (right):

https://codereview.chromium.org/2651583007/diff/20001/content/renderer/render...
content/renderer/render_frame_impl.cc:941:
render_frame->blink_interface_registry_.get(), opener);
The pattern I tried to follow in this CL is that wherever we pass
WebFrameClient*, we would also start passing blink::InterfaceProvider* and
blink::InterfaceRegistry*.  Saying this outloud makes it seem a bit ugly (if
these 2 things are always passed together with the client, then shouldn't they
be part of the client?).

https://codereview.chromium.org/2651583007/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/WebEmbeddedWorkerImpl.cpp (right):

https://codereview.chromium.org/2651583007/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/WebEmbeddedWorkerImpl.cpp:303:
WebTreeScopeType::Document, this, nullptr, nullptr));
blink::WebEmbeddedWorkerImpl doesn't override interface{Provider|Registry}, so
it just inherits the default implementation from blink::WebFrameClient which
used to return nullptr.

https://codereview.chromium.org/2651583007/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/WebSharedWorkerImpl.cpp (right):

https://codereview.chromium.org/2651583007/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/WebSharedWorkerImpl.cpp:136:
Platform::current()->interfaceProvider(), nullptr));
blink::WebEmbeddedWorkerImpl used to override interfaceProvider method - we just
inline the body of the old method (getting the interface provider from the
current platform) above.

blink::WebEmbeddedWorkerImpl did not override interfaceRegistry method - it just
inherited the default implementation from blink::WebFrameClient which used to
return nullptr.

https://codereview.chromium.org/2651583007/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/FrameTestHelpers.h (right):

https://codereview.chromium.org/2651583007/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/FrameTestHelpers.h:253: virtual
blink::InterfaceProvider* interfaceProvider() { return nullptr; }
This is needed for ScreenWakeLockTest.

[commit-bot: I haz the power, 2017-01-23 23:42:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-23 23:42:14]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)

[Łukasz Anforowicz, 2017-01-23 23:58:46]

The CQ bit was checked by lukasza@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-23 23:59:25]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-24 00:25:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-24 00:25:52]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Łukasz Anforowicz, 2017-01-24 00:50:36]

The CQ bit was checked by lukasza@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-24 00:51:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-24 05:20:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-24 05:20:11]

Dry run: This issue passed the CQ dry run.

[dcheng, 2017-01-24 09:26:41]

LGTM

It's a little ugly, but I think what we ultimately want anyway is some sort of
params struct. And I think it's nicer than WebInterface{Provider,Registry} =)

[Łukasz Anforowicz, 2017-01-24 13:35:09]

The CQ bit was checked by lukasza@chromium.org

[commit-bot: I haz the power, 2017-01-24 13:35:23]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-24 13:41:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-24 13:41:58]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[nasko, 2017-01-24 14:56:14]

nasko@chromium.org changed reviewers:
+ nasko@chromium.org

[nasko, 2017-01-24 14:56:14]

content/ LGTM

[Łukasz Anforowicz, 2017-01-24 17:07:58]

Description was changed from

==========
Plumbing blink::Interface{Provider|Registry} through WebLocalFrame's
constructor.

BUG=682560
==========

to

==========
Plumbing blink::Interface{Provider|Registry} through WebLocalFrame's
constructor.

This CL helps remove WebFrameClient::interfaceProvider methods which
would have clashed (after the Great Blink Rename) with either
InterfaceProvider type and/or RenderFrameImpl::GetInterfaceProvider
method.  Same for interfaceRegistry method.

BUG=682560
==========

[Łukasz Anforowicz, 2017-01-24 17:08:58]

lukasza@chromium.org changed reviewers:
+ jam@chromium.org

[Łukasz Anforowicz, 2017-01-24 17:08:59]

jam@ - can you PTAL at
    components/plugins/renderer/webview_plugin.cc
    components/printing/renderer/print_web_view_helper.cc
    extensions/renderer/scoped_web_frame.cc
    media/blink/multibuffer_data_source_unittest.cc
    media/blink/resource_multibuffer_data_provider_unittest.cc
    media/blink/webmediaplayer_impl_unittest.cc
?

[jam, 2017-01-24 17:49:42]

lgtm

[Łukasz Anforowicz, 2017-01-24 17:55:26]

The CQ bit was checked by lukasza@chromium.org

[commit-bot: I haz the power, 2017-01-24 17:55:45]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-24 18:00:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-24 18:00:33]

Failed to apply patch for content/renderer/render_frame_impl.cc:
While running git apply --index -p1;
  error: patch failed: content/renderer/render_frame_impl.cc:1004
  error: content/renderer/render_frame_impl.cc: patch does not apply

Patch:       content/renderer/render_frame_impl.cc
Index: content/renderer/render_frame_impl.cc
diff --git a/content/renderer/render_frame_impl.cc
b/content/renderer/render_frame_impl.cc
index
4d031368d5826a9f0996aa158da05c49f1c56fb7..7b2052006cc323459239efc87ddc07f057668708
100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -936,7 +936,9 @@ RenderFrameImpl* RenderFrameImpl::CreateMainFrame(
       RenderFrameImpl::Create(render_view, routing_id);
   render_frame->InitializeBlameContext(nullptr);
   WebLocalFrame* web_frame = WebLocalFrame::create(
-      blink::WebTreeScopeType::Document, render_frame, opener);
+      blink::WebTreeScopeType::Document, render_frame,
+      render_frame->blink_interface_provider_.get(),
+      render_frame->blink_interface_registry_.get(), opener);
   render_frame->BindToWebFrame(web_frame);
   render_view->webview()->setMainFrame(web_frame);
   render_frame->render_widget_ = RenderWidget::CreateForFrame(
@@ -983,6 +985,8 @@ void RenderFrameImpl::CreateFrame(
         replicated_state.scope, WebString::fromUTF8(replicated_state.name),
         WebString::fromUTF8(replicated_state.unique_name),
         replicated_state.sandbox_flags, render_frame,
+        render_frame->blink_interface_provider_.get(),
+        render_frame->blink_interface_registry_.get(),
         previous_sibling_web_frame,
         frame_owner_properties.ToWebFrameOwnerProperties(),
         ResolveOpener(opener_routing_id));
@@ -1004,7 +1008,9 @@ void RenderFrameImpl::CreateFrame(
     render_frame->InitializeBlameContext(nullptr);
     render_frame->proxy_routing_id_ = proxy_routing_id;
     web_frame = blink::WebLocalFrame::createProvisional(
-        render_frame, proxy->web_frame(), replicated_state.sandbox_flags);
+        render_frame, render_frame->blink_interface_provider_.get(),
+        render_frame->blink_interface_registry_.get(), proxy->web_frame(),
+        replicated_state.sandbox_flags);
   }
   render_frame->BindToWebFrame(web_frame);
   CHECK(parent_routing_id != MSG_ROUTING_NONE || !web_frame->parent());
@@ -3056,8 +3062,10 @@ blink::WebLocalFrame* RenderFrameImpl::createChildFrame(
   RenderFrameImpl* child_render_frame =
       RenderFrameImpl::Create(render_view_, child_routing_id);
   child_render_frame->InitializeBlameContext(this);
-  blink::WebLocalFrame* web_frame =
-      WebLocalFrame::create(scope, child_render_frame);
+  blink::WebLocalFrame* web_frame = WebLocalFrame::create(
+      scope, child_render_frame,
+      child_render_frame->blink_interface_provider_.get(),
+      child_render_frame->blink_interface_registry_.get());
   child_render_frame->BindToWebFrame(web_frame);
 
   // Add the frame to the frame tree and initialize it.
@@ -6707,14 +6715,6 @@ void
RenderFrameImpl::checkIfAudioSinkExistsAndIsAuthorized(
                    .device_status());
 }
 
-blink::InterfaceProvider* RenderFrameImpl::interfaceProvider() {
-  return blink_interface_provider_.get();
-}
-
-blink::InterfaceRegistry* RenderFrameImpl::interfaceRegistry() {
-  return blink_interface_registry_.get();
-}
-
 blink::WebPageVisibilityState RenderFrameImpl::visibilityState() const {
   const RenderFrameImpl* local_root = GetLocalRoot();
   blink::WebPageVisibilityState current_state =

[Łukasz Anforowicz, 2017-01-24 18:15:02]

The CQ bit was checked by lukasza@chromium.org

[Łukasz Anforowicz, 2017-01-24 18:15:02]

The patchset sent to the CQ was uploaded after l-g-t-m from jam@chromium.org,
dcheng@chromium.org, nasko@chromium.org
Link to the patchset: https://codereview.chromium.org/2651583007/#ps100001
(title: "Rebasing...")

[commit-bot: I haz the power, 2017-01-24 18:15:29]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-24 19:40:50]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1485281702261050,
"parent_rev": "f611080eb86df037b4e687dc9d2821d68721d724", "commit_rev":
"2d6720b079b98fb9104c7274ef033a08d9e68efc"}

[commit-bot: I haz the power, 2017-01-24 19:41:25]

Description was changed from

==========
Plumbing blink::Interface{Provider|Registry} through WebLocalFrame's
constructor.

This CL helps remove WebFrameClient::interfaceProvider methods which
would have clashed (after the Great Blink Rename) with either
InterfaceProvider type and/or RenderFrameImpl::GetInterfaceProvider
method.  Same for interfaceRegistry method.

BUG=682560
==========

to

==========
Plumbing blink::Interface{Provider|Registry} through WebLocalFrame's
constructor.

This CL helps remove WebFrameClient::interfaceProvider methods which
would have clashed (after the Great Blink Rename) with either
InterfaceProvider type and/or RenderFrameImpl::GetInterfaceProvider
method.  Same for interfaceRegistry method.

BUG=682560

Review-Url: https://codereview.chromium.org/2651583007
Cr-Commit-Position: refs/heads/master@{#445781}
Committed:
https://chromium.googlesource.com/chromium/src/+/2d6720b079b98fb9104c7274ef03...
==========

[commit-bot: I haz the power, 2017-01-24 19:41:27]

Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/2d6720b079b98fb9104c7274ef03...