<webview>: Support zoom in WebUI

<webview>: Support zoom in WebUI

This CL plumbs the embedder's zoom into the guest WebContents.

BUG=330818
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=244116

[nasko, 2014-01-06 21:58:40]

This looks mostly good, just a few small issues.

https://codereview.chromium.org/98073015/diff/40001/content/browser/browser_p...
File content/browser/browser_plugin/browser_plugin_guest_manager.cc (right):

https://codereview.chromium.org/98073015/diff/40001/content/browser/browser_p...
content/browser/browser_plugin/browser_plugin_guest_manager.cc:232: it !=
guest_web_contents_by_instance_id_.end(); ++it) {
nit: indent should match GuestInstanceMap.

https://codereview.chromium.org/98073015/diff/40001/content/browser/browser_p...
File content/browser/browser_plugin/browser_plugin_guest_manager.h (right):

https://codereview.chromium.org/98073015/diff/40001/content/browser/browser_p...
content/browser/browser_plugin/browser_plugin_guest_manager.h:91: void
GetGuests(WebContentsImpl* embedder_web_contents,
This method name doesn't make much sense. Maybe "ForEachGuest"? We use that
pattern elsewhere too.

https://codereview.chromium.org/98073015/diff/40001/content/public/browser/we...
File content/public/browser/web_contents.h (right):

https://codereview.chromium.org/98073015/diff/40001/content/public/browser/we...
content/public/browser/web_contents.h:485: // Sets the zoom level for the
current main frame.
Is this for the main frame only or for the full document displaying in this
WebContents?

[Fady Samuel, 2014-01-08 19:42:42]

PTAL

https://codereview.chromium.org/98073015/diff/40001/content/browser/browser_p...
File content/browser/browser_plugin/browser_plugin_guest_manager.cc (right):

https://codereview.chromium.org/98073015/diff/40001/content/browser/browser_p...
content/browser/browser_plugin/browser_plugin_guest_manager.cc:232: it !=
guest_web_contents_by_instance_id_.end(); ++it) {
On 2014/01/06 21:58:40, nasko wrote:
> nit: indent should match GuestInstanceMap.

Done.

https://codereview.chromium.org/98073015/diff/40001/content/browser/browser_p...
File content/browser/browser_plugin/browser_plugin_guest_manager.h (right):

https://codereview.chromium.org/98073015/diff/40001/content/browser/browser_p...
content/browser/browser_plugin/browser_plugin_guest_manager.h:91: void
GetGuests(WebContentsImpl* embedder_web_contents,
On 2014/01/06 21:58:40, nasko wrote:
> This method name doesn't make much sense. Maybe "ForEachGuest"? We use that
> pattern elsewhere too.

Done.

https://codereview.chromium.org/98073015/diff/40001/content/public/browser/we...
File content/public/browser/web_contents.h (right):

https://codereview.chromium.org/98073015/diff/40001/content/public/browser/we...
content/public/browser/web_contents.h:485: // Sets the zoom level for the
current main frame.
On 2014/01/06 21:58:40, nasko wrote:
> Is this for the main frame only or for the full document displaying in this
> WebContents?

That depends on your definition of zoom. This zooms the main frame and all
subframes stay at 1.0 RELATIVE zoom. Guests are separate processes so we need to
update zoom there too. I add a small clarification in the comment.

[nasko, 2014-01-08 21:59:05]

LGTM with nits.

https://codereview.chromium.org/98073015/diff/140001/content/browser/browser_...
File content/browser/browser_plugin/browser_plugin_embedder.cc (right):

https://codereview.chromium.org/98073015/diff/140001/content/browser/browser_...
content/browser/browser_plugin/browser_plugin_embedder.cc:125: return false;
nit: The comment you had in the other CL was useful, add one here as well.

https://codereview.chromium.org/98073015/diff/140001/content/public/browser/w...
File content/public/browser/web_contents.h (right):

https://codereview.chromium.org/98073015/diff/140001/content/public/browser/w...
content/public/browser/web_contents.h:485: // Sets the zoom level for the
current main frame and all BrowserPluginGuests
nit: My point was that it affects all frames, not just the main one. Maybe say
"for the current page and all ...",

[Fady Samuel, 2014-01-09 16:19:45]

+jam@ for content OWNER review. Thanks!

https://codereview.chromium.org/98073015/diff/140001/content/browser/browser_...
File content/browser/browser_plugin/browser_plugin_embedder.cc (right):

https://codereview.chromium.org/98073015/diff/140001/content/browser/browser_...
content/browser/browser_plugin/browser_plugin_embedder.cc:125: return false;
On 2014/01/08 21:59:06, nasko wrote:
> nit: The comment you had in the other CL was useful, add one here as well.

Done.

https://codereview.chromium.org/98073015/diff/140001/content/public/browser/w...
File content/public/browser/web_contents.h (right):

https://codereview.chromium.org/98073015/diff/140001/content/public/browser/w...
content/public/browser/web_contents.h:485: // Sets the zoom level for the
current main frame and all BrowserPluginGuests
On 2014/01/08 21:59:06, nasko wrote:
> nit: My point was that it affects all frames, not just the main one. Maybe say
> "for the current page and all ...",

Done.

[jam, 2014-01-09 21:03:33]

lgtm

[commit-bot: I haz the power, 2014-01-09 21:47:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fsamuel@chromium.org/98073015/290001

[commit-bot: I haz the power, 2014-01-09 22:26:38]

Retried try job too often on linux_rel for step(s) base_unittests,
browser_tests, interactive_ui_tests, net_unittests, unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2014-01-10 04:25:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/fsamuel@chromium.org/98073015/500001

[commit-bot: I haz the power, 2014-01-10 07:15:54]

Change committed as 244116