Migrate HostZoomMap to live in StoragePartition.

chrome/browser/profiles/profile.cc

Index: chrome/browser/profiles/profile.cc
diff --git a/chrome/browser/profiles/profile.cc b/chrome/browser/profiles/profile.cc
index 06b7d1730e30132d746b7fa17bd0663a557a8385..e960809c3036524e25dd429d5d841a59999dc5b7 100644
--- a/chrome/browser/profiles/profile.cc
+++ b/chrome/browser/profiles/profile.cc
@@ -36,6 +36,8 @@
 #include "extensions/browser/pref_names.h"
 #endif
 
+using content::HostZoomMap;
+
 Profile::Profile()
     : restored_last_session_(false),
       sent_destroyed_notification_(false),
@@ -262,3 +264,37 @@ bool ProfileCompare::operator()(Profile* a, Profile* b) const {
     return false;
   return a->GetOriginalProfile() < b->GetOriginalProfile();
 }
+
+void Profile::TrackZoomLevelsFromParent(Profile* parent) {
+  if (GetProfileType() != INCOGNITO_PROFILE ||

[Fady Samuel, 2014/08/13 19:45:42]

This seems like an assumption that may be violated in the future. Let's make
this a DCHECK instead?

[wjmaclean, 2014/08/14 18:18:21]

Done.

+      parent->GetProfileType() != REGULAR_PROFILE)
+    return;
+
+  // Here we only want to use zoom levels stored in the main-context's default
+  // storage partition. We're not interested in zoom levels in special
+  // partitions, e.g. those used by WebViewGuests.
+  HostZoomMap* host_zoom_map = HostZoomMap::GetForBrowserContext(this);
+  HostZoomMap* parent_host_zoom_map = HostZoomMap::GetForBrowserContext(parent);
+  host_zoom_map->CopyFrom(parent_host_zoom_map);
+  // Observe parenti profile's HostZoomMap changes so they can also be applied
+  // to this profile's HostZoomMap.
+  track_zoom_subscription_ = parent_host_zoom_map->AddZoomLevelChangedCallback(
+      base::Bind(&Profile::TrackZoomLevelChanged, base::Unretained(this)));

[Fady Samuel, 2014/08/13 19:45:42]

I worry about the lifetime implications here. What if "this" is deleted prior to
deleting the parent. Is it possible we end up calling TrackZoomLevelChanged with
an invalid "this"? I would use a WeakPtrFactory and get rid of the unretained.

[wjmaclean, 2014/08/14 18:18:21]

The HostZoomMap lives in a StoragePartitionImpl inside a StoragePartitionImplMap
owned by 'this'. If 'this' is deleted, the HostZoomMap is guaranteed to be
destructed first.

This is essentially code removed from OffTheRecordProfileImpl and moved here.

[Fady Samuel, 2014/08/14 18:56:26]

That doesn't seem to be the case looking at this code.

parent_host_zoom_map is owned by the parent profile, right? Thus "this" does not
own parent_host_zoom_map, and may be destroyed prior to parent_host_zoom_map's
destruction?

[wjmaclean, 2014/08/14 19:12:20]

My mistake, but we're still OK. Wrt to parent's HostZoomMap, notice that
track_zoom_subscription_ is a member var on the child (INCOGNITO) profile. I
assume (as the code did previously) that the parent outlives the child
*routinely*, but since the subscription is destroyed when the child dies, no
reference to 'this' will occur since the subscription has been un-registered
from the parent HostZoomMap.

+}
+
+void Profile::TrackZoomLevelChanged(
+    const HostZoomMap::ZoomLevelChange& change) {
+  HostZoomMap* host_zoom_map = HostZoomMap::GetForBrowserContext(this);
+  switch (change.mode) {
+    case HostZoomMap::ZOOM_CHANGED_TEMPORARY_ZOOM:
+       return;
+    case HostZoomMap::ZOOM_CHANGED_FOR_HOST:
+       host_zoom_map->SetZoomLevelForHost(change.host, change.zoom_level);
+       return;
+    case HostZoomMap::ZOOM_CHANGED_FOR_SCHEME_AND_HOST:
+       host_zoom_map->SetZoomLevelForHostAndScheme(change.scheme,
+           change.host,
+           change.zoom_level);
+       return;
+  }
+}