Revert "WebApp: Improve WebAppSyncBridge::UpdateSync to use both app states."

This reverts commit ff41c36d.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 715133 as the
culprit for failures in the build cycles as shown on:
https://analysis.chromium.org/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtL2ZmNDFjMzZkY2UzZTUzYmZjYWQ2Y2VkZGMxZmMyOWU2MjBhNGYxMTcM

Sample Failed Build: https://ci.chromium.org/b/8896853113607982064

Sample Failed Step: unit_tests

Original change's description:
> WebApp: Improve WebAppSyncBridge::UpdateSync to use both app states.
> 
> Improve WebAppSyncBridge::UpdateSync to use both current_state (before commit)
> and new_state (to be commit).
> 
> In the future, it will help to skip local updates which don't touch sync data.
> 
> Add WebAppSyncBridge::CommitUpdate unit tests (a.k.a. Local Changes).
> 
> These tests is a promised follow up for this CL:
> https://chromium-review.googlesource.com/c/chromium/src/+/1830494
> 
> This code is disabled by default behind kDesktopPWAsWithoutExtensions and
> kDesktopPWAsUSS base features.
> 
> Bug: 860583
> Change-Id: I51dfda8b6364e15f9b5eb398321d6010241b0ae9
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1914005
> Reviewed-by: Marc Treib <treib@chromium.org>
> Commit-Queue: Alexey Baskakov <loyso@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#715133}


Change-Id: Ia74c5f3586de10d9931ae8e23cd8deda44230508
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 860583
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1915545
Cr-Commit-Position: refs/heads/master@{#715170}

chrome/browser/web_applications/web_app_sync_bridge.cc

@@ -244,39 +244,36 @@ void WebAppSyncBridge::UpdateSync(
   if (!change_processor()->IsTrackingMetadata())
     return;
 
-  for (const std::unique_ptr<WebApp>& new_app : update_data.apps_to_create) {
-    if (new_app->IsSynced()) {
-      change_processor()->Put(new_app->app_id(), CreateSyncEntityData(*new_app),
+  for (const std::unique_ptr<WebApp>& app : update_data.apps_to_create) {
+    if (app->IsSynced()) {
+      change_processor()->Put(app->app_id(), CreateSyncEntityData(*app),
                               metadata_change_list);
     }
   }
 
-  for (const std::unique_ptr<WebApp>& new_state : update_data.apps_to_update) {
-    const AppId& app_id = new_state->app_id();
-    // Find the current state of the app to be overritten.
-    const WebApp* current_state = registrar_->GetAppById(app_id);
-    DCHECK(current_state);
-
-    // Include the app in the sync "view" if IsSynced flag becomes true. Update
-    // the app if IsSynced flag stays true. Exclude the app from the sync "view"
-    // if IsSynced flag becomes false.
+  // An app may obtain or may loose IsSynced flag without being deleted. We
+  // should conservatively include or exclude the app from the synced apps
+  // subset. TODO(loyso): Use previous state of the app (and CoW) to detect
+  // if IsSynced flag changed.
   //
   // TODO(loyso): Send an update to sync server only if any sync-specific
   // data was changed. Implement some dirty flags in WebApp setter methods.
-    if (new_state->IsSynced()) {
-      change_processor()->Put(app_id, CreateSyncEntityData(*new_state),
+  for (const std::unique_ptr<WebApp>& app : update_data.apps_to_update) {
+    if (app->IsSynced()) {
+      change_processor()->Put(app->app_id(), CreateSyncEntityData(*app),
                               metadata_change_list);
-    } else if (current_state->IsSynced()) {
-      change_processor()->Delete(app_id, metadata_change_list);
+    } else {
+      change_processor()->Delete(app->app_id(), metadata_change_list);
     }
   }
 
-  for (const AppId& app_id_to_delete : update_data.apps_to_delete) {
-    const WebApp* current_state = registrar_->GetAppById(app_id_to_delete);
-    DCHECK(current_state);
-    // Exclude the app from the sync "view" if IsSynced flag was true.
-    if (current_state->IsSynced())
-      change_processor()->Delete(app_id_to_delete, metadata_change_list);
+  // We should unconditionally delete from sync (in case IsSynced flag was
+  // removed during the update). TODO(loyso): Use previous state of the app (and
+  // CoW) to detect if IsSynced flag changed.
+  for (const AppId& app_id : update_data.apps_to_delete) {
+    const WebApp* app = registrar_->GetAppById(app_id);
+    DCHECK(app);
+    change_processor()->Delete(app_id, metadata_change_list);
   }
 }