Migrate HostZoomMap to live in StoragePartition.

chrome/browser/profiles/profile.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 "chrome/browser/profiles/profile.h"
 
 #include <string>
 
 #include "base/prefs/pref_service.h"
 #include "build/build_config.h"
@@ skipping 18 matching lines @@
 #endif
 
 #if defined(OS_ANDROID) && defined(FULL_SAFE_BROWSING)
 #include "chrome/browser/safe_browsing/safe_browsing_service.h"
 #endif
 
 #if defined(ENABLE_EXTENSIONS)
 #include "extensions/browser/pref_names.h"
 #endif
 
+using content::HostZoomMap;
+
 Profile::Profile()
     : restored_last_session_(false),
       sent_destroyed_notification_(false),
       accessibility_pause_level_(0) {
 }
 
 Profile::~Profile() {
 }
 
 // static
@@ skipping 206 matching lines @@
         content::NotificationService::NoDetails());
   }
 }
 
 bool ProfileCompare::operator()(Profile* a, Profile* b) const {
   DCHECK(a && b);
   if (a->IsSameProfile(b))
     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;
+  }
+}