Move ownership of AppSorting from ExtensionPrefs to ExtensionSystem

chrome/browser/extensions/chrome_app_sorting.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 "chrome/browser/extensions/chrome_app_sorting.h"
 
 #include <algorithm>
 #include <vector>
 
 #include "chrome/browser/chrome_notification_types.h"
@@ skipping 33 matching lines @@
 // ChromeAppSorting::AppOrdinals
 
 ChromeAppSorting::AppOrdinals::AppOrdinals() {}
 
 ChromeAppSorting::AppOrdinals::~AppOrdinals() {}
 
 ////////////////////////////////////////////////////////////////////////////////
 // ChromeAppSorting
 
 ChromeAppSorting::ChromeAppSorting(content::BrowserContext* browser_context)
-    : extension_scoped_prefs_(NULL),
-      browser_context_(browser_context),
+    : browser_context_(browser_context),
       default_ordinals_created_(false) {
 }
 
 ChromeAppSorting::~ChromeAppSorting() {
 }
 
-void ChromeAppSorting::SetExtensionScopedPrefs(ExtensionScopedPrefs* prefs) {
-  extension_scoped_prefs_ = prefs;
-}
-
-void ChromeAppSorting::CheckExtensionScopedPrefs() const {
-  CHECK(extension_scoped_prefs_);
-}
-
 void ChromeAppSorting::Initialize(
     const extensions::ExtensionIdList& extension_ids) {
-  CHECK(extension_scoped_prefs_);
   InitializePageOrdinalMap(extension_ids);
 
   MigrateAppIndex(extension_ids);
 }
 
 void ChromeAppSorting::CreateOrdinalsIfNecessary(size_t minimum_size) {
   // Create StringOrdinal values as required to ensure |ntp_ordinal_map_| has at
   // least |minimum_size| entries.
   if (ntp_ordinal_map_.empty() && minimum_size > 0)
     ntp_ordinal_map_[syncer::StringOrdinal::CreateInitialOrdinal()];
@@ skipping 14 matching lines @@
   // Convert all the page index values to page ordinals. If there are any
   // app launch values that need to be migrated, inserted them into a sorted
   // set to be dealt with later.
   typedef std::map<syncer::StringOrdinal, std::map<int, const std::string*>,
                    syncer::StringOrdinal::LessThanFn> AppPositionToIdMapping;
   AppPositionToIdMapping app_launches_to_convert;
   for (extensions::ExtensionIdList::const_iterator ext_id =
            extension_ids.begin(); ext_id != extension_ids.end(); ++ext_id) {
     int old_page_index = 0;
     syncer::StringOrdinal page = GetPageOrdinal(*ext_id);
-    if (extension_scoped_prefs_->ReadPrefAsInteger(
+    if (ExtensionPrefs::Get(browser_context_)->ReadPrefAsInteger(

[not at google - send to devlin, 2015/07/29 18:22:03]

I would hold onto a reference to this at the start of MigrateAppIndex or
something, it's used several times.

[Marc Treib, 2015/07/30 14:04:12]

Done.

             *ext_id,
             kPrefPageIndexDeprecated,
             &old_page_index)) {
       // Some extensions have invalid page index, so we don't
       // attempt to convert them.
       if (old_page_index < 0) {
         DLOG(WARNING) << "Extension " << *ext_id
                       << " has an invalid page index " << old_page_index
                       << ". Aborting attempt to convert its index.";
         break;
       }
 
       CreateOrdinalsIfNecessary(static_cast<size_t>(old_page_index) + 1);
 
       page = PageIntegerAsStringOrdinal(old_page_index);
       SetPageOrdinal(*ext_id, page);
-      extension_scoped_prefs_->UpdateExtensionPref(
+      ExtensionPrefs::Get(browser_context_)->UpdateExtensionPref(
           *ext_id, kPrefPageIndexDeprecated, NULL);
     }
 
     int old_app_launch_index = 0;
-    if (extension_scoped_prefs_->ReadPrefAsInteger(
+    if (ExtensionPrefs::Get(browser_context_)->ReadPrefAsInteger(
             *ext_id,
             kPrefAppLaunchIndexDeprecated,
             &old_app_launch_index)) {
       // We can't update the app launch index value yet, because we use
       // GetNextAppLaunchOrdinal to get the new ordinal value and it requires
       // all the ordinals with lower values to have already been migrated.
       // A valid page ordinal is also required because otherwise there is
       // no page to add the app to.
       if (page.IsValid())
         app_launches_to_convert[page][old_app_launch_index] = &*ext_id;
 
-      extension_scoped_prefs_->UpdateExtensionPref(
+      ExtensionPrefs::Get(browser_context_)->UpdateExtensionPref(
           *ext_id, kPrefAppLaunchIndexDeprecated, NULL);
     }
   }
 
   // Remove any empty pages that may have been added. This shouldn't occur,
   // but double check here to prevent future problems with conversions between
   // integers and StringOrdinals.
   for (PageOrdinalMap::iterator it = ntp_ordinal_map_.begin();
        it != ntp_ordinal_map_.end();) {
     if (it->second.empty()) {
@@ skipping 138 matching lines @@
       content::Details<const std::string>(&moved_extension_id));
 }
 
 
 syncer::StringOrdinal ChromeAppSorting::GetAppLaunchOrdinal(
     const std::string& extension_id) const {
   std::string raw_value;
   // If the preference read fails then raw_value will still be unset and we
   // will return an invalid StringOrdinal to signal that no app launch ordinal
   // was found.
-  extension_scoped_prefs_->ReadPrefAsString(
+  ExtensionPrefs::Get(browser_context_)->ReadPrefAsString(
       extension_id, kPrefAppLaunchOrdinal, &raw_value);
   return syncer::StringOrdinal(raw_value);
 }
 
 void ChromeAppSorting::SetAppLaunchOrdinal(
     const std::string& extension_id,
     const syncer::StringOrdinal& new_app_launch_ordinal) {
   // No work is required if the old and new values are the same.
   if (new_app_launch_ordinal.EqualsOrBothInvalid(
           GetAppLaunchOrdinal(extension_id))) {
     return;
   }
 
   syncer::StringOrdinal page_ordinal = GetPageOrdinal(extension_id);
   RemoveOrdinalMapping(
       extension_id, page_ordinal, GetAppLaunchOrdinal(extension_id));
   AddOrdinalMapping(extension_id, page_ordinal, new_app_launch_ordinal);
 
   base::Value* new_value = new_app_launch_ordinal.IsValid() ?
       new base::StringValue(new_app_launch_ordinal.ToInternalValue()) :
       NULL;
 
-  extension_scoped_prefs_->UpdateExtensionPref(
+  ExtensionPrefs::Get(browser_context_)->UpdateExtensionPref(
       extension_id,
       kPrefAppLaunchOrdinal,
       new_value);
   SyncIfNeeded(extension_id);
 }
 
 syncer::StringOrdinal ChromeAppSorting::CreateFirstAppLaunchOrdinal(
     const syncer::StringOrdinal& page_ordinal) const {
   const syncer::StringOrdinal& min_ordinal =
       GetMinOrMaxAppLaunchOrdinalsOnPage(page_ordinal,
@@ skipping 37 matching lines @@
   // Add a new page as all existing pages are full.
   syncer::StringOrdinal last_element = ntp_ordinal_map_.rbegin()->first;
   return last_element.CreateAfter();
 }
 
 syncer::StringOrdinal ChromeAppSorting::GetPageOrdinal(
     const std::string& extension_id) const {
   std::string raw_data;
   // If the preference read fails then raw_data will still be unset and we will
   // return an invalid StringOrdinal to signal that no page ordinal was found.
-  extension_scoped_prefs_->ReadPrefAsString(
+  ExtensionPrefs::Get(browser_context_)->ReadPrefAsString(
       extension_id, kPrefPageOrdinal, &raw_data);
   return syncer::StringOrdinal(raw_data);
 }
 
 void ChromeAppSorting::SetPageOrdinal(
     const std::string& extension_id,
     const syncer::StringOrdinal& new_page_ordinal) {
   // No work is required if the old and new values are the same.
   if (new_page_ordinal.EqualsOrBothInvalid(GetPageOrdinal(extension_id)))
     return;
 
   syncer::StringOrdinal app_launch_ordinal = GetAppLaunchOrdinal(extension_id);
   RemoveOrdinalMapping(
       extension_id, GetPageOrdinal(extension_id), app_launch_ordinal);
   AddOrdinalMapping(extension_id, new_page_ordinal, app_launch_ordinal);
 
   base::Value* new_value = new_page_ordinal.IsValid() ?
       new base::StringValue(new_page_ordinal.ToInternalValue()) :
       NULL;
 
-  extension_scoped_prefs_->UpdateExtensionPref(
+  ExtensionPrefs::Get(browser_context_)->UpdateExtensionPref(
       extension_id,
       kPrefPageOrdinal,
       new_value);
   SyncIfNeeded(extension_id);
 }
 
 void ChromeAppSorting::ClearOrdinals(const std::string& extension_id) {
   RemoveOrdinalMapping(extension_id,
                        GetPageOrdinal(extension_id),
                        GetAppLaunchOrdinal(extension_id));
 
-  extension_scoped_prefs_->UpdateExtensionPref(
+  ExtensionPrefs::Get(browser_context_)->UpdateExtensionPref(

[not at google - send to devlin, 2015/07/29 18:22:03]

Here as well.

[Marc Treib, 2015/07/30 14:04:12]

Done.

       extension_id, kPrefPageOrdinal, NULL);
-  extension_scoped_prefs_->UpdateExtensionPref(
+  ExtensionPrefs::Get(browser_context_)->UpdateExtensionPref(
       extension_id, kPrefAppLaunchOrdinal, NULL);
 }
 
 int ChromeAppSorting::PageStringOrdinalAsInteger(
     const syncer::StringOrdinal& page_ordinal) const {
   if (!page_ordinal.IsValid())
     return -1;
 
   PageOrdinalMap::const_iterator it = ntp_ordinal_map_.find(page_ordinal);
   return it != ntp_ordinal_map_.end() ?
@@ skipping 39 matching lines @@
       return_value = app_list.rbegin()->first;
     else if (return_type == ChromeAppSorting::MIN_ORDINAL)
       return_value = app_list.begin()->first;
   }
 
   return return_value;
 }
 
 void ChromeAppSorting::InitializePageOrdinalMap(
     const extensions::ExtensionIdList& extension_ids) {
-  // TODO(mgiuca): Added this CHECK to try and diagnose http://crbug.com/476648.
-  // Remove it after the investigation is concluded.
-  CHECK(extension_scoped_prefs_);
   for (extensions::ExtensionIdList::const_iterator ext_it =
            extension_ids.begin(); ext_it != extension_ids.end(); ++ext_it) {
     AddOrdinalMapping(*ext_it,
                       GetPageOrdinal(*ext_it),
                       GetAppLaunchOrdinal(*ext_it));
 
     // Ensure that the web store app still isn't found in this list, since
     // it is added after this loop.
     DCHECK(*ext_it != extensions::kWebStoreAppId);
     DCHECK(*ext_it != extension_misc::kChromeAppId);
@@ skipping 144 matching lines @@
   for (AppLaunchOrdinalMap::const_iterator it = m.begin(); it != m.end();
        ++it) {
     const std::string& id = it->second;
     if (ntp_hidden_extensions_.count(id) == 0)
       result++;
   }
   return result;
 }
 
 }  // namespace extensions