Add RemoteFileSyncService interface

chrome/browser/sync_file_system/remote_file_provider.h

Index: chrome/browser/sync_file_system/remote_file_provider.h
diff --git a/chrome/browser/sync_file_system/remote_file_provider.h b/chrome/browser/sync_file_system/remote_file_provider.h
new file mode 100644
index 0000000000000000000000000000000000000000..7eabefbbef9645bf472f58716716ff84e95c5c37
--- /dev/null
+++ b/chrome/browser/sync_file_system/remote_file_provider.h
@@ -0,0 +1,115 @@
+// 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.
+
+#ifndef CHROME_BROWSER_SYNC_FILE_SYSTEM_REMOTE_FILE_PROVIDER_H_
+#define CHROME_BROWSER_SYNC_FILE_SYSTEM_REMOTE_FILE_PROVIDER_H_
+
+#include "base/basictypes.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/callback_forward.h"
+#include "webkit/fileapi/syncable/sync_status_code.h"
+
+class FilePath;
+class GURL;
+
+namespace fileapi {
+class FileSystemURL;
+}
+
+namespace sync_file_system {
+
+class RemoteChangeProcessor {
+ public:
+  RemoteChangeProcessor();
+  virtual ~RemoteChangeProcessor();
+
+  // Called when RemoteFileProvider has pending remote change.
+  virtual void OnRemoteChangeAvailable() = 0;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(RemoteChangeProcessor);
+};
+
+// This class represents the changelist of the remote files.

[kinuko, 2012/10/18 13:54:32]

This class provides access to the remote side changes. ?

+class RemoteChangeProvider {
+ public:
+  // |processor| needs alive until RemoteChangeProvider destroyed.
+  explicit RemoteChangeProvider(RemoteChangeProcessor* processor);
+  virtual ~RemoteChangeProvider();
+
+  // Registers |origin| as a tracking origin.
+  // After this method is called, the representing changelist includes
+  // remote file changes for |origin|.

[kinuko, 2012/10/18 13:54:32]

Registers |origin| to track remote side changes for the |origin|. ?

[tzik, 2012/10/23 04:19:32]

Done.

+  virtual void RegisterOrigin(const GURL& origin) = 0;

[kinuko, 2012/10/18 13:54:32]

On the second thought I may prefer having this on RemoteFileProvider.  Or we
should make it clearer if each method is for a current change or for the
provider itself.

Also in the current interface it's unclear when we populate a next change...

I think I still like a simpler traditional design like following:

class RemoteChange {
  bool IsDeletion();
  bool IsDirectory();
  FileSystemURL& GetURL();
};

class RemoteFileSyncService {
  explicit RemoteFileSyncService(RemoteChangeObserver* observer);

  void RegisterOriginForTrackChanges(origin);
  void UnregisterOriginForTrackChanges(origin);

  // Returns a next available remote change.
  // This returns NULL if no changes are available.
  // The returned change becomes invalid once FinalizeChange() is called on this
  // change or after this service gets killed.
  // Also calling this again without finalizing or ungetting the returned change
  // (by calling FinalizeChange or UngetChange) is invalid.
  // The ownership of the returned change is owned by this service and must not
  // be deleted by the caller.
  RemoteChange* GetNextChange();

  // Finalizes the given change.
  // This must be called before next GetNextChange() is called.
  void FinalizeChange(RemoteChange*);

  // Ungets the given change. This is useful when the consumer of the change
  // is not ready for process and wants to revisit the change later.
  void UngetChange(RemoteChange*);

  // ...
};

WDYT? Let me know if I'm missing something.
(I changed some class names or reverted some name changes; let's talk offline
about them... sorry for not being really decisive)

+
+  // Unregisters |origin| as a tracking origin.
+  // After this method is called, the representing changelist no longer includes
+  // remote file changes for |origin|.

[kinuko, 2012/10/18 13:54:32]

Unregisters |origin| so that this provider no longer returns changes for the
|origin|.

[tzik, 2012/10/23 04:19:32]

Done.

+  virtual void UnregisterOrigin(const GURL& origin) = 0;
+
+  // Returns true if it has a pending remote change.
+  virtual bool HasRemoteChange() = 0;

[kinuko, 2012/10/18 13:54:32]

nit: Change -> Changes ?

+
+  // Returns URL where the current remote change was happend at.
+  // This requires HasRemoteChange() == true.
+  virtual const fileapi::FileSystemURL& GetURLForCurrentChange() = 0;
+
+  // Return true if the current remote change deleted the file.
+  // This requires HasRemoteChange() == true.
+  virtual bool IsDeletionChange() = 0;

[kinuko, 2012/10/18 13:54:32]

How do we know if a change is for a file or a directory?

[tzik, 2012/10/19 04:32:07]

Change data from Drive server seems not containing mime-type, which is needed to
determine the file was file or directory.
Is it needed for proceeding RemoteSync?

If it's needed, we can add a field to database entry to store that.

+
+  // Tells we have processed the current remote change.
+  // This requires HasRemoteChange() == true.
+  virtual void DidProcessRemoteChange() = 0;
+
+ protected:
+  void NotifyChangeAvailable();
+
+ private:
+  RemoteChangeProcessor* const processor_;
+
+  DISALLOW_COPY_AND_ASSIGN(RemoteChangeProvider);
+};
+
+class RemoteFileProvider {

[kinuko, 2012/10/18 13:54:32]

For consistency & since this interface is still specific to the sync service can
we rename this to RemoteFileSyncService?
(Or RemoteFileSyncBackend maybe)

[tzik, 2012/10/23 04:19:32]

Done.

+ public:
+  typedef base::Callback<void(fileapi::SyncStatusCode status,
+                              const FilePath& temp_file)> DownloadCallback;
+  typedef base::Callback<void(fileapi::SyncStatusCode status)> StatusCallback;
+
+  explicit RemoteFileProvider(
+      scoped_ptr<RemoteChangeProvider> remote_change_provider);
+  virtual ~RemoteFileProvider();
+
+  // Downloads the remote file for |url|. Upon completion, |callback| is
+  // invoked with status and temporary file.
+  // Its caller need to delete the temporary file after it processed locally.
+  virtual void DownloadFile(const fileapi::FileSystemURL& url,
+                            const DownloadCallback& callback) = 0;
+
+  // Deletes the remote file for |url|. Upon completion, |callback| is
+  // invoked with status.
+  virtual void DeleteFile(const fileapi::FileSystemURL& url,
+                          const StatusCallback& callback) = 0;
+
+  // Upload the file at |file_path| and update the remote file for |url|.
+  // Upon completion, |callback| is invoked with status.
+  // The caller must guarantee the file does not modified until its completion.
+  virtual void UploadFile(const fileapi::FileSystemURL& url,
+                          const FilePath& file_path,
+                          const StatusCallback& callback) = 0;
+
+  RemoteChangeProvider* change_provider() {
+    return remote_change_provider_.get();
+  }
+
+ private:
+  scoped_ptr<RemoteChangeProvider> remote_change_provider_;
+
+  DISALLOW_COPY_AND_ASSIGN(RemoteFileProvider);
+};
+
+}  // namespace sync_file_system
+
+#endif  // CHROME_BROWSER_SYNC_FILE_SYSTEM_REMOTE_FILE_PROVIDER_H_