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

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

 // Copyright 2016 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_CHROMEOS_ARC_FILEAPI_ARC_DOCUMENTS_PROVIDER_ROOT_H_
 #define CHROME_BROWSER_CHROMEOS_ARC_FILEAPI_ARC_DOCUMENTS_PROVIDER_ROOT_H_
 
 #include <stdint.h>
 
 #include <map>
@@ skipping 127 matching lines @@
                                mojom::DocumentPtr document);
 
   void ReadDirectoryWithDocumentId(const ReadDirectoryCallback& callback,
                                    const std::string& document_id);
   void ReadDirectoryWithNameToThinDocumentMap(
       const ReadDirectoryCallback& callback,
       base::File::Error error,
       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);
 
   // Resolves |path| to a document ID. Failures are indicated by an empty
   // document ID.
   void ResolveToDocumentId(const base::FilePath& path,
                            const ResolveToDocumentIdCallback& callback);
   void ResolveToDocumentIdRecursively(
       const std::string& document_id,
@@ skipping 22 matching lines @@
   // OnWatchersCleared() callback. Such watchers are still tracked here,
   // but they are not known by the remote service, so they are represented
   // by the invalid watcher ID (-1).
   //
   // Note that we do not use a document ID as a key here to guarantee that
   // a watch installed by AddWatcher() can be always identified in
   // RemoveWatcher() with the same file path specified.
   // 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>
       observer_wrapper_;
 
   base::WeakPtrFactory<ArcDocumentsProviderRoot> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(ArcDocumentsProviderRoot);
 };
 
 }  // namespace arc
 
 #endif  // CHROME_BROWSER_CHROMEOS_ARC_FILEAPI_ARC_DOCUMENTS_PROVIDER_ROOT_H_