Remove WebContents::GetRoutingID().

Remove WebContents::GetRoutingID().

Replace all definitions, declarations, and uses of
WebContents::GetRoutingID() with
web_contents->GetRenderViewHost()->GetRoutingID().
This will make it more clear which routing id is being used.
Having GetRoutingID() on WebContents is both confusing (WebContents
isn't a routable object) and is prone to misuse with OOPIFs.

No behaviour changes should be caused by this CL.

BUG=666525
Committed: https://crrev.com/996be1115fde6db6ff18ed5b03278a6610a5eb38
Cr-Commit-Position: refs/heads/master@{#435182}

[davidsac (gone - try alexmos), 2016-11-18 20:16:16]

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

[commit-bot: I haz the power, 2016-11-18 20:16:55]

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

[davidsac (gone - try alexmos), 2016-11-18 20:17:21]

davidsac@chromium.org changed reviewers:
+ alexmos@chromium.org

[commit-bot: I haz the power, 2016-11-18 20:58:15]

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

[commit-bot: I haz the power, 2016-11-18 20:58:16]

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

[davidsac (gone - try alexmos), 2016-11-18 23:13:18]

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

[commit-bot: I haz the power, 2016-11-18 23:13:44]

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, 2016-11-18 23:30:17]

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

[commit-bot: I haz the power, 2016-11-18 23:30:18]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[alexmos, 2016-11-21 18:04:10]

Thanks!  Looks like there's another call site in aw_contents.cc that you'll need
to update to make it compile.  Unfortunately, some Android-related things are
not indexed for codesearch, so trybots are the only way to find out about them
in a situation like this.

Also looks like visual_state_browsertest needs a header include.

[davidsac (gone - try alexmos), 2016-11-21 21:56:21]

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

[commit-bot: I haz the power, 2016-11-21 21:57:10]

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, 2016-11-21 22:38:24]

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

[commit-bot: I haz the power, 2016-11-21 22:38:26]

Dry run: Try jobs failed on following builders:
  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...)

[davidsac (gone - try alexmos), 2016-11-21 23:04:06]

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

[commit-bot: I haz the power, 2016-11-21 23:05:08]

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, 2016-11-21 23:28:56]

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

[commit-bot: I haz the power, 2016-11-21 23:28:56]

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

[davidsac (gone - try alexmos), 2016-11-21 23:34:05]

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

[commit-bot: I haz the power, 2016-11-21 23:34:51]

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, 2016-11-22 00:03:16]

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

[commit-bot: I haz the power, 2016-11-22 00:03:16]

Dry run: Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[davidsac (gone - try alexmos), 2016-11-22 01:23:06]

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

[commit-bot: I haz the power, 2016-11-22 01:23:40]

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

[alexmos, 2016-11-22 01:29:06]

Thanks, LGTM now that the trybots are getting green.  A few observations below,
but I don't think any of them need code changes.

https://codereview.chromium.org/2515823002/diff/80001/content/public/browser/...
File content/public/browser/web_contents.h (left):

https://codereview.chromium.org/2515823002/diff/80001/content/public/browser/...
content/public/browser/web_contents.h:268: virtual int GetRoutingID() const = 0;
Interestingly, the old code handled the case where the RenderViewHost was null,
but I don't think that's actually possible, and regardless, both old and new
code would crash on GetRoutingID() if the RenderViewHost was ever null.

https://codereview.chromium.org/2515823002/diff/80001/content/public/browser/...
File content/public/browser/web_contents_observer.cc (right):

