Make options zoom isolated, match embedder zoom on navigation.

extensions/browser/guest_view/guest_view_base.cc

Index: extensions/browser/guest_view/guest_view_base.cc
diff --git a/extensions/browser/guest_view/guest_view_base.cc b/extensions/browser/guest_view/guest_view_base.cc
index eefe4bb0758b900fe5187bc7e3100c6a92049011..1652bc884a80531e6bb38333ca1f79aeb1c44eca 100644
--- a/extensions/browser/guest_view/guest_view_base.cc
+++ b/extensions/browser/guest_view/guest_view_base.cc
@@ -233,6 +233,12 @@ void GuestViewBase::InitWithWebContents(
     content::WebContents* guest_web_contents) {
   DCHECK(guest_web_contents);
 
+  // Create a ZoomController to allow the guest's contents to be zoomed.
+  // Do this before adding the GuestView as a WebContents Observer so that
+  // the GuestView and its derived classes can re-configure the ZoomController
+  // after the latter has handled WebContentsObserver events.

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

I don't know what this means. What problem is this move solving?

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

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.

+  ui_zoom::ZoomController::CreateForWebContents(guest_web_contents);
+
   // At this point, we have just created the guest WebContents, we need to add
   // an observer to the owner WebContents. This observer will be responsible
   // for destroying the guest WebContents if the owner goes away.
@@ -246,9 +252,6 @@ void GuestViewBase::InitWithWebContents(
   GuestViewManager::FromBrowserContext(browser_context_)->
       AddGuest(guest_instance_id_, guest_web_contents);
 
-  // Create a ZoomController to allow the guest's contents to be zoomed.
-  ui_zoom::ZoomController::CreateForWebContents(guest_web_contents);
-
   // Populate the view instance ID if we have it on creation.
   create_params.GetInteger(guestview::kParameterInstanceId,
                            &view_instance_id_);
@@ -576,6 +579,13 @@ void GuestViewBase::WebContentsDestroyed() {
   delete this;
 }
 
+void GuestViewBase::DidNavigateMainFrame(
+    const content::LoadCommittedDetails& details,
+    const content::FrameNavigateParams& params) {
+  if (attached())
+    SetGuestZoomLevelToMatchEmbedder();
+}
+
 void GuestViewBase::ActivateContents(WebContents* web_contents) {
   if (!attached() || !embedder_web_contents()->GetDelegate())
     return;
@@ -815,6 +825,16 @@ void GuestViewBase::SetUpSizing(const base::DictionaryValue& params) {
   SetSize(set_size_params);
 }
 
+void GuestViewBase::SetGuestZoomLevelToMatchEmbedder() {
+  auto embedder_zoom_controller =
+      ui_zoom::ZoomController::FromWebContents(owner_web_contents());
+  if (!embedder_zoom_controller)
+    return;
+
+  ui_zoom::ZoomController::FromWebContents(web_contents())
+      ->SetZoomLevel(embedder_zoom_controller->GetZoomLevel());
+}
+
 void GuestViewBase::StartTrackingEmbedderZoomLevel() {
   if (!ZoomPropagatesFromEmbedderToGuest())
     return;
@@ -826,9 +846,9 @@ void GuestViewBase::StartTrackingEmbedderZoomLevel() {
     return;
   // Listen to the embedder's zoom changes.
   embedder_zoom_controller->AddObserver(this);
+
   // Set the guest's initial zoom level to be equal to the embedder's.
-  ui_zoom::ZoomController::FromWebContents(web_contents())
-      ->SetZoomLevel(embedder_zoom_controller->GetZoomLevel());
+  SetGuestZoomLevelToMatchEmbedder();
 }
 
 void GuestViewBase::StopTrackingEmbedderZoomLevel() {