Added implementation of SyncEventObserver to route events to Javascript Extension.

chrome/browser/extensions/api/sync_file_system/extension_sync_event_observer.cc

Index: chrome/browser/extensions/api/sync_file_system/extension_sync_event_observer.cc
diff --git a/chrome/browser/extensions/api/sync_file_system/extension_sync_event_observer.cc b/chrome/browser/extensions/api/sync_file_system/extension_sync_event_observer.cc
new file mode 100644
index 0000000000000000000000000000000000000000..954318aa31bed9a90acb66b9a3ca027cf534efb3
--- /dev/null
+++ b/chrome/browser/extensions/api/sync_file_system/extension_sync_event_observer.cc
@@ -0,0 +1,63 @@
+// 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/extensions/api/sync_file_system/extension_sync_event_observer.h"
+#include "chrome/browser/extensions/event_names.h"
+#include "chrome/browser/extensions/extension_system.h"
+#include "chrome/browser/sync_file_system/sync_event_observer.h"
+#include "chrome/common/extensions/api/sync_file_system.h"
+
+using sync_file_system::SyncEventObserver;
+
+namespace extensions {
+
+namespace {
+
+api::sync_file_system::SyncStateStatus SyncServiceStateEnumToExtensionEnum(
+    SyncEventObserver::SyncServiceState state) {
+  switch(state) {
+    case SyncEventObserver::SYNC_SERVICE_INITIALIZING:
+      return api::sync_file_system::
+          SYNC_FILE_SYSTEM_SYNC_STATE_STATUS_INITIALIZING;

[kinuko, 2012/11/21 10:59:04]

Wow is this line-break allowed?  (If it compiles it's fine)

[calvinlo, 2012/11/28 06:01:45]

Yes it actually compiles. I wasn't sure how else to make this shorter. I tried
using a "using" statement to shorten this but then the compiler complained.

If you have suggestions on how to shorten this, I'd really like to know. I'm not
familiar with how to do this in C++ (versus Java). 

+    case SyncEventObserver::SYNC_SERVICE_RUNNING:
+      return api::sync_file_system::SYNC_FILE_SYSTEM_SYNC_STATE_STATUS_RUNNING;
+    case SyncEventObserver::SYNC_SERVICE_AUTHENTICATION_REQUIRED:
+      return api::sync_file_system::
+          SYNC_FILE_SYSTEM_SYNC_STATE_STATUS_AUTHENTICATION_REQUIRED;
+    case SyncEventObserver::SYNC_SERVICE_TEMPORARY_UNAVAILABLE:
+      return api::sync_file_system::
+          SYNC_FILE_SYSTEM_SYNC_STATE_STATUS_TEMPORARY_UNAVAILABLE;
+    case SyncEventObserver::SYNC_SERVICE_DISABLED:
+      return api::sync_file_system::SYNC_FILE_SYSTEM_SYNC_STATE_STATUS_DISABLED;
+  }
+  NOTREACHED();
+  return api::sync_file_system::SYNC_FILE_SYSTEM_SYNC_STATE_STATUS_NONE;
+}
+
+}  // namespace
+
+ExtensionSyncEventObserver::ExtensionSyncEventObserver(
+    std::string extension_id,
+    Profile* profile)
+  : extension_id_(extension_id),
+    profile_(profile) {}

[kinuko, 2012/11/21 10:59:04]

nit: four spaces before ':' is the common pattern

    ...
    Profile* profile)
    : extension_id_(extension_id),
      profile_(profile) {}

[calvinlo, 2012/11/28 06:01:45]

Done.

+
+void ExtensionSyncEventObserver::OnSyncStateUpdated(
+    sync_file_system::SyncEventObserver::SyncServiceState state,
+    const std::string& description) {
+  // Convert state and description into SyncState Object
+  api::sync_file_system::SyncState syncState;

[kinuko, 2012/11/21 10:59:04]

syncState -> sync_state while you're in chrome c++ code :)

[calvinlo, 2012/11/28 06:01:45]

Done.

+  syncState.service_name = "drive";

[kinuko, 2012/11/21 10:59:04]

Maybe this should be also given in constructor. Or maybe even better I think we
can make the constructor just take a single pointer to the SyncEventManager and
the manager can hold profile_ and service_name_ fields (since they will be
shared by observers).

class ExtensionSyncEventManager {
 public:
  ExtensionSyncEventManager(Profile* profile, const std::string& service_name);

  // Getters for SyncEventObserver.
  Profile* profile() { return profile_; }
  const std::string& service_name() const { return service_name_; }
  ...
}

class ExtensionSyncEventObserver {
 public:
  explicit ExtensionSyncEventObserver(ExtensionSyncEventManager* manager);
  ...
}

If you're not going to have Manager code in this CL yet you can leave TODO
comment instead

[calvinlo, 2012/11/28 06:01:45]

Ok, finally able to get back to this. Based on our conversation, I don't think
we need a manager anymore since the app_origin/extension ID value is now passed
as a param to each function in the SyncEventObserver interface.

I still need to add set::extension IDs that are listening and ignore sending the
event if the extension_id isn't listening here.

Do you think it's a good idea for me to submit this first with TODOs and add all
the OnEventListener and NOTIFICATION_EXTENSION_UNLOADED hookups in another patch
or continue to do everything in this patch?

[kinuko, 2012/11/28 07:41:13]

I feel that EXTENSION_UNLOADED can surely come later. OnListenerAdded may be
crucial so I might want to see it together but please feel free to leave TODOs
when you think the patch has enough meaningful content.

+  syncState.state = SyncServiceStateEnumToExtensionEnum(state);
+  syncState.description = description;
+  scoped_ptr<base::ListValue> params(
+      api::sync_file_system::OnSyncStateChanged::Create(syncState));
+
+  // Dispatch the event to the extension
+  ExtensionSystem::Get(profile_)->event_router()->DispatchEventToExtension(
+      extension_id_, event_names::kOnSyncStateChanged, params.Pass(),
+      profile_, GURL());
+}
+
+}  // namespace extensions