[ew] Add basic classes.

webkit/browser/fileapi/watcher_manager.h

+// Copyright 2014 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 WEBKIT_BROWSER_FILEAPI_WATCHER_MANAGER_H_
+#define WEBKIT_BROWSER_FILEAPI_WATCHER_MANAGER_H_
+
+#include <vector>
+
+#include "base/basictypes.h"
+#include "base/callback_forward.h"
+#include "base/files/file.h"
+#include "base/files/file_util_proxy.h"
+#include "base/memory/scoped_ptr.h"
+#include "webkit/browser/fileapi/file_system_operation.h"
+#include "webkit/browser/webkit_storage_browser_export.h"
+#include "webkit/common/fileapi/directory_entry.h"
+
+namespace base {
+class Time;
+}
+
+namespace fileapi {
+
+class FileSystemOperationContext;
+class FileSystemURL;
+
+// An interface for providing entry observing capability for file system
+// backends.
+//
+// It is NOT valid to give null callback to this class, and implementors
+// can assume that they don't get any null callbacks.
+class WatcherManager {
+ public:
+  typedef base::Callback<void(base::File::Error result)> StatusCallback;
+
+  // Observes watched entries.
+  class Observer {
+   public:
+    Observer() {}
+    virtual ~Observer() {}
+
+    // Notifies about an entry represented by |url| being changed.
+    virtual void OnEntryChanged(const FileSystemURL& url) = 0;
+
+    // Notifies about an entry represented by |url| being removed.
+    virtual void OnEntryRemoved(const FileSystemURL& url) = 0;
+  };
+
+  virtual ~WatcherManager() {}
+
+  virtual void AddObserver(Observer* observer) = 0;
+  virtual void RemoveObserver(Observer* observer) = 0;
+  virtual bool HasObserver(Observer* observer) const = 0;
+
+  // Observes a directory entry. If the |recursive| mode is not supported then
+  // FILE_ERROR_INVALID_OPERATION must be returned as an error. If the |url| is
+  // already watched, or setting up the watcher fails, then |callback|

[kinaba, 2014/08/12 09:15:37]

I have some questions on the "if already watched => error" case.

Is the restriction also posed on JS-level user code? If so, it sounds a bit
inconvenient that you cannot set two ore more listeners on a single entry.

Does it have to be checked on each WatcherManager instance? What about rejecting
such a case in EntryWatcherService and allow WatcherManager to assume there's no
duplication?

[mtomasz, 2014/08/12 09:31:37]

I was thinking about it a lot, and I couldn't find a better solution.
Basically, I wanted notifications to be sent using events instead of
callback, since callbacks are killed suspending an event page.

Since we have events, then there is no way to create a watcher on the same
event multiple times. If we allowed that, then we still would get one event
invocation per change.

Note, that we can have multiple listeners on these events.
I tried it, but the code became very complicated. Basically, WatcherManager's
methods are asynchronous, so setting or removing a watcher may require some
rounds
of PostTasks.

If we checked for duplication in EntryWatcherService *before creating*, then we
could still end up on creating a duplicate.

Eg.
1. Request creating a watcher on /A.
2. Request creating a watcher on /A. Not created yet, so go ahead.
3. Watcher on /A created.
4. Watcher on /A created (duplicate).

To avoid that we could create some vector of watchers being created, but not
created yet. That wouldn't work though, since even if a watcher is being
created,
its creation may fail. So the second request can't be discarded just because
the same watcher is being created, because it may succeed this time.

Another solution would be to use a SequencedTaskRunner and enqueue all watcher
creation calls. However, this makes code complicated and slower if setting
watchers
is slow (eg. for File System Provider API it may require some HTTP call).

So, I thought that just putting responsibility on backends is the simplest
solution.

WDYT?

[kinaba, 2014/08/14 05:22:49]

Thanks for the explanation. Made sense to me.
I'm still not convinced. Wouldn't it be just shifting the difficulty from
EntryWatcherService to each backend? In the very same way that
EntryWatcherService
has to manage asynchronous tasks in a backend nicely, the backend has to manage
asynchronous work in its implementation (e.g., extensions, or ultimately,
some cloud API in case of FSP) if the backend has to check duplication.

I think the first "solution" is acceptable. In other words, it is ok to reject
the second request if some request is already in-flight for the same URL.

Well, however, either way, I don't have strong opinion for now, since we
anyhow impose the no-dup restriction. Let's see how it goes in the backend
implementation, and later come back here if needed.

[mtomasz, 2014/08/15 05:35:56]

How about doing it in the service, but with a SequencedWorkerPool created per
unique [url, extension_id] pair? That would be efficient and race-less. Also,
not
so complicated in implementation. WDYT?

[kinaba, 2014/08/18 02:32:15]

Sounds good.

+  // must be called with a specific error code. Otherwise |callback| must be
+  // called with the FILE_OK error code.
+  virtual void WatchDirectory(const FileSystemURL& url,
+                              bool recursive,
+                              const StatusCallback& callback) = 0;
+
+  // Stops observing an entry represented by |url|.
+  virtual void UnwatchEntry(const FileSystemURL& url,
+                            const StatusCallback& callback) = 0;
+};
+
+}  // namespace fileapi
+
+#endif  // WEBKIT_BROWSER_FILEAPI_WATCHER_MANAGER_H_