[Extensions] Remove ExtensionFunction usage of RenderViewHost

[Extensions] Remove ExtensionFunction usage of RenderViewHost

RenderViewHost is deprecated. Most ExtensionFunctions were updated
awhile back, but there were still some lingering ones. It looks like
one more (zooming in the file manager) can be safely removed.

BUG=498017
Committed: https://crrev.com/17f4094d5716c5c7b98031103a7d84c1474b28ab
Cr-Commit-Position: refs/heads/master@{#412536}

[Devlin, 2016-08-03 21:46:12]

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

[commit-bot: I haz the power, 2016-08-03 21:46:35]

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

[Devlin, 2016-08-03 22:29:19]

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

[Devlin, 2016-08-03 22:29:40]

Patchset #1 (id:1) has been deleted

[commit-bot: I haz the power, 2016-08-03 22:29:43]

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

[Devlin, 2016-08-03 22:31:17]

rdevlin.cronin@chromium.org changed reviewers:
+ dbeam@chromium.org, hirono@chromium.org

[Devlin, 2016-08-03 22:31:18]

+hirono and dbeam.  hirono@, mind taking a look?  dbeam@ (as an owner of
components/zoom) can you confirm that this is a reasonable replacement?  From
diving a bit into zoom code, it seems fine, but I wanna double check.

[Dan Beam, 2016-08-03 22:39:19]

lgtm

[commit-bot: I haz the power, 2016-08-03 23:54:42]

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

[commit-bot: I haz the power, 2016-08-03 23:54:43]

Dry run: This issue passed the CQ dry run.

[hirono, 2016-08-04 06:59:02]

On 2016/08/03 22:39:19, Dan Beam wrote:
> lgtm

I locally cherry-picked the change. It seems I cannot change zoom level in Files
app after applying the patch.

[Devlin, 2016-08-10 21:22:53]

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

[commit-bot: I haz the power, 2016-08-10 21:23: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-08-10 22:04:21]

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

[commit-bot: I haz the power, 2016-08-10 22:04:22]

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

[Devlin, 2016-08-10 23:04:02]

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

[commit-bot: I haz the power, 2016-08-10 23:05:18]

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

[Devlin, 2016-08-11 00:10:25]

On 2016/08/04 06:59:02, hirono wrote:
> I locally cherry-picked the change. It seems I cannot change zoom level in
Files
> app after applying the patch.

Ah, apparently it's untested.  Ensuring we create a zoom controller for app
windows makes it work.  Please take another look.

[Devlin, 2016-08-11 00:10:45]

rdevlin.cronin@chromium.org changed reviewers:
+ lazyboy@chromium.org

[Devlin, 2016-08-11 00:10:46]

+lazyboy for guestview stuff

[lazyboy, 2016-08-11 00:18:09]

lazyboy@chromium.org changed reviewers:
+ wjmaclean@chromium.org

[lazyboy, 2016-08-11 00:18:09]

+James who knows more about page zoom in guestview...

[commit-bot: I haz the power, 2016-08-11 02:47:15]

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

[commit-bot: I haz the power, 2016-08-11 02:47:16]

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

[wjmaclean, 2016-08-11 13:30:03]

LGTM, with a small question.

https://codereview.chromium.org/2204393002/diff/60001/extensions/browser/gues...
File extensions/browser/guest_view/web_view/web_view_guest.cc (right):

https://codereview.chromium.org/2204393002/diff/60001/extensions/browser/gues...
extensions/browser/guest_view/web_view/web_view_guest.cc:447: return
!did_set_explicit_zoom_;
This part of the CL seems a bit orthogonal to removing the reference to
RenderViewHost, though I suppose it's necessary to prevent the new pathway
through WebContents from stomping on the guest's zoom level if it was set
separately, is that right?

[Devlin, 2016-08-11 15:21:11]

https://codereview.chromium.org/2204393002/diff/60001/extensions/browser/gues...
File extensions/browser/guest_view/web_view/web_view_guest.cc (right):

https://codereview.chromium.org/2204393002/diff/60001/extensions/browser/gues...
extensions/browser/guest_view/web_view/web_view_guest.cc:447: return
!did_set_explicit_zoom_;
On 2016/08/11 13:30:03, wjmaclean wrote:
> This part of the CL seems a bit orthogonal to removing the reference to
> RenderViewHost, though I suppose it's necessary to prevent the new pathway
> through WebContents from stomping on the guest's zoom level if it was set
> separately, is that right?

Yep. Before AppWindows didn't have a ZoomController created, so we early-outed
in GuestViewBase.  Now that they have one, we need to make sure that we don't
track the embedder's zoom (e.g. when navigation commits or when the embedder
zoom adjusts).  It seems like this was the desired behavior all along, and that
previously we just lucked out by there not being a zoom controller in the
embedder.

[wjmaclean, 2016-08-11 15:22:53]