https://codereview.chromium.org/2515823002/diff/80001/content/public/browser/...
content/public/browser/web_contents_observer.cc:63: int
WebContentsObserver::routing_id() const {
Interesting.  This looks like another bad API that we might want to remove in a
follow-up CL, or at least audit its uses.  At the very least I think it should
be named something like render_view_routing_id().

https://codereview.chromium.org/2515823002/diff/80001/content/shell/browser/l...
File content/shell/browser/layout_test/blink_test_controller.cc (right):

https://codereview.chromium.org/2515823002/diff/80001/content/shell/browser/l...
content/shell/browser/layout_test/blink_test_controller.cc:924:
main_window_->web_contents()->GetRenderViewHost()->GetRoutingID(),
Seems like we get here to send a reply when layout tests use
getBluetoothManualChooserEvents (using the API in
https://webbluetoothcg.github.io/web-bluetooth/tests.html).  That might be
broken when we try to do it from an OOPIF.  But probably not a big deal as it's
only for testing.

[commit-bot: I haz the power, 2016-11-22 03:29:40]

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

[commit-bot: I haz the power, 2016-11-22 03:29:41]

Dry run: This issue passed the CQ dry run.

[davidsac (gone - try alexmos), 2016-11-22 21:23:18]

davidsac@chromium.org changed reviewers:
+ creis@chromium.org

[davidsac (gone - try alexmos), 2016-11-22 21:23:19]

Charlie, can you please review my CL in content/ for OWNERS?

[Charlie Reis, 2016-11-23 07:53:34]

creis@chromium.org changed reviewers:
+ nick@chromium.org

[Charlie Reis, 2016-11-23 07:53:35]

[+nick]

Thanks for the cleanup!  It's good to use this as a chance to audit things.  We
may have found one old bug below, but otherwise this CL looks good.

https://codereview.chromium.org/2515823002/diff/100001/chrome/browser/loader/...
File chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc (right):

https://codereview.chromium.org/2515823002/diff/100001/chrome/browser/loader/...
chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc:248:
web_contents->GetRenderViewHost()->GetRoutingID());
This line looks potentially wrong to me, before and after the change.  Nick, can
you help me confirm whether there's a bug here?

Looks like LaunchURL is run via a task posted from the IO thread to the UI
thread, with |render_process_id| set to a ResourceRequestInfo's GetChildID. 
That means there's potentially a race where |web_contents| has done a
cross-process navigation and is now showing a different RenderViewHost that
doesn't correspond to render_process_id.

This LaunchUrlWithoutSecurityCheck uses the process ID / routing ID pair to look
up the WebContents, so if we pass in a non-matching process ID and routing ID,
we could get the wrong WebContents.

Maybe we should be returning early at the top of LaunchURL if
web_contents->GetRenderViewHost()->GetProcess()->GetID() isn't equal to
|render_process_id|, similar to how we return if the tab is now gone?

https://codereview.chromium.org/2515823002/diff/100001/content/browser/web_co...
File content/browser/web_contents/web_contents_impl.cc (left):

https://codereview.chromium.org/2515823002/diff/100001/content/browser/web_co...
content/browser/web_contents/web_contents_impl.cc:902: return MSG_ROUTING_NONE;
Technically there's a small risk we could see some crashes in the new code if
anyone hit this case in practice, but I don't think that's likely.

[ncarter (slow), 2016-11-23 18:04:22]

https://codereview.chromium.org/2515823002/diff/100001/chrome/browser/loader/...
File chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc (right):

https://codereview.chromium.org/2515823002/diff/100001/chrome/browser/loader/...
chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc:248:
web_contents->GetRenderViewHost()->GetRoutingID());
On 2016/11/23 07:53:35, Charlie Reis (slow) wrote:
> This line looks potentially wrong to me, before and after the change.  Nick,
can
> you help me confirm whether there's a bug here?
> 
> Looks like LaunchURL is run via a task posted from the IO thread to the UI
> thread, with |render_process_id| set to a ResourceRequestInfo's GetChildID. 
> That means there's potentially a race where |web_contents| has done a
> cross-process navigation and is now showing a different RenderViewHost that
> doesn't correspond to render_process_id.
> 
> This LaunchUrlWithoutSecurityCheck uses the process ID / routing ID pair to
look
> up the WebContents, so if we pass in a non-matching process ID and routing ID,
> we could get the wrong WebContents.
> 
> Maybe we should be returning early at the top of LaunchURL if
> web_contents->GetRenderViewHost()->GetProcess()->GetID() isn't equal to
> |render_process_id|, similar to how we return if the tab is now gone?

