Split up SyncEngineEventListener callbacks

sync/engine/sync_engine_event.h

Index: sync/engine/sync_engine_event.h
diff --git a/sync/engine/sync_engine_event.h b/sync/engine/sync_engine_event.h
index 026d3292a7ac3e6bcc693503c5a244b932d605b0..9f507cb08208ece86798165d37090d2ec382b8a2 100644
--- a/sync/engine/sync_engine_event.h
+++ b/sync/engine/sync_engine_event.h
@@ -7,17 +7,13 @@
 
 #include <string>
 
-#include "base/observer_list.h"
 #include "sync/base/sync_export.h"
 #include "sync/internal_api/public/sessions/sync_session_snapshot.h"
-
-namespace syncable {
-class Id;
-}
+#include "sync/protocol/sync_protocol_error.h"

[Nicolas Zea, 2014/02/10 23:15:16]

Can forward declare this

[rlarocque, 2014/02/11 00:19:43]

Done.

 
 namespace syncer {
 
-struct SYNC_EXPORT_PRIVATE SyncEngineEvent {
+struct SYNC_EXPORT_PRIVATE SyncCycleEvent {

[Nicolas Zea, 2014/02/10 23:15:16]

Possibly move to its own file?

[rlarocque, 2014/02/11 00:19:43]

Done.

   enum EventCause {
     ////////////////////////////////////////////////////////////////
     // Sent on entry of Syncer state machine
@@ -28,52 +24,34 @@ struct SYNC_EXPORT_PRIVATE SyncEngineEvent {
 
     // We have reached the SYNCER_END state in the main sync loop.
     SYNC_CYCLE_ENDED,
-
-    ////////////////////////////////////////////////////////////////
-    // Generated in response to specific protocol actions or events.
-
-    // This is sent after the Syncer (and SyncerThread) have initiated self
-    // halt due to no longer being permitted to communicate with the server.
-    // The listener should sever the sync / browser connections and delete sync
-    // data (i.e. as if the user clicked 'Stop Syncing' in the browser.
-    STOP_SYNCING_PERMANENTLY,

[Nicolas Zea, 2014/02/10 23:15:16]

How is STOP_SYNCING_PERMANENTLY being handled now?

[rlarocque, 2014/02/11 00:19:43]

As far as I can tell, it's not handled at all.

According to sync_enums.proto: "|STOP_SYNCING_PERMANENTLY| event should never be
seen by the server in the absence of bugs."

That may explain why I haven't been able to find any code that would create a
SyncEngineEvent with case == STOP_SYNCING_PERMANENTLY.  I think that feature is
deprecated, and this is dead code.

See also r235696.

-
-    // This event is sent when we receive an actionable error. It is upto
-    // the listeners to figure out the action to take using the snapshot sent.
-    ACTIONABLE_ERROR,
-
-    // This event is sent when scheduler decides to wait before next request
-    // either because it gets throttled by server or because it backs off after
-    // request failure. Retry time is passed in retry_time field of event.
-    RETRY_TIME_CHANGED,
-
-    // This event is sent when types are throttled or unthrottled.
-    THROTTLED_TYPES_CHANGED,
   };
 
-  explicit SyncEngineEvent(EventCause cause);
-  ~SyncEngineEvent();
+  explicit SyncCycleEvent(EventCause cause);
+  ~SyncCycleEvent();
 
   EventCause what_happened;
 
   // The last session used for syncing.
   sessions::SyncSessionSnapshot snapshot;
-
-  // Update-Client-Auth returns a new token for sync use.
-  std::string updated_token;
-
-  // Time when scheduler will try to send request after backoff.
-  base::Time retry_time;
-
-  // Set of types that are currently throttled.
-  ModelTypeSet throttled_types;
 };
 
 class SYNC_EXPORT_PRIVATE SyncEngineEventListener {

[Nicolas Zea, 2014/02/10 23:15:16]

This file should probably be renamed sync_engine_event_listener.h (once
SyncCycleEvent is in its own file)

[rlarocque, 2014/02/11 00:19:43]

Done.

  public:
-  // TODO(tim): Consider splitting this up to multiple callbacks, rather than
-  // have to do Event e(type); OnSyncEngineEvent(e); at all callsites,
-  virtual void OnSyncEngineEvent(const SyncEngineEvent& event) = 0;
+  // Generated at various points during the sync cycle.
+  virtual void OnSyncCycleEvent(const SyncCycleEvent& event) = 0;
+
+  // This event is sent when we receive an actionable error. It is up to
+  // the listeners to figure out the action to take using the error sent.
+  virtual void OnActionableError(const SyncProtocolError& error) = 0;
+
+  // This event is sent when scheduler decides to wait before next request
+  // either because it gets throttled by server or because it backs off after
+  // request failure. Retry time is passed in retry_time field of event.
+  virtual void OnRetryTimeChanged(base::Time retry_time) = 0;
+
+  // This event is sent when types are throttled or unthrottled.
+  virtual void OnThrottledTypesChanged(ModelTypeSet throttled_types) = 0;
+
  protected:
   virtual ~SyncEngineEventListener() {}
 };