Make options zoom isolated, match embedder zoom on navigation.

Make options zoom isolated, match embedder zoom on navigation.

At present, the initialization of the guest's zoom level in
GuestViewBase::StartTrackingEmbedderZoomLevel() is ineffective since the
guest web contents may not have navigated yet ... this occurs for
example when embedding extension options pages in the settings-
extensions page.

Also, the extension options guest should place its guest into isolated
zoom mode since changes to the zoom of settings-extensions page should
not affect the zoom level of the associated extension.

BUG=468397, 468404
Committed: https://crrev.com/d99eafdd721def5856d083d00008d9fcd3ea312a
Cr-Commit-Position: refs/heads/master@{#322388}

[wjmaclean, 2015-03-25 15:12:39]

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

[wjmaclean, 2015-03-25 15:13:09]

Please take a look and let me know if the approach looks reasonable.

[Fady Samuel, 2015-03-25 21:07:16]

https://codereview.chromium.org/1009193003/diff/20001/extensions/browser/gues...
File extensions/browser/guest_view/guest_view_base.cc (right):

https://codereview.chromium.org/1009193003/diff/20001/extensions/browser/gues...
extensions/browser/guest_view/guest_view_base.cc:239: // after the latter has
handled WebContentsObserver events.
I don't know what this means. What problem is this move solving?

[wjmaclean, 2015-03-25 21:14:33]

https://codereview.chromium.org/1009193003/diff/20001/extensions/browser/gues...
File extensions/browser/guest_view/guest_view_base.cc (right):

https://codereview.chromium.org/1009193003/diff/20001/extensions/browser/gues...
extensions/browser/guest_view/guest_view_base.cc:239: // after the latter has
handled WebContentsObserver events.
On 2015/03/25 21:07:16, Fady Samuel wrote:
> I don't know what this means. What problem is this move solving?

When multiple observers exist for a given WebContents, they are called (e.g. for
functions like DidNavigateMainFrame()) in the order they registered as
observers.

The ZoomController registers as an observer in it's constructor.

If the ZoomController's DidNavigateMainFrame() gets called after
ExtensionOptionsGuest's DidNavigateMainFrame(), then ExtensionOptionsGuest
cannot set ISOLATED_ZOOM_MODE, as that will be reset in ZoomController's
DidNavigateMainFrame().

An alternative would be to add a function to ZoomController to override the
reset behaviour (controlled by a flag in ZoomController), but this seemed easier
since both observers are being registered in the same place.

[Fady Samuel, 2015-03-26 02:43:45]

On 2015/03/25 21:14:33, wjmaclean wrote:
>
https://codereview.chromium.org/1009193003/diff/20001/extensions/browser/gues...
> File extensions/browser/guest_view/guest_view_base.cc (right):
> 
>
https://codereview.chromium.org/1009193003/diff/20001/extensions/browser/gues...
> extensions/browser/guest_view/guest_view_base.cc:239: // after the latter has
> handled WebContentsObserver events.
> On 2015/03/25 21:07:16, Fady Samuel wrote:
> > I don't know what this means. What problem is this move solving?
> 
> When multiple observers exist for a given WebContents, they are called (e.g.
for
> functions like DidNavigateMainFrame()) in the order they registered as
> observers.
> 
> The ZoomController registers as an observer in it's constructor.
> 
> If the ZoomController's DidNavigateMainFrame() gets called after
> ExtensionOptionsGuest's DidNavigateMainFrame(), then ExtensionOptionsGuest
> cannot set ISOLATED_ZOOM_MODE, as that will be reset in ZoomController's
> DidNavigateMainFrame().
> 
> An alternative would be to add a function to ZoomController to override the
> reset behaviour (controlled by a flag in ZoomController), but this seemed
easier
> since both observers are being registered in the same place.

Could you please try to encompass more of those details in the comment? Thanks!
lgtm

[wjmaclean, 2015-03-26 14:24:38]

Patchset #3 (id:40001) has been deleted

[wjmaclean, 2015-03-26 14:27:04]

The CQ bit was checked by wjmaclean@chromium.org

[wjmaclean, 2015-03-26 14:27:05]

The patchset sent to the CQ was uploaded after l-g-t-m from fsamuel@chromium.org
Link to the patchset: https://codereview.chromium.org/1009193003/#ps60001
(title: "Improve comment.")

[commit-bot: I haz the power, 2015-03-26 14:27:35]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1009193003/60001

[commit-bot: I haz the power, 2015-03-26 15:27:31]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2015-03-26 15:28:01]

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