Agree, with charlie that there are bugs here that are worth fixing. In
particular:

 - In general, render_process_ids and routing_ids need to be conveyed as a
   (logical) tuple, since each process has its own namespace of route_ids.
   You never want to take a route_id from one place and pair it with a
   process_id taken from another place.
 - If this request originated from an oopifs, then render_process_id is going
   to differ from web_contents->GetRenderViewHost()->GetProcess()->GetID().
   Thus this routing pair might resolve to nullptr, or event, to a
   totally unrelated tab.
 - As Charlie mentioned, if there is a race with a process-transferring
   navigation in the main frame, it's possible that render_process_id
   pertained to the previous value of RenderViewHost, causing the same
   bug as above.
 - This is aesthetic, not functional, but in the called function,
   the route_id value is named |tab_contents_id|. Route ids don't
   (and can't) uniquely identify a tab; these plumbed arguments
   should be renamed to route_id.

As background, this codepath is what's exercised when you click a mailto: link
on
every platform except Android -- really, we get here for any protocol that isn't
directly handled by chrome, and which might have an OS-registered handler.
The external handler could be an application outside of Chrome, like Outlook.

[ncarter (slow), 2016-11-23 21:26:40]

Description was changed from

==========
Remove WebContents::GetRoutingID().

Replace all definitions, declarations, and uses of
WebContents::GetRoutingID() with
web_contents->GetRenderViewHost()->GetRoutingID().
This will make it more clear which routing id is being used.
Having GetRoutingID() on WebContents is both confusing (WebContents
isn't a routable object) and is prone to misuse with OOPIFs.

No behaviour changes should be caused by this CL.

BUG=666525
==========

to

==========
Remove WebContents::GetRoutingID().

Replace all definitions, declarations, and uses of
WebContents::GetRoutingID() with
web_contents->GetRenderViewHost()->GetRoutingID().
This will make it more clear which routing id is being used.
Having GetRoutingID() on WebContents is both confusing (WebContents
isn't a routable object) and is prone to misuse with OOPIFs.

No behaviour changes should be caused by this CL.

BUG=666525
==========

[ncarter (slow), 2016-11-23 21:26:40]

nick@chromium.org changed reviewers:
- creis@chromium.org

[ncarter (slow), 2016-11-23 21:26:54]

nick@chromium.org changed reviewers:
+ creis@chromium.org
- nick@chromium.org

[alexmos, 2016-11-23 22:46:20]

https://codereview.chromium.org/2515823002/diff/100001/chrome/browser/loader/...
File chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc (right):

https://codereview.chromium.org/2515823002/diff/100001/chrome/browser/loader/...
chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc:248:
web_contents->GetRenderViewHost()->GetRoutingID());
On 2016/11/23 18:04:22, ncarter wrote:
> On 2016/11/23 07:53:35, Charlie Reis (slow) wrote:
> > This line looks potentially wrong to me, before and after the change.  Nick,
> can
> > you help me confirm whether there's a bug here?
> > 
> > Looks like LaunchURL is run via a task posted from the IO thread to the UI
> > thread, with |render_process_id| set to a ResourceRequestInfo's GetChildID. 
> > That means there's potentially a race where |web_contents| has done a
> > cross-process navigation and is now showing a different RenderViewHost that
> > doesn't correspond to render_process_id.
> > 
> > This LaunchUrlWithoutSecurityCheck uses the process ID / routing ID pair to
> look
> > up the WebContents, so if we pass in a non-matching process ID and routing
ID,
> > we could get the wrong WebContents.
> > 
> > Maybe we should be returning early at the top of LaunchURL if
> > web_contents->GetRenderViewHost()->GetProcess()->GetID() isn't equal to
> > |render_process_id|, similar to how we return if the tab is now gone?
> 
> Agree, with charlie that there are bugs here that are worth fixing. In
> particular:
> 
>  - In general, render_process_ids and routing_ids need to be conveyed as a
>    (logical) tuple, since each process has its own namespace of route_ids.
>    You never want to take a route_id from one place and pair it with a
>    process_id taken from another place.
>  - If this request originated from an oopifs, then render_process_id is going
>    to differ from web_contents->GetRenderViewHost()->GetProcess()->GetID().
>    Thus this routing pair might resolve to nullptr, or event, to a
>    totally unrelated tab.
>  - As Charlie mentioned, if there is a race with a process-transferring
>    navigation in the main frame, it's possible that render_process_id
>    pertained to the previous value of RenderViewHost, causing the same
>    bug as above.
>  - This is aesthetic, not functional, but in the called function,
>    the route_id value is named |tab_contents_id|. Route ids don't
>    (and can't) uniquely identify a tab; these plumbed arguments
>    should be renamed to route_id.
> 
> As background, this codepath is what's exercised when you click a mailto: link
> on
> every platform except Android -- really, we get here for any protocol that
isn't
> directly handled by chrome, and which might have an OS-registered handler.
> The external handler could be an application outside of Chrome, like Outlook.

Nick/Charlie: thanks for pointing out these problems!  Indeed, links for
external protocols are broken in OOPIFs. I've filed
https://bugs.chromium.org/p/chromium/issues/detail?id=668289 with some more
details and discussion. 

Since that would likely require several more changes as well as a test, is it ok
for us to fix that in a followup CL, and land this one with a TODO here?

[Charlie Reis, 2016-11-23 22:48:27]

LGTM

https://codereview.chromium.org/2515823002/diff/100001/chrome/browser/loader/...
File chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc (right):

https://codereview.chromium.org/2515823002/diff/100001/chrome/browser/loader/...
chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc:248:
web_contents->GetRenderViewHost()->GetRoutingID());
On 2016/11/23 22:46:20, alexmos wrote:
> On 2016/11/23 18:04:22, ncarter wrote:
> > On 2016/11/23 07:53:35, Charlie Reis (slow) wrote:
> > > This line looks potentially wrong to me, before and after the change. 
Nick,
> > can
> > > you help me confirm whether there's a bug here?
> > > 
> > > Looks like LaunchURL is run via a task posted from the IO thread to the UI
> > > thread, with |render_process_id| set to a ResourceRequestInfo's
GetChildID. 
> > > That means there's potentially a race where |web_contents| has done a
> > > cross-process navigation and is now showing a different RenderViewHost
that
> > > doesn't correspond to render_process_id.
> > > 
> > > This LaunchUrlWithoutSecurityCheck uses the process ID / routing ID pair
to
> > look
> > > up the WebContents, so if we pass in a non-matching process ID and routing
> ID,
> > > we could get the wrong WebContents.
> > > 
> > > Maybe we should be returning early at the top of LaunchURL if
> > > web_contents->GetRenderViewHost()->GetProcess()->GetID() isn't equal to
> > > |render_process_id|, similar to how we return if the tab is now gone?
> > 
> > Agree, with charlie that there are bugs here that are worth fixing. In
> > particular:
> > 
> >  - In general, render_process_ids and routing_ids need to be conveyed as a
> >    (logical) tuple, since each process has its own namespace of route_ids.
> >    You never want to take a route_id from one place and pair it with a
> >    process_id taken from another place.
> >  - If this request originated from an oopifs, then render_process_id is
going
> >    to differ from web_contents->GetRenderViewHost()->GetProcess()->GetID().
> >    Thus this routing pair might resolve to nullptr, or event, to a
> >    totally unrelated tab.
> >  - As Charlie mentioned, if there is a race with a process-transferring
> >    navigation in the main frame, it's possible that render_process_id
> >    pertained to the previous value of RenderViewHost, causing the same
> >    bug as above.
> >  - This is aesthetic, not functional, but in the called function,
> >    the route_id value is named |tab_contents_id|. Route ids don't
> >    (and can't) uniquely identify a tab; these plumbed arguments
> >    should be renamed to route_id.
> > 
> > As background, this codepath is what's exercised when you click a mailto:
link
> > on
> > every platform except Android -- really, we get here for any protocol that
> isn't
> > directly handled by chrome, and which might have an OS-registered handler.
> > The external handler could be an application outside of Chrome, like
Outlook.
> 
> Nick/Charlie: thanks for pointing out these problems!  Indeed, links for
> external protocols are broken in OOPIFs. I've filed
> https://bugs.chromium.org/p/chromium/issues/detail?id=668289 with some more
> details and discussion. 
> 
> Since that would likely require several more changes as well as a test, is it
ok
> for us to fix that in a followup CL, and land this one with a TODO here?

Yes, a TODO citing that bug is fine.  No need to fix it in this CL.  Thanks!

[davidsac (gone - try alexmos), 2016-11-24 02:10:23]

davidsac@chromium.org changed reviewers:
+ rdevlin.cronin@chromium.org, sgurun@chromium.org, thestig@chromium.org

[davidsac (gone - try alexmos), 2016-11-24 02:10:24]

rdevlin.cronin@chromium.org for extensions/
sgurun@chromium.org for android_webview/
thestig@chromium.org for chrome/

[davidsac (gone - try alexmos), 2016-11-24 02:12:17]

On 2016/11/24 02:10:24, davidsac wrote:
> mailto:rdevlin.cronin@chromium.org for extensions/
> mailto:sgurun@chromium.org for android_webview/
> mailto:thestig@chromium.org for chrome/

Hi,

Would you all please review this CL for OWNER approval for your respective
areas?

Thanks,

Andrew

[Lei Zhang, 2016-11-24 17:18:26]

This looks good, but...

https://codereview.chromium.org/2515823002/diff/120001/chrome/browser/ui/webu...
File chrome/browser/ui/webui/print_preview/print_preview_handler.cc (right):

https://codereview.chromium.org/2515823002/diff/120001/chrome/browser/ui/webu...
chrome/browser/ui/webui/print_preview/print_preview_handler.cc:925:
initiator->GetRenderViewHost()->GetRoutingID());
Uh oh, this is wrong. I have a separate fix:
https://codereview.chromium.org/2527913002/

Hope you don't mind, but I'd like to land that separately and merge to M56.

[Devlin, 2016-11-28 16:19:29]

extensions lgtm

Note: WebContents::GetRoutingID() would check the render view host for nullness
first.  The extensions sites should be safe (the render view should always
exist), but we should be sure the others are if you haven't already.

[ncarter (slow), 2016-11-28 18:04:48]

On 2016/11/28 16:19:29, Devlin wrote:
> extensions lgtm
> 
> Note: WebContents::GetRoutingID() would check the render view host for
nullness
> first.  The extensions sites should be safe (the render view should always
> exist), but we should be sure the others are if you haven't already.

We don't null-check RVH and RFH's. The RVH should never be null outside of a
very brief window during WebContents destruction (and as far as I know, there's
no observable side effects during that window, though with the notification
service anything is possible).

[davidsac (gone - try alexmos), 2016-11-28 18:44:31]

The change to print_preview_handler.cc has been removed as discussed.

[sgurun-gerrit only, 2016-11-28 18:51:39]

On 2016/11/28 18:44:31, davidsac wrote:
> The change to print_preview_handler.cc has been removed as discussed.

aw lgtm

[mmenke, 2016-11-28 18:53:24]

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

[mmenke, 2016-11-28 18:53:25]

https://codereview.chromium.org/2515823002/diff/140001/content/browser/loader...
File content/browser/loader/resource_dispatcher_host_unittest.cc (right):

https://codereview.chromium.org/2515823002/diff/140001/content/browser/loader...
content/browser/loader/resource_dispatcher_host_unittest.cc:1201:
web_contents_->GetRenderViewHost()->GetRoutingID(), request_id, request);
Need to include RenderViewHost header, and update deps, if needed.

[davidsac (gone - try alexmos), 2016-11-28 19:09:35]

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

[commit-bot: I haz the power, 2016-11-28 19:10:42]

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

[davidsac (gone - try alexmos), 2016-11-28 19:12:42]

An include for "render_view_host.h" is now part of this Change List.

https://codereview.chromium.org/2515823002/diff/140001/content/browser/loader...
File content/browser/loader/resource_dispatcher_host_unittest.cc (right):

https://codereview.chromium.org/2515823002/diff/140001/content/browser/loader...
content/browser/loader/resource_dispatcher_host_unittest.cc:1201:
web_contents_->GetRenderViewHost()->GetRoutingID(), request_id, request);
On 2016/11/28 18:53:25, mmenke wrote:
> Need to include RenderViewHost header, and update deps, if needed.

