Migrate HostZoomMap to live in StoragePartition.

content/browser/host_zoom_level_context.h

+// Copyright (c) 2014 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.
+
+#ifndef CONTENT_BROWSER_HOST_ZOOM_LEVEL_CONTEXT

[Fady Samuel, 2014/08/15 15:03:01]

CONTENT_BROWSER_HOST_ZOOM_LEVEL_CONTEXT_H_ here and elsewhere.

[wjmaclean, 2014/08/15 22:13:50]

Already fixed in WIP patch.

+#define CONTENT_BROWSER_HOST_ZOOM_LEVEL_CONTEXT
+
+#include <string>

[Fady Samuel, 2014/08/15 15:03:01]

Is this necessary?

[wjmaclean, 2014/08/15 22:13:50]

No, I think it's a debugging artifact ... removed.

+
+#include "base/files/file_path.h"

[Fady Samuel, 2014/08/15 15:03:01]

Is this necesary? You can probably forward declare.

[wjmaclean, 2014/08/15 22:13:50]

Again, debugging artifact, thanks for catching it.

+#include "base/macros.h"
+#include "base/memory/ref_counted.h"

[Fady Samuel, 2014/08/15 15:03:01]

Is this necessary?

[wjmaclean, 2014/08/15 22:13:50]

Done.

+#include "content/public/browser/host_zoom_map.h"
+#include "content/public/browser/zoom_level_prefs_store.h"
+
+class JsonPrefStore;

[Fady Samuel, 2014/08/15 15:03:01]

Is this necessary?

[wjmaclean, 2014/08/15 22:13:50]

Done.

+class PrefService;
+class PrefServiceSyncable;

[Fady Samuel, 2014/08/15 15:03:01]

Is this necessary?

[wjmaclean, 2014/08/15 22:13:50]

Done.

+
+namespace content {
+
+class HostZoomMapImpl;
+
+class HostZoomLevelContext {
+  // This class manages a HostZoomMap and, associates it with a PrefsStore,
+  // if one is provided.

[Fady Samuel, 2014/08/15 15:03:01]

Put this ABOVE the class. Also, this description doesn't explain to me the need
for this class at all. Why can't Profile have these two? Is it a lifetime
management issue as described below or is there more to it than that?

[wjmaclean, 2014/08/15 22:13:50]

Profile? This is owned by StoragePartitionImpl.

The rest of StoragePartitionImpl seems to be organised in a manner such that
each functionality is encapsulated into its own context class, and in the
interests of keeping the structure of StoragePartitionImpl simple and
consistent, I opted to do the same here.

If you strongly feel it has to go into StoragePartitionImpl, let me know.

Done.

+ public:
+  HostZoomLevelContext(scoped_ptr<ZoomLevelPrefsStore> prefs_store,
+                       const base::FilePath& file_path,
+                       double default_zoom_level);
+  ~HostZoomLevelContext();
+
+  HostZoomMapImpl* GetHostZoomMap() const { return host_zoom_map_impl_.get(); }

[Fady Samuel, 2014/08/15 15:03:01]

host_zoom_map

[wjmaclean, 2014/08/15 22:13:49]

Done.

+  PrefService* GetZoomLevelPrefs() {

[Fady Samuel, 2014/08/15 15:03:01]

zoom_level_prefs

[wjmaclean, 2014/08/15 22:13:50]

Done.

+    return prefs_store_ ? prefs_store_->GetPrefs() : NULL;
+  }
+
+ private:
+  scoped_ptr<HostZoomMapImpl> host_zoom_map_impl_;
+  // The prefs_store pointer needs to be released before the destructor for the
+  // HostZoomMapImpl is called, and so must be declared in this order.
+  scoped_ptr<ZoomLevelPrefsStore> prefs_store_;
+
+  DISALLOW_COPY_AND_ASSIGN(HostZoomLevelContext);
+};  // HostZoomLevelContext

[Fady Samuel, 2014/08/15 15:03:01]

Remove this comment.

[wjmaclean, 2014/08/15 22:13:50]

Done.

+
+}  // namespace content
+
+#endif  // CONTENT_BROWSER_HOST_ZOOM_LEVEL_CONTEXT