Remove ViewMsg_SetZoomLevel

content/browser/host_zoom_map_impl.cc

 // Copyright (c) 2012 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/browser/host_zoom_map_impl.h"
 
+#include <algorithm>
 #include <cmath>
 
 #include "base/strings/string_piece.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
+#include "content/browser/frame_host/navigation_entry_impl.h"
 #include "content/browser/renderer_host/render_process_host_impl.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
+#include "content/browser/web_contents/web_contents_impl.h"
 #include "content/common/view_messages.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
 #include "content/public/browser/resource_context.h"
 #include "content/public/common/page_zoom.h"
 #include "net/base/net_util.h"
 
 static const char* kHostZoomMapKeyName = "content_host_zoom_map";
 
 namespace content {
 
 HostZoomMap* HostZoomMap::GetForBrowserContext(BrowserContext* context) {
   HostZoomMapImpl* rv = static_cast<HostZoomMapImpl*>(
       context->GetUserData(kHostZoomMapKeyName));
   if (!rv) {
     rv = new HostZoomMapImpl();
     context->SetUserData(kHostZoomMapKeyName, rv);
   }
   return rv;
 }
 
+// Helper function for setting/getting zoom levels for WebContents without
+// having to import HostZoomMapImpl everywhere.
+double HostZoomMap::GetZoomLevel(const WebContents* web_contents) {
+  HostZoomMapImpl* host_zoom_map = static_cast<HostZoomMapImpl*>(
+      HostZoomMap::GetForBrowserContext(web_contents->GetBrowserContext()));
+  return host_zoom_map->GetZoomLevelForWebContents(*web_contents);
+}
+
+void HostZoomMap::SetZoomLevel(const WebContents* web_contents, double level) {
+  HostZoomMapImpl* host_zoom_map = static_cast<HostZoomMapImpl*>(
+      HostZoomMap::GetForBrowserContext(web_contents->GetBrowserContext()));
+  host_zoom_map->SetZoomLevelForWebContents(*web_contents, level);
+}
+
 HostZoomMapImpl::HostZoomMapImpl()
     : default_zoom_level_(0.0) {
   registrar_.Add(
       this, NOTIFICATION_RENDER_VIEW_HOST_WILL_CLOSE_RENDER_VIEW,
       NotificationService::AllSources());
 }
 
 void HostZoomMapImpl::CopyFrom(HostZoomMap* copy_interface) {
   // This can only be called on the UI thread to avoid deadlocks, otherwise
   //   UI: a.CopyFrom(b);
@@ skipping 140 matching lines @@
 void HostZoomMapImpl::SetDefaultZoomLevel(double level) {
   default_zoom_level_ = level;
 }
 
 scoped_ptr<HostZoomMap::Subscription>
 HostZoomMapImpl::AddZoomLevelChangedCallback(
     const ZoomLevelChangedCallback& callback) {
   return zoom_level_changed_callbacks_.Add(callback);
 }
 
+double HostZoomMapImpl::GetZoomLevelForWebContents(
+    const WebContents& web_contents) const {
+  const WebContentsImpl& web_contents_impl =
+      static_cast<const WebContentsImpl&>(web_contents);

[Peter Kasting, 2014/05/28 22:49:58]

These unconditional downcasts scare me.

The API should be changed so that the functions that do this take a const
WebContentsImpl& directly.

[wjmaclean, 2014/05/29 14:49:45]

I've moved the downcasts to the static accessor functions, but they haven't
really gone away. Since the static functions are implemented in
web_contents_impl.cc, is that OK? The goal is to avoid having to know about the
impl interface at the call sites for the static accessors.

[Peter Kasting, 2014/05/29 18:16:08]

What do we really gain by making those callsites not know about the "impl"
class?  Is the "impl" code defined in a different module than the callsites, so
they don't have access to the header?  If not, it seems OK for them to use it. 
If so, then perhaps we should add the relevant methods to the abstract base, so
we can avoid these downcasts?

Certainly what you have now is better than before, and if this is truly the best
way I'll concede, but it makes me uneasy.

[jam, 2014/05/30 16:22:20]

casting interfaces to objects that are only implemented inside content is
legitimate and an often-used pattern, inherited from the webkit api. it's needed
because the embedder doesn't know about the implementation objects, and it needs
to call us back with pointers to objects that we created.

+  int render_process_id = web_contents_impl.GetRenderProcessHost()->GetID();
+  int routing_id = web_contents_impl.GetRenderViewHost()->GetRoutingID();
+
+  if (UsesTemporaryZoomLevel(render_process_id, routing_id)) {
+    return GetTemporaryZoomLevel(render_process_id, routing_id);
+  } else {

[Peter Kasting, 2014/05/28 22:49:58]

Nit: No else after return

[wjmaclean, 2014/05/29 14:49:45]

Done.

+    GURL url;
+    NavigationEntry* entry =
+        web_contents_impl.GetController().GetLastCommittedEntry();
+    // Since zoom map is updated using rewritten URL, use rewritten URL to get
+    // the zoom level.

[Peter Kasting, 2014/05/28 22:49:58]

(1) I don't know what this comment means.  What's a "rewritten" URL?
(2) This should be above the code that sets |entry|, at least, since that code
is relevant.

[wjmaclean, 2014/05/29 14:49:45]

Done.

In all honesty, I don't even recall writing the comment. But I think the idea is
that the URL representation as stored in the controller is what we originally
used for the key, so that's what we should use for lookup. I've amended the
comment, let me know what you think.

+    url = entry ? entry->GetURL() : GURL::EmptyGURL();

[Peter Kasting, 2014/05/28 22:49:58]

This use of EmptyGURL() is incorrect, and |url| is already empty anyway.  So do
this:

  if (entry)
    url = entry->GetURL();

[wjmaclean, 2014/05/29 14:49:45]

Done.

+    return GetZoomLevelForHostAndScheme(url.scheme(),
+                                        net::GetHostOrSpecFromURL(url));
+  }
+}
+
+void HostZoomMapImpl::SetZoomLevelForWebContents(
+    const WebContents& web_contents,
+    double level) {
+  const WebContentsImpl& web_contents_impl =
+      static_cast<const WebContentsImpl&>(web_contents);
+  int render_process_id = web_contents_impl.GetRenderProcessHost()->GetID();
+  int render_view_id = web_contents_impl.GetRenderViewHost()->GetRoutingID();
+  if (UsesTemporaryZoomLevel(render_process_id, render_view_id)) {
+
+    SetTemporaryZoomLevel(render_process_id, render_view_id, level);
+  } else {
+    SetZoomLevelForHost(
+        net::GetHostOrSpecFromURL(web_contents_impl.GetLastCommittedURL()),
+        level);
+  }
+}
+
+void HostZoomMapImpl::SetZoomLevelForView(int render_process_id,
+                                          int render_view_id,
+                                          double level,
+                                          const std::string& host) {
+  if (UsesTemporaryZoomLevel(render_process_id, render_view_id))
+    SetTemporaryZoomLevel(render_process_id, render_view_id, level);
+  else
+    SetZoomLevelForHost(host, level);
+}
+
+bool HostZoomMapImpl::UsesTemporaryZoomLevel(int render_process_id,
+                                             int render_view_id) const {
+  base::AutoLock auto_lock(lock_);
+  TemporaryZoomLevel zoom_level(render_process_id, render_view_id);

[Peter Kasting, 2014/05/28 22:49:58]

Tiny nit: You can move this line above the lock (just to absolutely minimize the
code inside locks)

[wjmaclean, 2014/05/29 14:49:45]

Done.

+  TemporaryZoomLevelList::const_iterator it = find(
+      temporary_zoom_levels_.begin(), temporary_zoom_levels_.end(), zoom_level);
+  return it != temporary_zoom_levels_.end();
+}
+
+void HostZoomMapImpl::SetUsesTemporaryZoomLevel(
+    int render_process_id,
+    int render_view_id,
+    bool uses_temporary_zoom_settings) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  base::AutoLock auto_lock(lock_);
+  TemporaryZoomLevel zoom_level(
+      render_process_id, render_view_id, default_zoom_level_);

[Peter Kasting, 2014/05/28 22:49:58]

Tiny nit: Again, this can go above the lock

[wjmaclean, 2014/05/29 14:49:45]

Done.

+  TemporaryZoomLevelList::iterator it = find(
+      temporary_zoom_levels_.begin(), temporary_zoom_levels_.end(), zoom_level);
+  if (uses_temporary_zoom_settings) {
+    if (it == temporary_zoom_levels_.end())
+      temporary_zoom_levels_.push_back(zoom_level);
+  } else {
+    if (it != temporary_zoom_levels_.end())

[Peter Kasting, 2014/05/28 22:49:58]

Nit: Can be combined with the else

[wjmaclean, 2014/05/29 14:49:45]

Done.

+      temporary_zoom_levels_.erase(it);
+  }
+}
+
 double HostZoomMapImpl::GetTemporaryZoomLevel(int render_process_id,
                                               int render_view_id) const {
   base::AutoLock auto_lock(lock_);
   for (size_t i = 0; i < temporary_zoom_levels_.size(); ++i) {
     if (temporary_zoom_levels_[i].render_process_id == render_process_id &&
         temporary_zoom_levels_[i].render_view_id == render_view_id) {
       return temporary_zoom_levels_[i].zoom_level;
     }
   }
+
   return 0;
 }
 
 void HostZoomMapImpl::SetTemporaryZoomLevel(int render_process_id,
                                             int render_view_id,
                                             double level) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   {
     base::AutoLock auto_lock(lock_);
     size_t i;
     for (i = 0; i < temporary_zoom_levels_.size(); ++i) {
       if (temporary_zoom_levels_[i].render_process_id == render_process_id &&
           temporary_zoom_levels_[i].render_view_id == render_view_id) {
         if (level) {
           temporary_zoom_levels_[i].zoom_level = level;
         } else {
           temporary_zoom_levels_.erase(temporary_zoom_levels_.begin() + i);
         }
         break;
       }
     }
 
     if (level && i == temporary_zoom_levels_.size()) {
-      TemporaryZoomLevel temp;
-      temp.render_process_id = render_process_id;
-      temp.render_view_id = render_view_id;
-      temp.zoom_level = level;
+      TemporaryZoomLevel temp(render_process_id, render_view_id, level);
       temporary_zoom_levels_.push_back(temp);
     }
   }
 
   HostZoomMap::ZoomLevelChange change;
   change.mode = HostZoomMap::ZOOM_CHANGED_TEMPORARY_ZOOM;
   change.zoom_level = level;
 
   zoom_level_changed_callbacks_.Notify(change);
 }
@@ skipping 19 matching lines @@
     }
     default:
       NOTREACHED() << "Unexpected preference observed.";
   }
 }
 
 HostZoomMapImpl::~HostZoomMapImpl() {
 }
 
 }  // namespace content