Done.

[mmenke, 2016-11-28 19:14:07]

loader/ LGTM

[commit-bot: I haz the power, 2016-11-28 19:34:21]

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

[commit-bot: I haz the power, 2016-11-28 19:34:22]

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

[Lei Zhang, 2016-11-29 03:49:46]

lgtm now that https://crrev.com/434853 has landed. Thanks for waiting!

[Lei Zhang, 2016-11-29 06:56:08]

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

[commit-bot: I haz the power, 2016-11-29 06:56:30]

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, 2016-11-29 08:22:49]

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

[commit-bot: I haz the power, 2016-11-29 08:22:51]

Dry run: This issue passed the CQ dry run.

[davidsac (gone - try alexmos), 2016-11-29 18:27:54]

The CQ bit was checked by davidsac@chromium.org

[davidsac (gone - try alexmos), 2016-11-29 18:27:54]

The patchset sent to the CQ was uploaded after l-g-t-m from creis@chromium.org,
alexmos@chromium.org, rdevlin.cronin@chromium.org, sgurun@chromium.org
Link to the patchset: https://codereview.chromium.org/2515823002/#ps160001
(title: "add render_view_host.h include in
resource_dispatcher_host_unittest.cc")

[commit-bot: I haz the power, 2016-11-29 18:28:17]

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, 2016-11-29 18:48:30]

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

