drive: Also lock SearchOperation when locking ChangeListLoader

chrome/browser/chromeos/drive/change_list_loader.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_CHROMEOS_DRIVE_CHANGE_LIST_LOADER_H_
 #define CHROME_BROWSER_CHROMEOS_DRIVE_CHANGE_LIST_LOADER_H_
 
 #include <map>
 #include <set>
 #include <string>
@@ skipping 28 matching lines @@
 class ResourceEntry;
 
 namespace internal {
 
 class ChangeList;
 class ChangeListLoaderObserver;
 class ChangeListProcessor;
 class DirectoryFetchInfo;
 class ResourceMetadata;
 
+// Delays execution of tasks as long as more than one lock is alive.
+// Used to ensure that ChangeListLoader does not cause race condition by adding
+// new entries created by sync tasks before they do.
+// All code which may add entries found on the server to the local metadata
+// should use this class.
+class LoaderController {
+ public:
+  LoaderController();
+  ~LoaderController();
+
+  // Increments the lock count and returns an object which decrements the count
+  // on its destruction.
+  // While the lock count is positive, tasks will be pending.
+  scoped_ptr<base::ScopedClosureRunner> GetLock();
+
+  // Runs the task if the lock count is 0, otherwise it will be pending.
+  void ScheduleRun(const base::Closure& task);
+
+ private:
+  // Decrements the lock count.
+  void Unlock();
+
+  int lock_count_;
+  std::vector<base::Closure> pending_tasks_;
+
+  base::WeakPtrFactory<LoaderController> weak_ptr_factory_;
+  DISALLOW_COPY_AND_ASSIGN(LoaderController);
+};
+
 // ChangeListLoader is used to load the change list, the full resource list,
 // and directory contents, from WAPI (codename for Documents List API)
 // or Google Drive API.  The class also updates the resource metadata with
 // the change list loaded from the server.
 //
 // Note that the difference between "resource list" and "change list" is
 // subtle hence the two words are often used interchangeably. To be precise,
 // "resource list" refers to metadata from the server when fetching the full
 // resource metadata, or fetching directory contents, whereas "change list"
 // refers to metadata from the server when fetching changes (delta).
 class ChangeListLoader {
  public:
   // Resource feed fetcher from the server.
   class FeedFetcher;
 
   ChangeListLoader(base::SequencedTaskRunner* blocking_task_runner,
                    ResourceMetadata* resource_metadata,
                    JobScheduler* scheduler,
-                   DriveServiceInterface* drive_service);
+                   DriveServiceInterface* drive_service,
+                   LoaderController* apply_task_controller);
   ~ChangeListLoader();
 
   // Indicates whether there is a request for full resource list or change
   // list fetching is in flight (i.e. directory contents fetching does not
   // count).
   bool IsRefreshing() const;
 
   // Adds and removes the observer.
   void AddObserver(ChangeListLoaderObserver* observer);
   void RemoveObserver(ChangeListLoaderObserver* observer);
@@ skipping 19 matching lines @@
                              const FileOperationCallback& callback);
 
   // Calls Load() with an empty DirectoryFetchInfo(). Only for testing purposes.
   void LoadForTesting(const FileOperationCallback& callback);
 
   // Gets the about resource from the cache or the server. If the cache is
   // availlavle, just runs |callback| with the cached about resource. If not,
   // calls |UpdateAboutResource| passing |callback|.
   void GetAboutResource(const google_apis::AboutResourceCallback& callback);
 
-  // Increments the lock count and returns an object which decrements the count
-  // on its destruction.
-  // While the lock count is positive, ChangeListLoader does not add any entries
-  // to the local metadata.
-  scoped_ptr<base::ScopedClosureRunner> GetLock();
-
  private:
   // Part of LoadDirectoryIfNeeded().
   void LoadDirectoryIfNeededAfterGetEntry(const base::FilePath& directory_path,
                                           const FileOperationCallback& callback,
                                           bool should_try_loading_parent,
                                           const ResourceEntry* entry,
                                           FileError error);
   void LoadDirectoryIfNeededAfterLoadParent(
       const base::FilePath& directory_path,
       const FileOperationCallback& callback,
@@ skipping 68 matching lines @@
       FileError error,
       ScopedVector<ChangeList> change_lists);
 
   // Part of LoadDirectoryFromServer().
   void LoadDirectoryFromServerAfterRefresh(
       const DirectoryFetchInfo& directory_fetch_info,
       const FileOperationCallback& callback,
       const base::FilePath* directory_path,
       FileError error);
 
-  // ================= Implementation for lock mechanism ====================
-  // Runs the closure if the lock count is 0, otherwise it will be pending.
-  // All tasks which may add entries should be run via this method.
-  void ApplyChange(const base::Closure& closure);
-
-  // Decrements the lock count.
-  void Unlock();
-
   // ================= Implementation for other stuff =================
 
   // Gets the about resource from the server, and caches it if successful. This
   // function calls JobScheduler::GetAboutResource internally. The cache will be
   // used in |GetAboutResource|.
   void UpdateAboutResource(
       const google_apis::AboutResourceCallback& callback);
   // Part of UpdateAboutResource().
   // This function should be called when the latest about resource is being
   // fetched from the server. The retrieved about resoure is cloned, and one is
   // cached and the other is passed to |callback|.
   void UpdateAboutResourceAfterGetAbout(
       const google_apis::AboutResourceCallback& callback,
       google_apis::GDataErrorCode status,
       scoped_ptr<google_apis::AboutResource> about_resource);
 
   scoped_refptr<base::SequencedTaskRunner> blocking_task_runner_;
   ResourceMetadata* resource_metadata_;  // Not owned.
   JobScheduler* scheduler_;  // Not owned.
   DriveServiceInterface* drive_service_;  // Not owned.
+  LoaderController* loader_controller_;  // Not owned.
   ObserverList<ChangeListLoaderObserver> observers_;
   typedef std::map<std::string, std::vector<FileOperationCallback> >
       LoadCallbackMap;
   LoadCallbackMap pending_load_callback_;
   FileOperationCallback pending_update_check_callback_;
 
-  int lock_count_;
-  std::vector<base::Closure> pending_apply_closures_;
-
   // Running feed fetcher.
   scoped_ptr<FeedFetcher> change_feed_fetcher_;
 
   // Set of the running feed fetcher for the fast fetch.
   std::set<FeedFetcher*> fast_fetch_feed_fetcher_set_;
 
   // The cache of the about resource.
   scoped_ptr<google_apis::AboutResource> cached_about_resource_;
 
   // True if the full resource list is loaded (i.e. the resource metadata is
   // stored locally).
   bool loaded_;
 
   // Note: This should remain the last member so it'll be destroyed and
   // invalidate its weak pointers before any other members are destroyed.
   base::WeakPtrFactory<ChangeListLoader> weak_ptr_factory_;
   DISALLOW_COPY_AND_ASSIGN(ChangeListLoader);
 };
 
 }  // namespace internal
 }  // namespace drive
 
 #endif  // CHROME_BROWSER_CHROMEOS_DRIVE_CHANGE_LIST_LOADER_H_