Add FrameHost mojo service

Add FrameHost mojo service

Adds new frame-level service with one initial method to handle host zoom
level.

This moves zoom level supply from async_resource_handler.cc to being a
request made when render_frame_impl handles a willSendRequest.

The goal of this change is to remove the dependency of
content/browser/loader on the rest of content/browser in particular
here, removing the use of c/b/host_zoom_map_impl.h in
content/browser/loader/async_resource_handler.cc.

BUG=598073, 609607
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/68c6f2ce16d9807b5cb82679099c82c40f39e911
Cr-Commit-Position: refs/heads/master@{#394547}

[scottmg, 2016-05-10 23:49:13]

Description was changed from

==========
Add HostZoomLevel mojo service

BUG=609607
==========

to

==========
Add HostZoomLevel mojo service

BUG=609607
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[scottmg, 2016-05-11 23:46:33]

Description was changed from

==========
Add HostZoomLevel mojo service

BUG=609607
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Add HostZoomLevel mojo service

This moves the zoom level 

BUG=609607
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[scottmg, 2016-05-11 23:47:27]

Description was changed from

==========
Add HostZoomLevel mojo service

This moves the zoom level 

BUG=609607
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Add HostZoomLevel mojo service

This moves zoom level supply from async_resource_handler.cc to being a
request made when render_frame_impl starts a navigation.

BUG=609607
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[scottmg, 2016-05-11 23:50:05]

Description was changed from

==========
Add HostZoomLevel mojo service

This moves zoom level supply from async_resource_handler.cc to being a
request made when render_frame_impl starts a navigation.

BUG=609607
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Add HostZoomLevel mojo service

This moves zoom level supply from async_resource_handler.cc to being a
request made when render_frame_impl starts a navigation.

The goal of this change is to remove the dependency of
content/browser/loader on the rest of content/browser.

BUG=598073,609607
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[scottmg, 2016-05-11 23:50:34]

Description was changed from

==========
Add HostZoomLevel mojo service

This moves zoom level supply from async_resource_handler.cc to being a
request made when render_frame_impl starts a navigation.

The goal of this change is to remove the dependency of
content/browser/loader on the rest of content/browser.

BUG=598073,609607
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Add HostZoomLevel mojo service

This moves zoom level supply from async_resource_handler.cc to being a
request made when render_frame_impl starts a navigation.

The goal of this change is to remove the dependency of
content/browser/loader on the rest of content/browser in particular here,
removing the use of c/b/host_zoom_map_impl.h in
content/browser/loader/async_resource_handler.cc.

BUG=598073,609607
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[scottmg, 2016-05-14 00:50:24]

Description was changed from

==========
Add HostZoomLevel mojo service

This moves zoom level supply from async_resource_handler.cc to being a
request made when render_frame_impl starts a navigation.

The goal of this change is to remove the dependency of
content/browser/loader on the rest of content/browser in particular here,
removing the use of c/b/host_zoom_map_impl.h in
content/browser/loader/async_resource_handler.cc.

BUG=598073,609607
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Add HostZoomLevel mojo service

This moves zoom level supply from async_resource_handler.cc to being a
request made when render_frame_impl handles a willSendRequest.

The goal of this change is to remove the dependency of
content/browser/loader on the rest of content/browser in particular here,
removing the use of c/b/host_zoom_map_impl.h in
content/browser/loader/async_resource_handler.cc.

BUG=598073,609607
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[scottmg, 2016-05-14 02:32:12]

Patchset #12 (id:220001) has been deleted

[scottmg, 2016-05-16 17:51:48]

scottmg@chromium.org changed reviewers:
+ jam@chromium.org, nasko@chromium.org, wjmaclean@chromium.org

[scottmg, 2016-05-16 17:51:49]

jam: for overall review
nasko: for _messages
wjmaclean: I removed one of your TODO to make the test work on Android, just in
case you have any comments/concerns.

[jam, 2016-05-16 18:28:32]

https://codereview.chromium.org/1964273002/diff/260001/content/browser/host_z...
File content/browser/host_zoom_level_impl.h (right):

https://codereview.chromium.org/1964273002/diff/260001/content/browser/host_z...
content/browser/host_zoom_level_impl.h:19: int render_process_id,
these two parameters are redundant, since one can get to BrowserContext from
render_process_id.

Can you just pass in render_process_id, and then when you need the
BHostZoomMapImpl you can do:
RenderProcessHost::FromID(render_process_id)->GetStoragePartition()->GetHostZoomMap()

https://codereview.chromium.org/1964273002/diff/260001/content/common/host_zo...
File content/common/host_zoom_level.mojom (right):

https://codereview.chromium.org/1964273002/diff/260001/content/common/host_zo...
content/common/host_zoom_level.mojom:10: GetZoomLevel(url.mojom.Url url, int32
render_view_id)
we eventually want to get rid of routing IDs. 

Can we make this service per-frame instead of per-process? then we don't need to
pass routing IDs.

[wjmaclean, 2016-05-16 18:53:07]

https://codereview.chromium.org/1964273002/diff/260001/content/browser/loader...
File content/browser/loader/async_resource_handler.cc (left):

https://codereview.chromium.org/1964273002/diff/260001/content/browser/loader...
content/browser/loader/async_resource_handler.cc:347: const HostZoomMapImpl*
host_zoom_map =
If I remember correctly, this is the only consumer for
ResourceMessageFilter::GetHostZoomMap(), so you should trace back through that
code dependency and remove it.

https://codereview.chromium.org/1964273002/diff/260001/content/renderer/rende...
File content/renderer/render_frame_impl.h (right):

https://codereview.chromium.org/1964273002/diff/260001/content/renderer/rende...
content/renderer/render_frame_impl.h:1188: mojom::HostZoomLevelPtr
host_zoom_level_;
nit: these names are *very* similar and might be confusing to someone reading
the code ... maybe make host_zoom_levels_ something less similar?

https://codereview.chromium.org/1964273002/diff/260001/content/renderer/rende...
File content/renderer/render_view_impl.cc (left):

https://codereview.chromium.org/1964273002/diff/260001/content/renderer/rende...
content/renderer/render_view_impl.cc:2414: void
RenderViewImpl::OnSetZoomLevelForLoadingURL(const GURL& url,
Happy to see this go, actually :-)

[wjmaclean, 2016-05-16 18:59:53]

https://codereview.chromium.org/1964273002/diff/260001/content/common/host_zo...
File content/common/host_zoom_level.mojom (right):

https://codereview.chromium.org/1964273002/diff/260001/content/common/host_zo...
content/common/host_zoom_level.mojom:10: GetZoomLevel(url.mojom.Url url, int32
render_view_id)
On 2016/05/16 18:28:32, jam wrote:
> we eventually want to get rid of routing IDs. 
> 
> Can we make this service per-frame instead of per-process? then we don't need
to
> pass routing IDs.

Sorry, just noticed this comment.

The main concern here I think is whether the zoom level is based on the URL as
stored in the HostZoomMap, or to a view/page in which the normal zoom level is
overriden (TemporaryZoomLevel). In either case I think it's fine to think of the
zoom level as being tied to the frame, since any frame will be tied to a
particular view from which a zoom level can be found. If the frame is top-level,
then its preferred zoom level is dictated by that of the main frame anyways.

[Ben Goodger (Google), 2016-05-16 19:14:41]

On 2016/05/16 17:51:49, scottmg wrote:
> jam: for overall review
> nasko: for _messages
> wjmaclean: I removed one of your TODO to make the test work on Android, just
in
> case you have any comments/concerns.

Drive-by:

With full mojo-ification of the view interfaces, it's not clear to me that we'll
want a separate HostZoom interface. i.e. I think it might be reasonable to
assert that all views are zoomable. I want to avoid a proliferation of
interfaces for single IPCs since I think that could get out of hand.

With that said, there's no View mojom interface yet... so perhaps it's OK to do
this. 

One question: do you think it'd be possible to implement  on an object
associated with one of the browser side view hosts instead of the render process
host? If that were possible, we wouldn't have to pass routing ids around... as
jam mentions those should go away once the view interfaces are mojoified.

[scottmg, 2016-05-16 20:23:51]

Patchset #14 (id:280001) has been deleted

[scottmg, 2016-05-16 21:56:03]

Patchset #15 (id:320001) has been deleted

[scottmg, 2016-05-16 22:54:44]

Thanks all, routing id gone now.

On 2016/05/16 19:14:41, Ben Goodger (Google) wrote:
> On 2016/05/16 17:51:49, scottmg wrote:
> > jam: for overall review
> > nasko: for _messages
> > wjmaclean: I removed one of your TODO to make the test work on Android, just
> in
> > case you have any comments/concerns.
> 
> Drive-by:
> 
> With full mojo-ification of the view interfaces, it's not clear to me that
we'll
> want a separate HostZoom interface. i.e. I think it might be reasonable to
> assert that all views are zoomable. I want to avoid a proliferation of
> interfaces for single IPCs since I think that could get out of hand.
> 
> With that said, there's no View mojom interface yet... so perhaps it's OK to
do
> this. 
> 
> One question: do you think it'd be possible to implement  on an object
> associated with one of the browser side view hosts instead of the render
process
> host? If that were possible, we wouldn't have to pass routing ids around... as
> jam mentions those should go away once the view interfaces are mojoified.

Not sure if I understood this, do you mean making the mojom the beginnings of a
"FrameService" with one method GetZoomLevel(), instead of the very specific
"HostZoomLevel" mojom?

[scottmg, 2016-05-16 22:54:56]

https://codereview.chromium.org/1964273002/diff/260001/content/browser/host_z...
File content/browser/host_zoom_level_impl.h (right):

https://codereview.chromium.org/1964273002/diff/260001/content/browser/host_z...
content/browser/host_zoom_level_impl.h:19: int render_process_id,
On 2016/05/16 18:28:32, jam wrote:
> these two parameters are redundant, since one can get to BrowserContext from
> render_process_id.
> 
> Can you just pass in render_process_id, and then when you need the
> BHostZoomMapImpl you can do:
>
RenderProcessHost::FromID(render_process_id)->GetStoragePartition()->GetHostZoomMap()

Done.

https://codereview.chromium.org/1964273002/diff/260001/content/browser/loader...
File content/browser/loader/async_resource_handler.cc (left):

https://codereview.chromium.org/1964273002/diff/260001/content/browser/loader...
content/browser/loader/async_resource_handler.cc:347: const HostZoomMapImpl*
host_zoom_map =
On 2016/05/16 18:53:07, wjmaclean wrote:
> If I remember correctly, this is the only consumer for
> ResourceMessageFilter::GetHostZoomMap(), so you should trace back through that
> code dependency and remove it. 

Done.

https://codereview.chromium.org/1964273002/diff/260001/content/common/host_zo...
File content/common/host_zoom_level.mojom (right):

https://codereview.chromium.org/1964273002/diff/260001/content/common/host_zo...
content/common/host_zoom_level.mojom:10: GetZoomLevel(url.mojom.Url url, int32
render_view_id)
On 2016/05/16 18:59:53, wjmaclean wrote:
> On 2016/05/16 18:28:32, jam wrote:
> > we eventually want to get rid of routing IDs. 
> > 
> > Can we make this service per-frame instead of per-process? then we don't
need
> to
> > pass routing IDs.
> 
> Sorry, just noticed this comment.
> 
> The main concern here I think is whether the zoom level is based on the URL as
> stored in the HostZoomMap, or to a view/page in which the normal zoom level is
> overriden (TemporaryZoomLevel). In either case I think it's fine to think of
the
> zoom level as being tied to the frame, since any frame will be tied to a
> particular view from which a zoom level can be found. If the frame is
top-level,
> then its preferred zoom level is dictated by that of the main frame anyways.

OK, thanks. Moved to the frame.

https://codereview.chromium.org/1964273002/diff/260001/content/renderer/rende...
File content/renderer/render_frame_impl.h (right):

https://codereview.chromium.org/1964273002/diff/260001/content/renderer/rende...
content/renderer/render_frame_impl.h:1188: mojom::HostZoomLevelPtr
host_zoom_level_;
On 2016/05/16 18:53:07, wjmaclean wrote:
> nit: these names are *very* similar and might be confusing to someone reading
> the code ... maybe make host_zoom_levels_ something less similar?

How about host_zoom_levels_ and mojo_host_zoom_level_? Not great...

[scottmg, 2016-05-17 00:11:50]

Description was changed from

==========
Add HostZoomLevel mojo service

This moves zoom level supply from async_resource_handler.cc to being a
request made when render_frame_impl handles a willSendRequest.

The goal of this change is to remove the dependency of
content/browser/loader on the rest of content/browser in particular here,
removing the use of c/b/host_zoom_map_impl.h in
content/browser/loader/async_resource_handler.cc.

BUG=598073,609607
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Add FrameService mojo service

This moves zoom level supply from async_resource_handler.cc to being a
request made when render_frame_impl handles a willSendRequest.

The goal of this change is to remove the dependency of
content/browser/loader on the rest of content/browser in particular here,
removing the use of c/b/host_zoom_map_impl.h in
content/browser/loader/async_resource_handler.cc.

BUG=598073,609607
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[scottmg, 2016-05-17 00:13:08]

Description was changed from

==========
Add FrameService mojo service

This moves zoom level supply from async_resource_handler.cc to being a
request made when render_frame_impl handles a willSendRequest.

The goal of this change is to remove the dependency of
content/browser/loader on the rest of content/browser in particular here,
removing the use of c/b/host_zoom_map_impl.h in
content/browser/loader/async_resource_handler.cc.

BUG=598073,609607
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Add FrameService mojo service

Adds new frame-level service with one initial method to handle host zoom
level.

This moves zoom level supply from async_resource_handler.cc to being a
request made when render_frame_impl handles a willSendRequest.

The goal of this change is to remove the dependency of
content/browser/loader on the rest of content/browser in particular
here, removing the use of c/b/host_zoom_map_impl.h in
content/browser/loader/async_resource_handler.cc.

BUG=598073,609607
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[scottmg, 2016-05-17 00:16:26]

OK, renamed from HostZoomLevel.mojom to FrameService.mojom, and the method from
GetZoomLevel() to GetHostZoomLevel(). Let me know if you had a better name in
mind.

(This also resolves the concern over names of the members in RenderFrameImpl.)

[wjmaclean, 2016-05-17 14:27:17]

On 2016/05/17 00:16:26, scottmg wrote:
> OK, renamed from HostZoomLevel.mojom to FrameService.mojom, and the method
from
> GetZoomLevel() to GetHostZoomLevel(). Let me know if you had a better name in
> mind.
> 
> (This also resolves the concern over names of the members in RenderFrameImpl.)

This seems fine, thanks! lgtm

[Ben Goodger (Google), 2016-05-17 14:44:50]

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

[Ben Goodger (Google), 2016-05-17 14:44:52]

https://codereview.chromium.org/1964273002/diff/410001/content/browser/frame_...
File content/browser/frame_service_impl.h (right):

https://codereview.chromium.org/1964273002/diff/410001/content/browser/frame_...
content/browser/frame_service_impl.h:16: class FrameServiceImpl : public
mojom::FrameService {
can you have RenderFrameHostImpl just implement this interface directly?

https://codereview.chromium.org/1964273002/diff/410001/content/common/frame_s...
File content/common/frame_service.mojom (right):

https://codereview.chromium.org/1964273002/diff/410001/content/common/frame_s...
content/common/frame_service.mojom:9: interface FrameService {
If you're going to create this, how about calling it FrameHost, corresponding to
what the classes/_messages refer to it as?

[scottmg, 2016-05-17 17:10:20]

Description was changed from

==========
Add FrameService mojo service

Adds new frame-level service with one initial method to handle host zoom
level.

This moves zoom level supply from async_resource_handler.cc to being a
request made when render_frame_impl handles a willSendRequest.

The goal of this change is to remove the dependency of
content/browser/loader on the rest of content/browser in particular
here, removing the use of c/b/host_zoom_map_impl.h in
content/browser/loader/async_resource_handler.cc.

BUG=598073,609607
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Add FrameHost mojo service

Adds new frame-level service with one initial method to handle host zoom
level.

This moves zoom level supply from async_resource_handler.cc to being a
request made when render_frame_impl handles a willSendRequest.

The goal of this change is to remove the dependency of
content/browser/loader on the rest of content/browser in particular
here, removing the use of c/b/host_zoom_map_impl.h in
content/browser/loader/async_resource_handler.cc.

BUG=598073,609607
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[jam, 2016-05-17 17:20:30]

https://codereview.chromium.org/1964273002/diff/410001/content/renderer/rende...
File content/renderer/render_view_impl.cc (left):

https://codereview.chromium.org/1964273002/diff/410001/content/renderer/rende...
content/renderer/render_view_impl.cc:2385: #if !defined(OS_ANDROID)
hmm maybe best to keep android behavior same in this cl, and change that in a
different cl if you want? that way if there's any regressions, it's a simple cl
to revert.

[scottmg, 2016-05-17 17:36:16]

Thanks

https://codereview.chromium.org/1964273002/diff/410001/content/browser/frame_...
File content/browser/frame_service_impl.h (right):

https://codereview.chromium.org/1964273002/diff/410001/content/browser/frame_...
content/browser/frame_service_impl.h:16: class FrameServiceImpl : public
mojom::FrameService {
On 2016/05/17 14:44:51, Ben Goodger (Google) wrote:
> can you have RenderFrameHostImpl just implement this interface directly?

Done.

https://codereview.chromium.org/1964273002/diff/410001/content/common/frame_s...
File content/common/frame_service.mojom (right):

https://codereview.chromium.org/1964273002/diff/410001/content/common/frame_s...
content/common/frame_service.mojom:9: interface FrameService {
On 2016/05/17 14:44:51, Ben Goodger (Google) wrote:
> If you're going to create this, how about calling it FrameHost, corresponding
to
> what the classes/_messages refer to it as?

Done.

https://codereview.chromium.org/1964273002/diff/410001/content/renderer/rende...
File content/renderer/render_view_impl.cc (left):

https://codereview.chromium.org/1964273002/diff/410001/content/renderer/rende...
content/renderer/render_view_impl.cc:2385: #if !defined(OS_ANDROID)
On 2016/05/17 17:20:30, jam wrote:
> hmm maybe best to keep android behavior same in this cl, and change that in a
> different cl if you want? that way if there's any regressions, it's a simple
cl
> to revert.

Yeah, I only enabled it because it made the test work. I couldn't understand why
just moving it from here to render_frame_impl_browsertest made it start failing,
so after a while I gave up and enabled this code.

But I had forgotten that Android uses the crazy out-of-source test disabling :/
https://code.google.com/p/chromium/codesearch#chromium/src/build/android/pyli...

So... re-disabled on OS_ANDROID, and changed the test disable thingy in
build/android.

[Ben Goodger (Google), 2016-05-17 19:38:03]

On 2016/05/17 17:36:16, scottmg wrote:
> Thanks
> 
>
https://codereview.chromium.org/1964273002/diff/410001/content/browser/frame_...
> File content/browser/frame_service_impl.h (right):
> 
>
https://codereview.chromium.org/1964273002/diff/410001/content/browser/frame_...
> content/browser/frame_service_impl.h:16: class FrameServiceImpl : public
> mojom::FrameService {
> On 2016/05/17 14:44:51, Ben Goodger (Google) wrote:
> > can you have RenderFrameHostImpl just implement this interface directly?
> 
> Done.
> 
>
https://codereview.chromium.org/1964273002/diff/410001/content/common/frame_s...
> File content/common/frame_service.mojom (right):
> 
>
https://codereview.chromium.org/1964273002/diff/410001/content/common/frame_s...
> content/common/frame_service.mojom:9: interface FrameService {
> On 2016/05/17 14:44:51, Ben Goodger (Google) wrote:
> > If you're going to create this, how about calling it FrameHost,
corresponding
> to
> > what the classes/_messages refer to it as?
> 
> Done.
> 
>
https://codereview.chromium.org/1964273002/diff/410001/content/renderer/rende...
> File content/renderer/render_view_impl.cc (left):
> 
>
https://codereview.chromium.org/1964273002/diff/410001/content/renderer/rende...
> content/renderer/render_view_impl.cc:2385: #if !defined(OS_ANDROID)
> On 2016/05/17 17:20:30, jam wrote:
> > hmm maybe best to keep android behavior same in this cl, and change that in
a
> > different cl if you want? that way if there's any regressions, it's a simple
> cl
> > to revert.
> 
> Yeah, I only enabled it because it made the test work. I couldn't understand
why
> just moving it from here to render_frame_impl_browsertest made it start
failing,
> so after a while I gave up and enabled this code.
> 
> But I had forgotten that Android uses the crazy out-of-source test disabling
:/
>
https://code.google.com/p/chromium/codesearch#chromium/src/build/android/pyli...
> 
> So... re-disabled on OS_ANDROID, and changed the test disable thingy in
> build/android.

LGTM from my perspective!

[jam, 2016-05-17 19:43:22]

lgtm

[scottmg, 2016-05-17 20:16:29]

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

[scottmg, 2016-05-17 20:16:30]

->dcheng for content/common/view_messages.h as nasko is out.

[dcheng, 2016-05-18 04:46:18]

https://codereview.chromium.org/1964273002/diff/470001/content/browser/frame_...
File content/browser/frame_host/render_frame_host_impl.cc (right):

https://codereview.chromium.org/1964273002/diff/470001/content/browser/frame_...
content/browser/frame_host/render_frame_host_impl.cc:2577:
callback.Run(std::move(url), zoom_level);
std::move() here is a no-op, since |url| is a constant reference.

https://codereview.chromium.org/1964273002/diff/470001/content/renderer/rende...
File content/renderer/render_frame_impl.cc (right):

https://codereview.chromium.org/1964273002/diff/470001/content/renderer/rende...
content/renderer/render_frame_impl.cc:3944: weak_factory_.GetWeakPtr()));
Why pass the URL through like this? Seems like we could just base::Bind() it on
this end?

[scottmg, 2016-05-18 18:31:44]

https://codereview.chromium.org/1964273002/diff/470001/content/browser/frame_...
File content/browser/frame_host/render_frame_host_impl.cc (right):

https://codereview.chromium.org/1964273002/diff/470001/content/browser/frame_...
content/browser/frame_host/render_frame_host_impl.cc:2577:
callback.Run(std::move(url), zoom_level);
On 2016/05/18 04:46:17, dcheng wrote:
> std::move() here is a no-op, since |url| is a constant reference.

Oops, it used to be something more complicated before I realized how typemaps
worked. But removed per other suggestion.

https://codereview.chromium.org/1964273002/diff/470001/content/renderer/rende...
File content/renderer/render_frame_impl.cc (right):

https://codereview.chromium.org/1964273002/diff/470001/content/renderer/rende...
content/renderer/render_frame_impl.cc:3944: weak_factory_.GetWeakPtr()));
On 2016/05/18 04:46:17, dcheng wrote:
> Why pass the URL through like this? Seems like we could just base::Bind() it
on
> this end?

I was sort of thinking of a message protocol where it'd be weird to get a
disembodied float response. But totally makes sense here, done.

[dcheng, 2016-05-18 19:46:46]

lgtm

[scottmg, 2016-05-18 19:51:22]

The CQ bit was checked by scottmg@chromium.org

[scottmg, 2016-05-18 19:51:22]

The patchset sent to the CQ was uploaded after l-g-t-m from
wjmaclean@chromium.org, jam@chromium.org, ben@chromium.org
Link to the patchset: https://codereview.chromium.org/1964273002/#ps490001
(title: ".")

[commit-bot: I haz the power, 2016-05-18 19:52:22]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1964273002/490001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1964273002/490001

[commit-bot: I haz the power, 2016-05-18 21:10:49]

Description was changed from

==========
Add FrameHost mojo service

Adds new frame-level service with one initial method to handle host zoom
level.

This moves zoom level supply from async_resource_handler.cc to being a
request made when render_frame_impl handles a willSendRequest.

The goal of this change is to remove the dependency of
content/browser/loader on the rest of content/browser in particular
here, removing the use of c/b/host_zoom_map_impl.h in
content/browser/loader/async_resource_handler.cc.

BUG=598073,609607
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Add FrameHost mojo service

Adds new frame-level service with one initial method to handle host zoom
level.

This moves zoom level supply from async_resource_handler.cc to being a
request made when render_frame_impl handles a willSendRequest.

The goal of this change is to remove the dependency of
content/browser/loader on the rest of content/browser in particular
here, removing the use of c/b/host_zoom_map_impl.h in
content/browser/loader/async_resource_handler.cc.

BUG=598073,609607
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[commit-bot: I haz the power, 2016-05-18 21:10:51]

Committed patchset #23 (id:490001)

[commit-bot: I haz the power, 2016-05-18 21:11:48]

Description was changed from

==========
Add FrameHost mojo service

Adds new frame-level service with one initial method to handle host zoom
level.

This moves zoom level supply from async_resource_handler.cc to being a
request made when render_frame_impl handles a willSendRequest.

The goal of this change is to remove the dependency of
content/browser/loader on the rest of content/browser in particular
here, removing the use of c/b/host_zoom_map_impl.h in
content/browser/loader/async_resource_handler.cc.

BUG=598073,609607
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Add FrameHost mojo service

Adds new frame-level service with one initial method to handle host zoom
level.

This moves zoom level supply from async_resource_handler.cc to being a
request made when render_frame_impl handles a willSendRequest.

The goal of this change is to remove the dependency of
content/browser/loader on the rest of content/browser in particular
here, removing the use of c/b/host_zoom_map_impl.h in
content/browser/loader/async_resource_handler.cc.

BUG=598073,609607
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/68c6f2ce16d9807b5cb82679099c82c40f39e911
Cr-Commit-Position: refs/heads/master@{#394547}
==========

[commit-bot: I haz the power, 2016-05-18 21:11:50]

Patchset 23 (id:??) landed as
https://crrev.com/68c6f2ce16d9807b5cb82679099c82c40f39e911
Cr-Commit-Position: refs/heads/master@{#394547}

[scottmg, 2016-05-24 22:47:24]

A revert of this CL (patchset #23 id:490001) has been created in
https://codereview.chromium.org/2007203002/ by scottmg@chromium.org.

The reason for reverting is: Async request for zoom level doesn't work in all
cases https://crbug.com/614348 https://crbug.com/613979.

I thought
https://www.chromium.org/developers/design-documents/mojo/chrome-ipc-to-mojo-...
implied the ordering would be correct, but it seems that was too hopeful..