mediaview: Hack to avoid Files app to freeze soon after login.

chrome/browser/chromeos/arc/fileapi/arc_documents_provider_root.h

Index: chrome/browser/chromeos/arc/fileapi/arc_documents_provider_root.h
diff --git a/chrome/browser/chromeos/arc/fileapi/arc_documents_provider_root.h b/chrome/browser/chromeos/arc/fileapi/arc_documents_provider_root.h
index d2afb37f48a950f686c82c3bd9fdde2a1e55e71a..7855789cbafdb1c3da284990ee75be2ccace43e7 100644
--- a/chrome/browser/chromeos/arc/fileapi/arc_documents_provider_root.h
+++ b/chrome/browser/chromeos/arc/fileapi/arc_documents_provider_root.h
@@ -145,16 +145,19 @@ class ArcDocumentsProviderRoot : public ArcFileSystemOperationRunner::Observer {
       NameToThinDocumentMap mapping);
 
   void AddWatcherWithDocumentId(const base::FilePath& path,
+                                uint64_t watcher_request_id,
                                 const WatcherCallback& watcher_callback,
-                                const StatusCallback& callback,
                                 const std::string& document_id);
   void OnWatcherAdded(const base::FilePath& path,
-                      const StatusCallback& callback,
+                      uint64_t watcher_request_id,
                       int64_t watcher_id);
-  void OnWatcherAddedButRemoved(const StatusCallback& callback, bool success);
+  void OnWatcherAddedButRemoved(bool success);
 
   void OnWatcherRemoved(const StatusCallback& callback, bool success);
 
+  bool IsWatcherRequestCanceled(const base::FilePath& path,

[hidehiko, 2017/03/10 15:16:02]

"const" is missing. Could you write the document of this function?

BTW, this function returns "true" after OnWatcherAdded(), even if it is not
cancelled. Practically, it is not big problem, so could you document it as well?
(or, if you agree the change I proposed below, it is naturally addressed,
IIUC?).

[Shuhei Takahashi, 2017/03/14 05:42:47]

Done.
Yup, let's add comments.

+                                uint64_t watcher_request_id);
+
   void ResolveToContentUrlWithDocumentId(
       const ResolveToContentUrlCallback& callback,
       const std::string& document_id);
@@ -197,6 +200,20 @@ class ArcDocumentsProviderRoot : public ArcFileSystemOperationRunner::Observer {
   // See the documentation of AddWatcher() for more details.
   std::map<base::FilePath, int64_t> path_to_watcher_id_;
 
+  // Map from a file path to an in-flight watcher request ID.
+  //
+  // While AddWatcher() is in-flight, a unique request ID is set here.
+  //
+  // Invariant: for any |path|, an entry for |path| may exist in

[hidehiko, 2017/03/10 15:16:02]

Optional: Instead of having this restriction, how about;

struct WatcherData {  // any better name?
  const int64_t request_id;
  int64_t id;  // watcher ID returned from container.
};
std::map<base::FilePath, WatcherData> path_to_watcher_data_;

etc.?

Then you can simply track Add/RemoveWatcher() calls. WDYT?

[Shuhei Takahashi, 2017/03/14 05:42:47]

Sounds good, let's do that!

+  // |path_to_watcher_id_| or |path_to_watcher_request_id_|, but not in both.
+  //
+  // TODO(crbug.com/698624): Remove this hack. It was introduced because Files
+  // app freezes until AddWatcher() finishes, but it should be handled in Files
+  // app rather than here.
+  std::map<base::FilePath, uint64_t> path_to_watcher_request_id_;
+
+  uint64_t next_watcher_request_id_ = 1;
+
   // Can be null if this instance is not observing ArcFileSystemOperationRunner.
   // Observation is started on the first call of AddWatcher().
   scoped_refptr<file_system_operation_runner_util::ObserverIOThreadWrapper>