[commit-bot: I haz the power, 2016-11-29 18:48:32]

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...)

[davidsac (gone - try alexmos), 2016-11-29 19:14:48]

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

[commit-bot: I haz the power, 2016-11-29 19:15:11]

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, 2016-11-29 19:18:23]

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

[commit-bot: I haz the power, 2016-11-29 19:18:25]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[davidsac (gone - try alexmos), 2016-11-29 23:39:58]

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

[commit-bot: I haz the power, 2016-11-29 23:41:18]

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, 2016-11-30 01:43:17]

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

[commit-bot: I haz the power, 2016-11-30 01:43:18]

Dry run: Try jobs failed on following builders:
  blimp_linux_dbg on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)

[Lei Zhang, 2016-11-30 07:35:56]

The CQ bit was checked by thestig@chromium.org

[Lei Zhang, 2016-11-30 07:35:57]

The patchset sent to the CQ was uploaded after l-g-t-m from sgurun@chromium.org,
rdevlin.cronin@chromium.org, thestig@chromium.org, alexmos@chromium.org,
creis@chromium.org, mmenke@chromium.org
Link to the patchset: https://codereview.chromium.org/2515823002/#ps200001
(title: "add render_view_host.h include in
resource_dispatcher_host_unittest.cc")

[commit-bot: I haz the power, 2016-11-30 07:36:28]

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, 2016-11-30 08:56:42]

