Move ZoomController out of ChromeWebViewGuestDelegate.

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 b585170006431b97e906050de3d1d2c45198159c..e0cc57ded7f35c26cce6da3f9107b300574ae96f 100644
--- a/extensions/browser/guest_view/guest_view_base.cc
+++ b/extensions/browser/guest_view/guest_view_base.cc
@@ -6,11 +6,13 @@
 
 #include "base/lazy_instance.h"
 #include "base/strings/utf_string_conversions.h"
+#include "components/ui/zoom/zoom_controller.h"
 #include "content/public/browser/navigation_details.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/common/page_zoom.h"
 #include "content/public/common/url_constants.h"
 #include "extensions/browser/api/extensions_api_client.h"
 #include "extensions/browser/event_router.h"
@@ -101,6 +103,7 @@ class GuestViewBase::OwnerLifetimeObserver : public WebContentsObserver {
       return;
 
     destroyed_ = true;
+    guest_->StopObservingOwnersZoomControllerIfNecessary();

[Fady Samuel, 2014/12/11 23:18:11]

Move this to GuestViewBase::Destroy then you don't need to call it from multiple
call sites.

[wjmaclean, 2014/12/11 23:45:24]

Ok, but then we have to preserver owner_weeb_contents_ until after the call to
Destroy().

[Fady Samuel, 2014/12/12 00:03:46]

OK, that seems fine. I can't think of reason for that to be a problem.

     guest_->EmbedderWillBeDestroyed();
     guest_->owner_web_contents_ = NULL;
     guest_->Destroy();
@@ -146,6 +149,7 @@ GuestViewBase::GuestViewBase(content::BrowserContext* browser_context,
       is_being_destroyed_(false),
       auto_size_enabled_(false),
       is_full_page_plugin_(false),
+      observing_owners_zoom_controller_(false),

[Fady Samuel, 2014/12/11 23:18:12]

Is this necessary?

[wjmaclean, 2014/12/11 23:45:24]

Yes ... I think we need to make sure we can detect if a GVB is ever destructed
with its owners_web_contents_ cleared without removing the observer on the
ZoomController.

[Fady Samuel, 2014/12/12 00:03:46]

GuestViewBase should always have an owner_web_contents_ at this point. After
your move in OwnerLifetimeObserver::Destroy, this should always be the case.

       weak_ptr_factory_(this) {
 }
 
@@ -209,6 +213,9 @@ 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);
+
   // Give the derived class an opportunity to perform additional initialization.
   DidInitialize();
 }
@@ -309,6 +316,9 @@ bool GuestViewBase::IsDragAndDropEnabled() const {
 void GuestViewBase::DidAttach(int guest_proxy_routing_id) {
   opener_lifetime_observer_.reset();
 
+  // Any zoom events from the embedder should be relayed to the guest.
+  StartObservingOwnersZoomController();
+
   // Give the derived class an opportunity to perform some actions.
   DidAttachToEmbedder();
 
@@ -483,6 +493,18 @@ bool GuestViewBase::PreHandleGestureEvent(content::WebContents* source,
 }
 
 GuestViewBase::~GuestViewBase() {
+  StopObservingOwnersZoomControllerIfNecessary();

[Fady Samuel, 2014/12/11 23:18:12]

This should be in Destroy.

[wjmaclean, 2014/12/11 23:45:24]

OK. Then we are *guaranteed* never to call the destructor without call Destroy()
first?

[Fady Samuel, 2014/12/12 00:03:46]

Yes, ~GuestViewBase is protected, and in derived types it's protected. The only
way to delete a GuestViewBase is to call Destroy()

[Fady Samuel, 2014/12/12 00:07:44]

Actually the derived types' destructors are private!

+}
+
+void GuestViewBase::OnZoomChanged(
+    const ui_zoom::ZoomController::ZoomChangedEventData& data) {
+  if (content::ZoomValuesEqual(data.old_zoom_level, data.new_zoom_level))
+    return;
+
+  // When the embedder's zoom level is changed, then we also update the
+  // guest's zoom level to match.
+  ui_zoom::ZoomController::FromWebContents(web_contents())
+      ->SetZoomLevel(data.new_zoom_level);
 }
 
 void GuestViewBase::DispatchEventToEmbedder(Event* event) {
@@ -532,6 +554,32 @@ void GuestViewBase::CompleteInit(const std::string& owner_extension_id,
   callback.Run(guest_web_contents);
 }
 
+void GuestViewBase::StartObservingOwnersZoomController() {
+  ui_zoom::ZoomController* zoom_controller =
+      ui_zoom::ZoomController::FromWebContents(embedder_web_contents());
+  if (zoom_controller) {

[Fady Samuel, 2014/12/11 23:18:11]

Early exit preferred.

[wjmaclean, 2014/12/11 23:45:24]

Done.

+    // Listen to the embedder's zoom changes.
+    zoom_controller->AddObserver(this);

[Fady Samuel, 2014/12/12 00:03:46]

A GuestViewBase's owner/embedder can change during its lifetime. You probably
want to remove the observer here. In fact, now that I think about it, you
probably want to call this in GuestViewBase::WillAttach instead of DidAttach.
That way you can see what the old owner was and remove the observer from the old
owner.

+    observing_owners_zoom_controller_ = true;
+    // Set the guest's initial zoom level to be equal to the embedder's.
+    ui_zoom::ZoomController::FromWebContents(web_contents())
+        ->SetZoomLevel(zoom_controller->GetZoomLevel());
+  }
+}
+
+void GuestViewBase::StopObservingOwnersZoomControllerIfNecessary() {
+  // We use owner_web_contents_ below and not embedder_web_contents() since
+  // we may have been detached by this point.
+  DCHECK(!observing_owners_zoom_controller_ || owner_web_contents_);
+  if (!owner_web_contents_ || !observing_owners_zoom_controller_)
+    return;
+
+  ui_zoom::ZoomController* zoom_controller =
+      ui_zoom::ZoomController::FromWebContents(owner_web_contents_);
+  zoom_controller->RemoveObserver(this);
+  observing_owners_zoom_controller_ = false;
+}
+
 // static
 void GuestViewBase::RegisterGuestViewTypes() {
   AppViewGuest::Register();