Add RemoteFileSyncService interface

chrome/browser/sync_file_system/remote_file_provider.h

+// 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 "webkit/fileapi/syncable/sync_status_code.h"
+
+class FilePath;
+class GURL;
+
+namespace fileapi {
+class FileSystemURL;
+}
+
+namespace sync_file_system {
+
+class RemoteFileProvider;
+
+class RemoteChange {
+ public:
+  // Returns URL that the change happened at.
+  virtual const fileapi::FileSystemURL& url() = 0;
+
+  // Returns true if the change deleted the file.
+  virtual bool deleted() = 0;

[kinuko, 2012/10/17 09:22:57]

Is it possible to reuse fileapi::FileChange for this purpose?
Does a remote change have distinction between kind=file or folder?

[tzik, 2012/10/18 07:18:08]

I think it's true iff we use single provider.

When we advance to multi provider at some point, we'll need to add some extra
data for each change (e.g. md5, resource id, changestamp), and might want to
hide them from SyncService.

+};
+
+class RemoteFileObserver {
+ public:
+  // Called when RemoteFileProvider has pending RemoteChange.
+  virtual void OnRemoteChangeAvailable(
+      RemoteFileProvider* remote_file_provider) = 0;

[kinuko, 2012/10/17 09:22:57]

Maybe... for now I don't think we need this argument?

[tzik, 2012/10/18 07:18:08]

It's correct if we start from single provider, as we chatted.

+};
+
+class RemoteFileProvider {
+ public:
+  typedef base::Callback<void(fileapi::SyncStatusCode status,
+                              const FilePath& temp_file)> DownloadCallback;
+  typedef base::Callback<void(fileapi::SyncStatusCode status)> StatusCallback;
+
+  virtual void SetObserver(RemoteFileObserver* observer) = 0;

[kinuko, 2012/10/17 09:22:57]

Can you add a comment about the expected lifetime of the |observer|? (E.g. the
observer needs to be alive etc)

Also if we're using usual (non-thread-safe / non-task-runner-safe) observer
pattern I think AddObserver()/RemoveObserver() (used together with
base::ObserverList) is more common pattern

[tzik, 2012/10/18 07:18:08]

Done.

+  virtual void ResetObserver() = 0;
+
+  // Registers |origin| as tracking origin.
+  // After this method is called, GetRemoteChange() returns remote-side changes
+  // for the origin.
+  virtual void RegisterOrigin(const GURL& origin) = 0;
+
+  // Unregisters |origin| as tracking origin.
+  // After this method is called, GetRemoteChange() doesn't return remote-side

[kinuko, 2012/10/17 09:22:57]

... GetRemoteChange() won't include ... ?

+  // changes for the origin.
+  virtual void UnregisterOrigin(const GURL& origin) = 0;
+
+  // Returns true if it has a pending remote change.
+  virtual bool HasRemoteChange() = 0;
+
+  // Returns remote change for a tracking origin.
+  virtual scoped_ptr<RemoteChange> GetRemoteChange() = 0;

[kinuko, 2012/10/17 09:22:57]

Returns a remote change

Can you also comment what happens if it's called when HasRemoteChange == false?

[tzik, 2012/10/18 07:18:08]

Done.

+
+  // Notifies RemoveFileProvider of |change| is applied local filesystem.

[kinuko, 2012/10/17 09:22:57]

'Notifies' may be confusing with observer related methods.

"Tells this RemoteFileProvider that the |change| has been applied to the local
side." ...?

Or maybe simply we can name it DidProcessRemoteChange(...)

(Let's be consistent with terms in the method names: Change or RemoteChange)

[tzik, 2012/10/18 07:18:08]

Done.

+  virtual void ChangeApplied(const RemoteChange& change) = 0;
+
+  // Downloads the remote file for |url|. Upon completion, |callback| is
+  // invoked with status and temporary file.

[kinuko, 2012/10/17 09:22:57]

Please comment if the temporary file needs to be discarded on the caller side or
not.

[tzik, 2012/10/18 07:18:08]

Done.

+  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.

[kinuko, 2012/10/17 09:22:57]

Please comment if the caller needs to make sure file_path is unmodified while
upload is ongoing.

[tzik, 2012/10/18 07:18:08]

Done.

+  virtual void UploadFile(const fileapi::FileSystemURL& url,
+                          const FilePath& file_path,
+                          const StatusCallback& callback) = 0;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(RemoteFileProvider);
+};
+
+}  // namespace sync_file_system
+
+#endif  // CHROME_BROWSER_SYNC_FILE_SYSTEM_REMOTE_FILE_PROVIDER_H_