Propagate pageScaleFactor to GuestViews

content/renderer/render_frame_impl.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/renderer/render_frame_impl.h"
 
 #include <map>
 #include <string>
 
 #include "base/auto_reset.h"
@@ skipping 2815 matching lines @@
       InternalDocumentStateData::FromDocumentState(document_state);
 
   if (document_state->commit_load_time().is_null())
     document_state->set_commit_load_time(Time::Now());
 
   if (internal_data->must_reset_scroll_and_scale_state()) {
     render_view_->webview()->resetScrollAndScaleState();
     internal_data->set_must_reset_scroll_and_scale_state(false);
   }
 
+  // The browser wants to be notified of the page scale factor upon navigation.
+  if (is_main_frame_)

[Charlie Reis, 2015/10/30 22:29:08]

In patch 6, you had this operation inside a
details.is_navigation_to_different_page() block, which meant that it wouldn't
happen for in-page navigations.  I thought that was intentional, because page
scale wouldn't change for in-page navigations.

Here you're not checking for that.  Should you check for something like
!navigation_state->WasWithinSamePage(), such as the block on line 2812?  Is that
the correct check?  (I'm not sure when page scale can change on a navigation.)

I was also trying to suggest the
internal_data->must_reset_scroll_and_scale_state() block, just based on the
name.  Is there some proper way to know whether the page scale has changed, and
thus we need to send an update?

In that sense, I'm also wondering why Blink isn't calling
pageScaleFactorChanged() directly after a navigation, if it does change in some
cases.  If it did, we might not need to add code in content for this.

+    render_view_->pageScaleFactorChanged();
+
   bool is_new_navigation = commit_type == blink::WebStandardCommit;
   if (is_new_navigation) {
     // We bump our Page ID to correspond with the new session history entry.
     render_view_->page_id_ = render_view_->next_page_id_++;
 
     // Don't update history list values for kSwappedOutURL, since
     // we don't want to forget the entry that was there, and since we will
     // never come back to kSwappedOutURL.  Note that we have to call
     // SendUpdateState and update page_id_ even in this case, so that
     // the current entry gets a state update and so that we don't send a
@@ skipping 2441 matching lines @@
   mojo::ServiceProviderPtr service_provider;
   mojo::URLRequestPtr request(mojo::URLRequest::New());
   request->url = mojo::String::From(url);
   mojo_shell_->ConnectToApplication(request.Pass(), GetProxy(&service_provider),
                                     nullptr, nullptr,
                                     base::Bind(&OnGotContentHandlerID));
   return service_provider.Pass();
 }
 
 }  // namespace content