drive: Stop making multiple GetAppList() requests from DriveAppRegistry

Calling DriveAppRegistry::Update() results in being ignored when there is an ongoing one already.
Add FakeDriveService::app_list_load_count()

BUG=268707
TEST=unit_tests
R=hidehiko@chromium.org, kinaba@chromium.org

Review URL: https://codereview.chromium.org/22297004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@215806 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 37 insertions, 4 deletions

diff --git a/chrome/browser/chromeos/drive/drive_app_registry.cc b/chrome/browser/chromeos/drive/drive_app_registry.cc
index 75b0418..9e4bae6 100644
--- a/chrome/browser/chromeos/drive/drive_app_registry.cc
+++ b/chrome/browser/chromeos/drive/drive_app_registry.cc
@@ -95,6 +95,7 @@ DriveAppRegistry::DriveAppFileSelector::~DriveAppFileSelector() {
 
 DriveAppRegistry::DriveAppRegistry(JobScheduler* scheduler)
     : scheduler_(scheduler),
+      is_updating_(false),
       weak_ptr_factory_(this) {
 }
 
@@ -137,6 +138,11 @@ void DriveAppRegistry::GetAppsForFile(
 void DriveAppRegistry::Update() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
+  if (is_updating_)  // There is already an update in progress.
+    return;
+
+  is_updating_ = true;
+
   scheduler_->GetAppList(
       base::Bind(&DriveAppRegistry::UpdateAfterGetAppList,
                  weak_ptr_factory_.GetWeakPtr()));
@@ -147,6 +153,9 @@ void DriveAppRegistry::UpdateAfterGetAppList(
     scoped_ptr<google_apis::AppList> app_list) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
+  DCHECK(is_updating_);
+  is_updating_ = false;
+
   FileError error = GDataToFileError(gdata_error);
   if (error != FILE_ERROR_OK) {
     // Failed to fetch the data from the server. We can do nothing here.
diff --git a/chrome/browser/chromeos/drive/drive_app_registry.h b/chrome/browser/chromeos/drive/drive_app_registry.h
index 935c61a..2b89a41 100644
--- a/chrome/browser/chromeos/drive/drive_app_registry.h
+++ b/chrome/browser/chromeos/drive/drive_app_registry.h
@@ -22,7 +22,7 @@ class FilePath;
 
 namespace google_apis {
 class AppList;
-}  // namespace AppList
+}  // namespace google_apis
 
 namespace drive {
 
@@ -145,6 +145,8 @@ class DriveAppRegistry {
   // Map of MIME type to application info.
   DriveAppFileSelectorMap app_mimetypes_map_;
 
+  bool is_updating_;
+
   // Note: This should remain the last member so it'll be destroyed and
   // invalidate the weak pointers before any other members are destroyed.
   base::WeakPtrFactory<DriveAppRegistry> weak_ptr_factory_;
diff --git a/chrome/browser/chromeos/drive/drive_app_registry_unittest.cc b/chrome/browser/chromeos/drive/drive_app_registry_unittest.cc
index 6228f42..eacfae6 100644
--- a/chrome/browser/chromeos/drive/drive_app_registry_unittest.cc
+++ b/chrome/browser/chromeos/drive/drive_app_registry_unittest.cc
@@ -30,8 +30,6 @@ class DriveAppRegistryTest : public testing::Test {
                                       base::MessageLoopProxy::current().get()));
 
     web_apps_registry_.reset(new DriveAppRegistry(scheduler_.get()));
-    web_apps_registry_->Update();
-    base::RunLoop().RunUntilIdle();
   }
 
   bool VerifyApp(const ScopedVector<DriveAppInfo>& list,
@@ -54,7 +52,7 @@ class DriveAppRegistryTest : public testing::Test {
       }
     }
     EXPECT_TRUE(found) << "Unable to find app with web_store_id "
-        << web_store_id;
+                       << web_store_id;
     return found;
   }
 
@@ -66,6 +64,9 @@ class DriveAppRegistryTest : public testing::Test {
 };
 
 TEST_F(DriveAppRegistryTest, LoadAndFindDriveApps) {
+  web_apps_registry_->Update();
+  base::RunLoop().RunUntilIdle();
+
   // Find by primary extension 'exe'.
   ScopedVector<DriveAppInfo> ext_results;
   base::FilePath ext_file(FILE_PATH_LITERAL("drive/file.exe"));
@@ -91,4 +92,17 @@ TEST_F(DriveAppRegistryTest, LoadAndFindDriveApps) {
             "Drive app 1", "", false);
 }
 
+TEST_F(DriveAppRegistryTest, MultipleUpdate) {
+  // Call Update().
+  web_apps_registry_->Update();
+
+  // Call Update() again.
+  // This call should be ignored because there is already an ongoing update.
+  web_apps_registry_->Update();
+
+  // The app list should be loaded only once.
+  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(1, fake_drive_service_->app_list_load_count());
+}
+
 }  // namespace drive
diff --git a/chrome/browser/drive/fake_drive_service.cc b/chrome/browser/drive/fake_drive_service.cc
index 80816f4..1729966 100644
--- a/chrome/browser/drive/fake_drive_service.cc
+++ b/chrome/browser/drive/fake_drive_service.cc
@@ -165,6 +165,7 @@ FakeDriveService::FakeDriveService()
       change_list_load_count_(0),
       directory_load_count_(0),
       about_resource_load_count_(0),
+      app_list_load_count_(0),
       offline_(false) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 }
@@ -551,6 +552,7 @@ CancelCallback FakeDriveService::GetAppList(
     return CancelCallback();
   }
 
+  ++app_list_load_count_;
   scoped_ptr<AppList> app_list(AppList::CreateFrom(*app_info_value_));
   base::MessageLoop::current()->PostTask(
       FROM_HERE,
diff --git a/chrome/browser/drive/fake_drive_service.h b/chrome/browser/drive/fake_drive_service.h
index 9b1a4e5..1a1e5d9 100644
--- a/chrome/browser/drive/fake_drive_service.h
+++ b/chrome/browser/drive/fake_drive_service.h
@@ -68,6 +68,10 @@ class FakeDriveService : public DriveServiceInterface {
     return about_resource_load_count_;
   }
 
+  // Returns the number of times the app list is successfully loaded by
+  // GetAppList().
+  int app_list_load_count() const { return app_list_load_count_; }
+
   // Returns the file path whose request is cancelled just before this method
   // invocation.
   const base::FilePath& last_cancelled_file() const {
@@ -272,6 +276,7 @@ class FakeDriveService : public DriveServiceInterface {
   int change_list_load_count_;
   int directory_load_count_;
   int about_resource_load_count_;
+  int app_list_load_count_;
   bool offline_;
   base::FilePath last_cancelled_file_;
 
diff --git a/chrome/browser/drive/fake_drive_service_unittest.cc b/chrome/browser/drive/fake_drive_service_unittest.cc
index ea988fe..51fdce5 100644
--- a/chrome/browser/drive/fake_drive_service_unittest.cc
+++ b/chrome/browser/drive/fake_drive_service_unittest.cc
@@ -738,6 +738,7 @@ TEST_F(FakeDriveServiceTest, GetAppList) {
   EXPECT_EQ(HTTP_SUCCESS, error);
 
   ASSERT_TRUE(app_list);
+  EXPECT_EQ(1, fake_service_.app_list_load_count());
 }
 
 TEST_F(FakeDriveServiceTest, GetAppList_Offline) {