Remove some WebContents::GetRenderViewHost() calls.

Remove some WebContents::GetRenderViewHost() calls.

web_contents->GetRenderViewHost()->GetProcess() becomes
web_contents->GetMainFrame()->GetProcess() unless the call is part of a
process id + routing id pair.

Also remove an obsolete RenderViewGone reference in
background_printing_manager.cc.

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

[Lei Zhang, 2016-11-24 11:30:10]

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

[commit-bot: I haz the power, 2016-11-24 11:30:45]

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-24 13:32:48]

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

[commit-bot: I haz the power, 2016-11-24 13:32:48]

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_amd64-generic_chromium_compile_only_ng 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_asan_rel_ng 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)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)

[Lei Zhang, 2016-11-24 17:30:25]

thestig@chromium.org changed reviewers:
+ creis@chromium.org, nasko@chromium.org, rdevlin.cronin@chromium.org

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

No rush, and probably conflicts with https://codereview.chromium.org/2515823002/
so I'll land this later.

[Devlin, 2016-11-28 16:12:03]

One question: When is web_contents->GetMainFrame()->GetProcess() different than
web_contents->GetRenderProcessHost()?  I would *expect* them to be the same, but
it's not 100% clear.  From the implementation, it also seems that
GetRenderProcessHost() always == GetRenderViewHost()->GetProcess(), but it's not
immediately clear that the same is true for GetMainFrame()->GetProcess().

Is there supposed to be a (potential) behavior change in this CL?

https://codereview.chromium.org/2527133002/diff/1/chrome/browser/extensions/a...
File
chrome/browser/extensions/api/declarative_content/declarative_content_condition_tracker_test.cc
(right):

https://codereview.chromium.org/2527133002/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/declarative_content/declarative_content_condition_tracker_test.cc:9:
#include "content/public/browser/render_view_host.h"
do we still need this?  And separately we should add render_frame_host for iwyu
(though possibly moot with high-level comment).

[nasko, 2016-11-28 19:34:01]

LGTM

[nasko, 2016-11-28 19:39:17]

On 2016/11/28 16:12:03, Devlin wrote:
> One question: When is web_contents->GetMainFrame()->GetProcess() different
than
> web_contents->GetRenderProcessHost()?  I would *expect* them to be the same,
but
> it's not 100% clear.  

The latter API (WC::GetRenderProcessHost) is deprecated and dangerous. It is
actively being removed as part of https://crbug.com/666525.

> From the implementation, it also seems that
> GetRenderProcessHost() always == GetRenderViewHost()->GetProcess(), but it's
not
> immediately clear that the same is true for GetMainFrame()->GetProcess().

If you look at the implementation of GetRenderViewHost(), it will return the
RenderViewHost associated with the main frame and they are in the same process
by design. As such, I don't think they will return different values, even in the
case of out-of-process iframes, when the context is always the main frame.

> Is there supposed to be a (potential) behavior change in this CL?

I don't expect one.

[Devlin, 2016-11-28 19:41:08]

On 2016/11/28 19:39:17, nasko wrote:
> On 2016/11/28 16:12:03, Devlin wrote:
> > One question: When is web_contents->GetMainFrame()->GetProcess() different
> than
> > web_contents->GetRenderProcessHost()?  I would *expect* them to be the same,
> but
> > it's not 100% clear.  
> 
> The latter API (WC::GetRenderProcessHost) is deprecated and dangerous. It is
> actively being removed as part of https://crbug.com/666525.
> 
> > From the implementation, it also seems that
> > GetRenderProcessHost() always == GetRenderViewHost()->GetProcess(), but it's
> not
> > immediately clear that the same is true for GetMainFrame()->GetProcess().
> 
> If you look at the implementation of GetRenderViewHost(), it will return the
> RenderViewHost associated with the main frame and they are in the same process
> by design. As such, I don't think they will return different values, even in
the
> case of out-of-process iframes, when the context is always the main frame.
> 
> > Is there supposed to be a (potential) behavior change in this CL?
> 
> I don't expect one.

Coolio, thanks for the explanation.  Can we mark
WebContents::GetRenderProcessHost() as deprecated?

extensions lgtm

[Charlie Reis, 2016-11-28 21:00:41]

Thanks, LGTM with one change.  Also, please list 666525 as the bug number in the
CL, since it's close enough in spirit to that work.

https://codereview.chromium.org/2527133002/diff/1/content/test/test_web_conte...
File content/test/test_web_contents.cc (right):

