SyncFS: store disabled origins in DriveMetadataStore

chrome/browser/sync_file_system/sync_file_system_service.cc

 // Copyright (c) 2012 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/sync_file_system/sync_file_system_service.h"
 
 #include <string>
 
 #include "base/bind.h"
 #include "base/logging.h"
@@ skipping 178 matching lines @@
       ProfileSyncServiceFactory::GetForProfile(profile_);
   if (profile_sync_service) {
     UpdateSyncEnabledStatus(profile_sync_service);
     profile_sync_service->AddObserver(this);
   }
 
   registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_UNLOADED,
                  content::Source<Profile>(profile_));
   registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_LOADED,
                  content::Source<Profile>(profile_));
+  registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_ENABLED,
+                 content::Source<Profile>(profile_));
 }
 
 void SyncFileSystemService::DidInitializeFileSystem(
     const GURL& app_origin,
     const SyncStatusCallback& callback,
     SyncStatusCode status) {
   DVLOG(1) << "DidInitializeFileSystem: "
            << app_origin.spec() << " " << status;
 
   if (status != SYNC_STATUS_OK) {
@@ skipping 182 matching lines @@
       SyncEventObserver, observers_,
       OnSyncStateUpdated(GURL(),
                          RemoteStateToSyncServiceState(state),
                          description));
 }
 
 void SyncFileSystemService::Observe(
     int type,
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
-  if (chrome::NOTIFICATION_EXTENSION_UNLOADED == type) {
-    // Unregister origin for remote synchronization.
-    std::string extension_id =
-        content::Details<const extensions::UnloadedExtensionInfo>(
-            details)->extension->id();
-    GURL app_origin = extensions::Extension::GetBaseURLFromExtensionId(
-        extension_id);
-    remote_file_service_->UnregisterOriginForTrackingChanges(
-        app_origin, base::Bind(&DidHandleOriginForExtensionEvent,
-                               type, app_origin));
-    local_file_service_->SetOriginEnabled(app_origin, false);
-  } else if (chrome::NOTIFICATION_EXTENSION_LOADED == type) {
-    std::string extension_id =
-        content::Details<const extensions::Extension>(
-            details)->id();
-    GURL app_origin = extensions::Extension::GetBaseURLFromExtensionId(
-        extension_id);
-    local_file_service_->SetOriginEnabled(app_origin, true);
-  } else {
-    NOTREACHED() << "Unknown notification.";
+  switch (type) {

[kinuko, 2013/03/16 23:16:51]

Can you add a comment about when each event is delivered?

[nhiroki, 2013/03/18 09:40:18]

Done.

+    case chrome::NOTIFICATION_EXTENSION_UNLOADED:
+      HandleExtensionUnloaded(type, details);
+      break;
+    case chrome::NOTIFICATION_EXTENSION_LOADED:
+      HandleExtensionLoaded(details);
+      break;
+    case chrome::NOTIFICATION_EXTENSION_ENABLED:
+      HandleExtensionEnabled(type, details);
+      break;
+    default:
+      NOTREACHED() << "Unknown notification.";
+      break;
   }
 }
 
+void SyncFileSystemService::HandleExtensionUnloaded(
+    int type,
+    const content::NotificationDetails& details) {
+  content::Details<const extensions::UnloadedExtensionInfo> info =
+      content::Details<const extensions::UnloadedExtensionInfo>(details);
+  std::string extension_id = info->extension->id();
+  GURL app_origin =
+      extensions::Extension::GetBaseURLFromExtensionId(extension_id);
+
+  switch (info->reason) {
+    case extension_misc::UNLOAD_REASON_DISABLE:
+      DVLOG(1) << "Handle extension notification for UNLOAD(DISABLE): "
+               << app_origin;
+      remote_file_service_->DisableOriginForTrackingChanges(
+          app_origin,
+          base::Bind(&DidHandleOriginForExtensionEvent, type, app_origin));
+      break;
+    case extension_misc::UNLOAD_REASON_UNINSTALL:
+      DVLOG(1) << "Handle extension notification for UNLOAD(UNINSTALL): "
+               << app_origin;
+      remote_file_service_->UnregisterOriginForTrackingChanges(
+          app_origin,
+          base::Bind(&DidHandleOriginForExtensionEvent, type, app_origin));
+      break;
+    default:
+      // Nothing to do.
+      break;
+  }
+
+  local_file_service_->SetOriginEnabled(app_origin, false);
+}
+
+void SyncFileSystemService::HandleExtensionLoaded(
+    const content::NotificationDetails& details) {
+  std::string extension_id =
+      content::Details<const extensions::Extension>(details)->id();
+  GURL app_origin =
+      extensions::Extension::GetBaseURLFromExtensionId(extension_id);
+  DVLOG(1) << "Handle extension notification for LOADED: " << app_origin;
+  local_file_service_->SetOriginEnabled(app_origin, true);
+}
+
+void SyncFileSystemService::HandleExtensionEnabled(
+    int type,
+    const content::NotificationDetails& details) {
+  std::string extension_id =
+      content::Details<const extensions::Extension>(details)->id();
+  GURL app_origin =
+      extensions::Extension::GetBaseURLFromExtensionId(extension_id);
+  DVLOG(1) << "Handle extension notification for ENABLED: " << app_origin;
+  remote_file_service_->EnableOriginForTrackingChanges(
+      app_origin,
+      base::Bind(&DidHandleOriginForExtensionEvent, type, app_origin));

[kinuko, 2013/03/16 23:16:51]

Looking into the table in
https://code.google.com/p/chromium/issues/detail?id=184742#c5, should we call
local_file_service_->SetOriginEnabled(app_origin, true) here and remove
HandleExtensionLoaded method?

Also while you're there could you remove the following line in
LocalFileSyncService::OriginChangeMap::SetOriginEnabled?  IIUC looks like
ENABLED could be called for origins that are not in the in-memory map (e.g. when
an extension is enabled after chrome relaunch).

line 85: DCHECK(ContainsKey(disabled_origins_, origin));

This map is only there to suppress local sync notifications, so I think just
removing this line'd work (or tests would tell).

[nhiroki, 2013/03/18 09:40:18]

Removed HandleExtensionLoaded and DCHECK from LocalFileSyncService. Done.

+}
+
 void SyncFileSystemService::OnStateChanged() {
   ProfileSyncServiceBase* profile_sync_service =
       ProfileSyncServiceFactory::GetForProfile(profile_);
   if (profile_sync_service)
     UpdateSyncEnabledStatus(profile_sync_service);
 }
 
 void SyncFileSystemService::OnFileStatusChanged(
     const FileSystemURL& url,
     SyncFileStatus sync_status,
@@ skipping 12 matching lines @@
       syncer::APPS);
   remote_file_service_->SetSyncEnabled(sync_enabled_);
   if (sync_enabled_) {
     base::MessageLoopProxy::current()->PostTask(
         FROM_HERE, base::Bind(&SyncFileSystemService::MaybeStartSync,
                               AsWeakPtr()));
   }
 }
 
 }  // namespace sync_file_system