Lazily build the app list model.

chrome/browser/ui/app_list/app_list_syncable_service.cc

Index: chrome/browser/ui/app_list/app_list_syncable_service.cc
diff --git a/chrome/browser/ui/app_list/app_list_syncable_service.cc b/chrome/browser/ui/app_list/app_list_syncable_service.cc
index 9dd44d63232d59b82f0ad308d705dc9c1af0b1f5..391fa0c7b3da58d9c307413cadfaeb3530ac24c3 100644
--- a/chrome/browser/ui/app_list/app_list_syncable_service.cc
+++ b/chrome/browser/ui/app_list/app_list_syncable_service.cc
@@ -7,7 +7,6 @@
 #include "base/command_line.h"
 #include "base/strings/string_util.h"
 #include "chrome/browser/apps/drive/drive_app_provider.h"
-#include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/app_list/app_list_prefs.h"
@@ -19,7 +18,6 @@
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/grit/generated_resources.h"
-#include "content/public/browser/notification_source.h"
 #include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/uninstall_reason.h"
@@ -181,11 +179,11 @@ class AppListSyncableService::ModelObserver : public AppListModelObserver {
       : owner_(owner),
         adding_item_(NULL) {
     DVLOG(2) << owner_ << ": ModelObserver Added";
-    owner_->model()->AddObserver(this);
+    owner_->GetModel()->AddObserver(this);
   }
 
   ~ModelObserver() override {
-    owner_->model()->RemoveObserver(this);
+    owner_->GetModel()->RemoveObserver(this);
     DVLOG(2) << owner_ << ": ModelObserver Removed";
   }
 
@@ -244,20 +242,6 @@ AppListSyncableService::AppListSyncableService(
 
   oem_folder_name_ =
       l10n_util::GetStringUTF8(IDS_APP_LIST_OEM_DEFAULT_FOLDER_NAME);
-
-  // Note: model_observer_ is constructed after the initial sync changes are
-  // received in MergeDataAndStartSyncing(). Changes to the model before that
-  // will be synced after the initial sync occurs.
-  if (extension_system->extension_service() &&

[tapted, 2015/03/10 19:19:54]

How do you know this logic isn't needed any more? (maybe `git annotate` to see
why it exists - I think it was something obscure)

I guess model() would have just returned an empty model in those cases. But
could something call GetModel() too soon and die? Or: build an empty model, but
then never receive any signal that the model is ready to be populated?

[stevenjb, 2015/03/10 19:56:06]

Since AppListSyncableService is a context keyed service, it might be initialized
before or after the extension service, so we needed to initialize the model
either here or once the extension service has been created and we are notified
(although that notification appears deprecated, I'm not familiar with the
situation there).

Nothing should actually access the model before the extension service is ready
however. We could add some DCHECKs to BuildModel() to make that clear.
(Otherwise we will crash in ExtensionAppModelBuilder::BuildModel).

[calamity, 2015/03/12 21:23:22]

DCHECKs added.

-      extension_system->extension_service()->is_ready()) {
-    BuildModel();
-    return;
-  }
-
-  // The extensions for this profile have not yet all been loaded.
-  registrar_.Add(this,
-                 extensions::NOTIFICATION_EXTENSIONS_READY_DEPRECATED,
-                 content::Source<Profile>(profile));
 }
 
 AppListSyncableService::~AppListSyncableService() {
@@ -283,7 +267,7 @@ void AppListSyncableService::BuildModel() {
   if (app_list::switches::IsAppListSyncEnabled()) {
     VLOG(1) << this << ": AppListSyncableService: InitializeWithService.";
     SyncStarted();
-    apps_builder_->InitializeWithService(this);
+    apps_builder_->InitializeWithService(this, model_.get());
   } else {
     VLOG(1) << this << ": AppListSyncableService: InitializeWithProfile.";
     apps_builder_->InitializeWithProfile(profile_, model_.get());
@@ -296,6 +280,14 @@ void AppListSyncableService::BuildModel() {
     drive_app_provider_.reset(new DriveAppProvider(profile_, this));
 }
 
+size_t AppListSyncableService::GetNumSyncItemsForTest() {
+  // If the model isn't built yet, there will be no sync items.
+  if (!apps_builder_)
+    BuildModel();

[stevenjb, 2015/03/10 19:56:06]

nit: GetModel();

[calamity, 2015/03/12 21:23:22]

Done.

+
+  return sync_items_.size();
+}
+
 void AppListSyncableService::ResetDriveAppProviderForTest() {
   drive_app_provider_.reset();
 }
@@ -333,16 +325,6 @@ void AppListSyncableService::UntrackUninstalledDriveApp(
   DeleteSyncItem(sync_item);
 }
 
-void AppListSyncableService::Observe(
-    int type,
-    const content::NotificationSource& source,
-    const content::NotificationDetails& details) {
-  DCHECK_EQ(extensions::NOTIFICATION_EXTENSIONS_READY_DEPRECATED, type);
-  DCHECK_EQ(profile_, content::Source<Profile>(source).ptr());
-  registrar_.RemoveAll();
-  BuildModel();
-}
-
 const AppListSyncableService::SyncItem*
 AppListSyncableService::GetSyncItem(const std::string& id) const {
   SyncItemMap::const_iterator iter = sync_items_.find(id);
@@ -358,6 +340,13 @@ void AppListSyncableService::SetOemFolderName(const std::string& name) {
     model_->SetItemName(oem_folder, oem_folder_name_);
 }
 
+AppListModel* AppListSyncableService::GetModel() {
+  if (!apps_builder_)
+    BuildModel();
+
+  return model_.get();
+}
+
 void AppListSyncableService::AddItem(scoped_ptr<AppListItem> app_item) {
   SyncItem* sync_item = FindOrAddSyncItem(app_item.get());
   if (!sync_item)