On 2016/08/11 15:21:11, Devlin wrote:
>
https://codereview.chromium.org/2204393002/diff/60001/extensions/browser/gues...
> File extensions/browser/guest_view/web_view/web_view_guest.cc (right):
> 
>
https://codereview.chromium.org/2204393002/diff/60001/extensions/browser/gues...
> extensions/browser/guest_view/web_view/web_view_guest.cc:447: return
> !did_set_explicit_zoom_;
> On 2016/08/11 13:30:03, wjmaclean wrote:
> > This part of the CL seems a bit orthogonal to removing the reference to
> > RenderViewHost, though I suppose it's necessary to prevent the new pathway
> > through WebContents from stomping on the guest's zoom level if it was set
> > separately, is that right?
> 
> Yep. Before AppWindows didn't have a ZoomController created, so we early-outed
> in GuestViewBase.  Now that they have one, we need to make sure that we don't
> track the embedder's zoom (e.g. when navigation commits or when the embedder
> zoom adjusts).  It seems like this was the desired behavior all along, and
that
> previously we just lucked out by there not being a zoom controller in the
> embedder.

That's good to know; thanks for following up!

[Devlin, 2016-08-12 01:01:41]

On 2016/08/11 00:10:25, Devlin (ooo aug12) wrote:
> On 2016/08/04 06:59:02, hirono wrote:
> > I locally cherry-picked the change. It seems I cannot change zoom level in
> Files
> > app after applying the patch.
> 
> Ah, apparently it's untested.  Ensuring we create a zoom controller for app
> windows makes it work.  Please take another look.

friendly ping -> hirono@

[hirono, 2016-08-15 03:57:07]

lgtm! Sorry for not replying soon. I was on vacation.

[Devlin, 2016-08-15 17:43:57]

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

[commit-bot: I haz the power, 2016-08-15 17:44:20]

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-08-15 18:31:12]

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

[commit-bot: I haz the power, 2016-08-15 18:31:13]

Dry run: This issue passed the CQ dry run.

[Devlin, 2016-08-15 19:16:13]

Description was changed from

==========
[Extensions] Remove a usage of RenderViewHost

RenderViewHost is deprecated. Most ExtensionFunctions were updated
awhile back, but there were still some lingering ones. It looks like
one more (zooming in the file manager) can be safely removed.

BUG=498017
==========

to

==========
[Extensions] Remove ExtensionFunction usage of RenderViewHost

RenderViewHost is deprecated. Most ExtensionFunctions were updated
awhile back, but there were still some lingering ones. It looks like
one more (zooming in the file manager) can be safely removed.

BUG=498017
==========

[Devlin, 2016-08-15 19:16:20]

The CQ bit was checked by rdevlin.cronin@chromium.org

[Devlin, 2016-08-15 19:16:21]

The patchset sent to the CQ was uploaded after l-g-t-m from dbeam@chromium.org,
hirono@chromium.org, wjmaclean@chromium.org
Link to the patchset: https://codereview.chromium.org/2204393002/#ps80001
(title: "Rebase")

[commit-bot: I haz the power, 2016-08-15 19:16:47]

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-08-15 19:20:52]

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

[commit-bot: I haz the power, 2016-08-15 19:20:54]

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

[Devlin, 2016-08-15 19:22:29]

rdevlin.cronin@chromium.org changed reviewers:
+ benwells@chromium.org

[Devlin, 2016-08-15 19:22:30]

+Ben for chrome/browser/ui/apps.

[benwells, 2016-08-17 04:25:49]

lgtm

[Devlin, 2016-08-17 14:44:55]

The CQ bit was checked by rdevlin.cronin@chromium.org

[commit-bot: I haz the power, 2016-08-17 14:45:18]

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-08-17 15:27:59]

Description was changed from

==========
[Extensions] Remove ExtensionFunction usage of RenderViewHost

RenderViewHost is deprecated. Most ExtensionFunctions were updated
awhile back, but there were still some lingering ones. It looks like
one more (zooming in the file manager) can be safely removed.

BUG=498017
==========

to

==========
[Extensions] Remove ExtensionFunction usage of RenderViewHost

RenderViewHost is deprecated. Most ExtensionFunctions were updated
awhile back, but there were still some lingering ones. It looks like
one more (zooming in the file manager) can be safely removed.

BUG=498017
==========

[commit-bot: I haz the power, 2016-08-17 15:28:00]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2016-08-17 15:29:23]

Description was changed from

==========
[Extensions] Remove ExtensionFunction usage of RenderViewHost

RenderViewHost is deprecated. Most ExtensionFunctions were updated
awhile back, but there were still some lingering ones. It looks like
one more (zooming in the file manager) can be safely removed.

BUG=498017
==========

to

==========
[Extensions] Remove ExtensionFunction usage of RenderViewHost

RenderViewHost is deprecated. Most ExtensionFunctions were updated
awhile back, but there were still some lingering ones. It looks like
one more (zooming in the file manager) can be safely removed.

BUG=498017

Committed: https://crrev.com/17f4094d5716c5c7b98031103a7d84c1474b28ab
Cr-Commit-Position: refs/heads/master@{#412536}
==========

[commit-bot: I haz the power, 2016-08-17 15:29:25]

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