Propagate pageScaleFactor to GuestViews

content/browser/web_contents/web_contents_impl.cc

Index: content/browser/web_contents/web_contents_impl.cc
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
index a938e330d89984cf0a2911855a2209a2f6ea33ba..2eef7f1910bf9b175fa64777c66e7d58b81f30ed 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -413,6 +413,7 @@ WebContentsImpl::WebContentsImpl(BrowserContext* browser_context)
           BrowserAccessibilityStateImpl::GetInstance()->accessibility_mode()),
       audio_stream_monitor_(this),
       virtual_keyboard_requested_(false),
+      page_scale_factor_is_one_(true),
       loading_weak_factory_(this) {
   frame_tree_.SetFrameRemoveListener(
       base::Bind(&WebContentsImpl::OnFrameRemoved,
@@ -637,6 +638,8 @@ bool WebContentsImpl::OnMessageReceived(RenderViewHost* render_view_host,
                         OnDidRunInsecureContent)
     IPC_MESSAGE_HANDLER(ViewHostMsg_GoToEntryAtOffset, OnGoToEntryAtOffset)
     IPC_MESSAGE_HANDLER(ViewHostMsg_UpdateZoomLimits, OnUpdateZoomLimits)
+    IPC_MESSAGE_HANDLER(ViewHostMsg_PageScaleFactorChanged,
+                        OnPageScaleFactorChanged)
     IPC_MESSAGE_HANDLER(ViewHostMsg_EnumerateDirectory, OnEnumerateDirectory)
     IPC_MESSAGE_HANDLER(FrameHostMsg_RegisterProtocolHandler,
                         OnRegisterProtocolHandler)
@@ -2618,8 +2621,8 @@ int WebContentsImpl::GetMaximumZoomPercent() const {
   return maximum_zoom_percent_;
 }
 
-void WebContentsImpl::ResetPageScale() {
-  Send(new ViewMsg_ResetPageScale(GetRoutingID()));
+void WebContentsImpl::SetPageScale(float page_scale_factor) {
+  Send(new ViewMsg_SetPageScale(GetRoutingID(), page_scale_factor));
 }
 
 gfx::Size WebContentsImpl::GetPreferredSize() const {
@@ -3117,6 +3120,28 @@ void WebContentsImpl::OnUpdateZoomLimits(int minimum_percent,
   maximum_zoom_percent_ = maximum_percent;
 }
 
+void WebContentsImpl::OnPageScaleFactorChanged(float page_scale_factor) {
+  bool is_one = page_scale_factor == 1.f;
+  if (is_one != page_scale_factor_is_one_) {
+    page_scale_factor_is_one_ = is_one;
+
+    if (GetSiteInstance()) {
+      HostZoomMapImpl* host_zoom_map =
+          static_cast<HostZoomMapImpl*>(HostZoomMap::Get(GetSiteInstance()));

[Charlie Reis, 2015/10/21 21:37:09]

I'm concerned about this, because there isn't a single SiteInstance for
WebContents anymore (due to out-of-process iframes).  Things like
GetSiteInstance(), GetRenderProcessHost(), and GetRoutingID() generally
shouldn't be called on WebContents, since each RenderFrameHost can have a
different value for them.

In this case, the zoom logic hasn't been updated for OOPIFs yet.  wjmaclean@:
Have you thought about how it will have to change?  Here, it seems like each
RenderViewHost would have to be notified.

I could be ok with landing this change without fixing it for OOPIFs, but we need
a TODO mentioning a bug (maybe 528407?) to fix it.

[wjmaclean, 2015/10/22 15:03:14]

I would like to suggest that we do land this change, with a bug as you suggest.

The entire, current host-zoom infrastructure is predicated on tying zoom level
to a site instance, which related to the notion of an origin.

I suppose, going forward, we might expect the outermost (top-level?) frame's
origin/site-instance to set the zoom level for the entire page, though it's not
entirely obvious to me if that should always be the case. Maybe there will be
cases where we want a nested frame to render with a different zoom?

In all honesty, I haven't really thought about how HostZoomMap will need to
change for OOPIF beyond what I've said above: we need a plan.

All attempts to use methods like HostZoomMap::GetForWebContents() will need to
be revised as part of the change to OOPIF.

In the short term we can change the call here to use
HostZoomMap::GetForWebContents(this) so we will easily catch it when we update
the rest of these calls.

[Fady Samuel, 2015/10/22 15:09:01]

I'm not sure this needs changing for OOPIFs.

Zoom is a property that applies for the whole page including iframes. Zoom is
also a property that stored on a per-origin (really per-SiteInstance) basis. The
zoom of the page is the zoom of the site instance of the top level frame. All
subframes inherit the same zoom level as the top level frame.

[Charlie Reis, 2015/10/22 19:09:25]

Sure.
I think you probably want the zoom to stay the same for all frames in a
WebContents.
Yes, and I think that time is coming soon, since we're getting closer to turning
on OOPIFs in some cases.  I'll start a thread on site-isolation-dev@ for it.
Works for me.

On 2015/10/22 15:09:01, Fady Samuel wrote:
I'm ok with this proposed behavior, but that's not how it works today.  If you
zoom a page with an OOPIF, the OOPIF doesn't match the zoom level of the page. 
We definitely need some changes to make it work.


On 2015/10/22 15:29:02, wjmaclean wrote:
That's a different case, though.  GuestViews will have a separate WebContents
from their embedder page, so they can have separate zoom levels if you want.  I
think you probably want to keep zoom as a WebContents-level setting.

[Kevin McNee - google account, 2015/10/22 20:28:48]

Done.

+
+      if (host_zoom_map && GetRenderProcessHost()) {
+        host_zoom_map->SetPageScaleFactorIsOneForView(
+            GetRenderProcessHost()->GetID(), GetRoutingID(),
+            page_scale_factor_is_one_);
+      }
+    }
+  }
+
+  FOR_EACH_OBSERVER(WebContentsObserver,
+                    observers_,
+                    OnPageScaleFactorChanged(page_scale_factor));
+}
+
 void WebContentsImpl::OnEnumerateDirectory(int request_id,
                                            const base::FilePath& path) {
   if (!delegate_)