https://codereview.chromium.org/2527133002/diff/1/content/test/test_web_conte...
content/test/test_web_contents.cc:268: ProcessRoutingIdPair
key(contents->GetMainFrame()->GetProcess()->GetID(),
We shouldn't change this one.  We should always get the process ID and routing
ID from the same object.  (It *should* be the case that the RVH and RFH will
give the same process ID here, but it's quite confusing to get the process ID
from one and the route ID from the other.)

Nick has a CL converting CreateNewWindow logic away from RVH, so we can leave
this one for that CL.

[Lei Zhang, 2016-11-30 10:02:48]

Description was changed from

==========
Remove some WebContents::GetRenderViewHost() calls.

web_contents->GetRenderViewHost()->GetProcess() becomes
web_contents->GetMainFrame()->GetProcess().

Also remove an obsolete RenderViewGone reference in
background_printing_manager.cc.
==========

to

==========
Remove some WebContents::GetRenderViewHost() calls.

web_contents->GetRenderViewHost()->GetProcess() becomes
web_contents->GetMainFrame()->GetProcess().

Also remove an obsolete RenderViewGone reference in
background_printing_manager.cc.

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

[Lei Zhang, 2016-11-30 10:04:45]

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

[commit-bot: I haz the power, 2016-11-30 10:05:03]

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 10:17:58]

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

[commit-bot: I haz the power, 2016-11-30 10:17:59]

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

[Lei Zhang, 2016-11-30 10:25:24]

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

[commit-bot: I haz the power, 2016-11-30 10:25:48]

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

[Lei Zhang, 2016-11-30 10:26:43]

Description was changed from

==========
Remove some WebContents::GetRenderViewHost() calls.

web_contents->GetRenderViewHost()->GetProcess() becomes
web_contents->GetMainFrame()->GetProcess().

Also remove an obsolete RenderViewGone reference in
background_printing_manager.cc.

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

to

==========
Remove some WebContents::GetRenderViewHost() calls.

web_contents->GetRenderViewHost()->GetProcess() becomes
web_contents->GetMainFrame()->GetProcess() unless the call is part of a
process id + routing id pair.

Also remove an obsolete RenderViewGone reference in
background_printing_manager.cc.

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

[Lei Zhang, 2016-11-30 10:27:00]

https://codereview.chromium.org/2527133002/diff/1/content/test/test_web_conte...
File content/test/test_web_contents.cc (right):

https://codereview.chromium.org/2527133002/diff/1/content/test/test_web_conte...
content/test/test_web_contents.cc:268: ProcessRoutingIdPair
key(contents->GetMainFrame()->GetProcess()->GetID(),
On 2016/11/28 21:00:41, Charlie Reis (slow) wrote:
> We shouldn't change this one.  We should always get the process ID and routing
> ID from the same object.  (It *should* be the case that the RVH and RFH will
> give the same process ID here, but it's quite confusing to get the process ID
> from one and the route ID from the other.)
> 
> Nick has a CL converting CreateNewWindow logic away from RVH, so we can leave
> this one for that CL.

Reverted.

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

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

[commit-bot: I haz the power, 2016-11-30 11:34:53]

Dry run: This issue passed the CQ dry run.

[Lei Zhang, 2016-11-30 18:45:33]

The CQ bit was checked by thestig@chromium.org

[Lei Zhang, 2016-11-30 18:45:33]

The patchset sent to the CQ was uploaded after l-g-t-m from creis@chromium.org,
rdevlin.cronin@chromium.org, nasko@chromium.org
Link to the patchset: https://codereview.chromium.org/2527133002/#ps60001
(title: "More reverts for the same reason as patch set 3, fixes merge conflict")

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

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 18:50:59]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1480531533029890,
"parent_rev": "282a57d5b3d9c2930a6e49caf3604b331cc83f9e", "commit_rev":
"e30724e8a7ec1deff634f092d2377120e6a5fa70"}

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

Description was changed from

==========
Remove some WebContents::GetRenderViewHost() calls.

web_contents->GetRenderViewHost()->GetProcess() becomes
web_contents->GetMainFrame()->GetProcess() unless the call is part of a
process id + routing id pair.

Also remove an obsolete RenderViewGone reference in
background_printing_manager.cc.

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

to

==========
Remove some WebContents::GetRenderViewHost() calls.

web_contents->GetRenderViewHost()->GetProcess() becomes
web_contents->GetMainFrame()->GetProcess() unless the call is part of a
process id + routing id pair.

Also remove an obsolete RenderViewGone reference in
background_printing_manager.cc.

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

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

Committed patchset #4 (id:60001)

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

Description was changed from

==========
Remove some WebContents::GetRenderViewHost() calls.

web_contents->GetRenderViewHost()->GetProcess() becomes
web_contents->GetMainFrame()->GetProcess() unless the call is part of a
process id + routing id pair.

Also remove an obsolete RenderViewGone reference in
background_printing_manager.cc.

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

to

==========
Remove some WebContents::GetRenderViewHost() calls.

web_contents->GetRenderViewHost()->GetProcess() becomes
web_contents->GetMainFrame()->GetProcess() unless the call is part of a
process id + routing id pair.

Also remove an obsolete RenderViewGone reference in
background_printing_manager.cc.

BUG=666525

Committed: https://crrev.com/c0cb07a009c9d607d265928ece3ead010a736e2b
Cr-Commit-Position: refs/heads/master@{#435358}
==========

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

Patchset 4 (id:??) landed as
https://crrev.com/c0cb07a009c9d607d265928ece3ead010a736e2b
Cr-Commit-Position: refs/heads/master@{#435358}