CQ is committing da patch.

Bot data: {"patchset_id": 200001, "attempt_start_ts": 1480491356131570,
"parent_rev": "f39af337000ae01151440dcabc3e9d60367b1115", "commit_rev":
"9c9d6bb2826e82befab81937966a6bfb8645aded"}

[commit-bot: I haz the power, 2016-11-30 08:57:09]

Description was changed from

==========
Remove WebContents::GetRoutingID().

Replace all definitions, declarations, and uses of
WebContents::GetRoutingID() with
web_contents->GetRenderViewHost()->GetRoutingID().
This will make it more clear which routing id is being used.
Having GetRoutingID() on WebContents is both confusing (WebContents
isn't a routable object) and is prone to misuse with OOPIFs.

No behaviour changes should be caused by this CL.

BUG=666525
==========

to

==========
Remove WebContents::GetRoutingID().

Replace all definitions, declarations, and uses of
WebContents::GetRoutingID() with
web_contents->GetRenderViewHost()->GetRoutingID().
This will make it more clear which routing id is being used.
Having GetRoutingID() on WebContents is both confusing (WebContents
isn't a routable object) and is prone to misuse with OOPIFs.

No behaviour changes should be caused by this CL.

BUG=666525
==========

[commit-bot: I haz the power, 2016-11-30 08:57:11]

Committed patchset #11 (id:200001)

[commit-bot: I haz the power, 2016-11-30 09:02:03]

Description was changed from

==========
Remove WebContents::GetRoutingID().

Replace all definitions, declarations, and uses of
WebContents::GetRoutingID() with
web_contents->GetRenderViewHost()->GetRoutingID().
This will make it more clear which routing id is being used.
Having GetRoutingID() on WebContents is both confusing (WebContents
isn't a routable object) and is prone to misuse with OOPIFs.

No behaviour changes should be caused by this CL.

BUG=666525
==========

to

==========
Remove WebContents::GetRoutingID().

Replace all definitions, declarations, and uses of
WebContents::GetRoutingID() with
web_contents->GetRenderViewHost()->GetRoutingID().
This will make it more clear which routing id is being used.
Having GetRoutingID() on WebContents is both confusing (WebContents
isn't a routable object) and is prone to misuse with OOPIFs.

No behaviour changes should be caused by this CL.

BUG=666525

Committed: https://crrev.com/996be1115fde6db6ff18ed5b03278a6610a5eb38
Cr-Commit-Position: refs/heads/master@{#435182}
==========

[commit-bot: I haz the power, 2016-11-30 09:02:06]

Patchset 11 (id:??) landed as
https://crrev.com/996be1115fde6db6ff18ed5b03278a6610a5eb38
Cr-Commit-Position: refs/heads/master@{#435182}