app_list: fix some folder issues.

Fix 1:
If app_list_controller_ is not ready, which means the model adapter is
inactive, we add this item in Chrome first.

Fix 2:
While sending a whole trunk of model data to Ash, we also send folders
in case folder information was updated.

Bug: 733662
Bug: 820873
Change-Id: I60aa899acff2a5a5203496498b278f2d6e6cfddb
Reviewed-on: https://chromium-review.googlesource.com/958244
Commit-Queue: Jiaquan He <hejq@google.com>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542525}

ash/app_list/app_list_controller_impl.cc

@@ -166,8 +166,17 @@ void AppListControllerImpl::SetModelData(
   model_.DeleteAllItems();
   search_model_.DeleteAllResults();
 
-  // Populate new models.
+  // Populate new models. First populate folders and then other items to avoid
+  // automatically creating items in |AddItemToFolder|.
   for (auto& app : apps) {
+    if (!app->is_folder)
+      continue;
+    DCHECK(app->folder_id.empty());
+    AddItem(std::move(app));
+  }
+  for (auto& app : apps) {
+    if (!app)
+      continue;
     const std::string folder_id = app->folder_id;
     if (folder_id.empty())
       AddItem(std::move(app));

chrome/browser/ui/app_list/chrome_app_list_item.cc

@@ -183,6 +183,19 @@ void ChromeAppListItem::SetChromeFolderId(const std::string& folder_id) {
   metadata_->folder_id = folder_id;
 }
 
+void ChromeAppListItem::SetChromeIsFolder(bool is_folder) {
+  metadata_->is_folder = is_folder;
+}
+
+void ChromeAppListItem::SetChromeName(const std::string& name) {
+  metadata_->name = name;
+}
+
+void ChromeAppListItem::SetChromePosition(
+    const syncer::StringOrdinal& position) {
+  metadata_->position = position;
+}
+
 bool ChromeAppListItem::CompareForTest(const ChromeAppListItem* other) const {
   return id() == other->id() && folder_id() == other->folder_id() &&
          name() == other->name() && GetItemType() == other->GetItemType() &&

chrome/browser/ui/app_list/chrome_app_list_item.h

@@ -73,10 +73,12 @@ class ChromeAppListItem {
   void SetFolderId(const std::string& folder_id);
   void SetPosition(const syncer::StringOrdinal& position);
 
-  // Setting the folder id of this item.
-  // Note: this method won't make changes to Ash and it should be called by
-  //       this item itself or the model updater.
+  // The following methods won't make changes to Ash and it should be called
+  // by this item itself or the model updater.
   void SetChromeFolderId(const std::string& folder_id);
+  void SetChromeIsFolder(bool is_folder);
+  void SetChromeName(const std::string& name);
+  void SetChromePosition(const syncer::StringOrdinal& position);
 
   // Activates (opens) the item. Does nothing by default.
   virtual void Activate(int event_flags);
@@ -113,8 +115,6 @@ class ChromeAppListItem {
     model_updater_ = model_updater;
   }
 
-  void SetIsFolder(bool is_folder) { metadata_->is_folder = is_folder; }
-
   // Updates item position and name from |sync_item|. |sync_item| must be valid.
   void UpdateFromSync(
       const app_list::AppListSyncableService::SyncItem* sync_item);

chrome/browser/ui/app_list/chrome_app_list_model_updater.cc

@@ -37,11 +37,8 @@ void ChromeAppListModelUpdater::SetActive(bool active) {
 
   // Activating this model updater should sync the cached model to Ash.
   std::vector<ash::mojom::AppListItemMetadataPtr> items_to_sync;
-  for (auto const& item : items_) {
-    ChromeAppListItem* chrome_item = item.second.get();
-    if (!chrome_item->is_folder())
-      items_to_sync.push_back(chrome_item->CloneMetadata());
-  }
+  for (auto const& item : items_)
+    items_to_sync.push_back(item.second->CloneMetadata());
   app_list_controller_->SetModelData(std::move(items_to_sync),
                                      search_engine_is_google_);
 }
@@ -402,18 +399,33 @@ void ChromeAppListModelUpdater::AddItemToOemFolder(
   if (oem_sync_item && oem_sync_item->item_ordinal.IsValid())
     position_to_try = oem_sync_item->item_ordinal;
 
-  app_list_controller_->FindOrCreateOemFolder(
-      oem_folder_id, oem_folder_name, position_to_try,
-      base::BindOnce(
-          [](base::WeakPtr<ChromeAppListModelUpdater> self,
-             std::unique_ptr<ChromeAppListItem> item,
-             const std::string& oem_folder_id,
-             ash::mojom::AppListItemMetadataPtr /* oem_folder */) {
-            if (!self)
-              return;
-            self->AddItemToFolder(std::move(item), oem_folder_id);
-          },
-          weak_ptr_factory_.GetWeakPtr(), std::move(item), oem_folder_id));
+  if (app_list_controller_) {
+    app_list_controller_->FindOrCreateOemFolder(
+        oem_folder_id, oem_folder_name, position_to_try,
+        base::BindOnce(
+            [](base::WeakPtr<ChromeAppListModelUpdater> self,
+               std::unique_ptr<ChromeAppListItem> item,
+               const std::string& oem_folder_id,
+               ash::mojom::AppListItemMetadataPtr /* oem_folder */) {
+              if (!self)
+                return;
+              self->AddItemToFolder(std::move(item), oem_folder_id);
+            },
+            weak_ptr_factory_.GetWeakPtr(), std::move(item), oem_folder_id));
+  } else {
+    ChromeAppListItem* item_added = AddChromeItem(std::move(item));
+    item_added->SetChromeFolderId(oem_folder_id);
+    // If we don't have an OEM folder in Chrome, create one first.
+    ChromeAppListItem* oem_folder = FindFolderItem(oem_folder_id);
+    if (!oem_folder) {
+      std::unique_ptr<ChromeAppListItem> new_oem_folder =
+          std::make_unique<ChromeAppListItem>(profile_, oem_folder_id);
+      oem_folder = AddChromeItem(std::move(new_oem_folder));
+      oem_folder->SetChromeIsFolder(true);
+    }
+    oem_folder->SetChromeName(oem_folder_name);
+    oem_folder->SetChromePosition(position_to_try);
+  }
 }
 
 void ChromeAppListModelUpdater::UpdateAppItemFromSyncItem(