Less dependency for DriveAppRegistry.

- It now directly accesses DriveServiceInterface instead of JobScheduler
  so that the class can be used outside chromeos/.
- Use of ScopedVector is replaced by std::vector.

BUG=332014,312566

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

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

chrome/browser/chromeos/drive/drive_app_registry.cc

@@ -5,14 +5,11 @@
 #include "chrome/browser/chromeos/drive/drive_app_registry.h"
 
 #include <algorithm>
-#include <string>
+#include <set>
 #include <utility>
-#include <vector>
 
 #include "base/files/file_path.h"
-#include "base/strings/string_util.h"
-#include "chrome/browser/chromeos/drive/file_system_util.h"
-#include "chrome/browser/chromeos/drive/job_scheduler.h"
+#include "chrome/browser/drive/drive_service_interface.h"
 #include "content/public/browser/browser_thread.h"
 #include "google_apis/drive/drive_api_parser.h"
 
@@ -61,8 +58,8 @@ DriveAppInfo::DriveAppInfo(
 DriveAppInfo::~DriveAppInfo() {
 }
 
-DriveAppRegistry::DriveAppRegistry(JobScheduler* scheduler)
-    : scheduler_(scheduler),
+DriveAppRegistry::DriveAppRegistry(DriveServiceInterface* drive_service)
+    : drive_service_(drive_service),
       is_updating_(false),
       weak_ptr_factory_(this) {
 }
@@ -73,7 +70,7 @@ DriveAppRegistry::~DriveAppRegistry() {
 void DriveAppRegistry::GetAppsForFile(
     const base::FilePath::StringType& file_extension,
     const std::string& mime_type,
-    ScopedVector<DriveAppInfo>* apps) const {
+    std::vector<DriveAppInfo>* apps) const {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   std::vector<std::string> matched_apps;
@@ -92,7 +89,7 @@ void DriveAppRegistry::GetAppsForFile(
       std::map<std::string, DriveAppInfo>::const_iterator it =
           all_apps_.find(matched_apps[i]);
       DCHECK(it != all_apps_.end());
-      apps->push_back(new DriveAppInfo(it->second));
+      apps->push_back(it->second);
     }
   }
 }
@@ -104,8 +101,9 @@ void DriveAppRegistry::Update() {
     return;
   is_updating_ = true;
 
-  scheduler_->GetAppList(base::Bind(&DriveAppRegistry::UpdateAfterGetAppList,
-                                    weak_ptr_factory_.GetWeakPtr()));
+  drive_service_->GetAppList(
+      base::Bind(&DriveAppRegistry::UpdateAfterGetAppList,
+                 weak_ptr_factory_.GetWeakPtr()));
 }
 
 void DriveAppRegistry::UpdateAfterGetAppList(
@@ -116,11 +114,9 @@ void DriveAppRegistry::UpdateAfterGetAppList(
   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.
+  // Failed to fetch the data from the server. We can do nothing here.
+  if (gdata_error != google_apis::HTTP_SUCCESS)
     return;
-  }
 
   DCHECK(app_list);
   UpdateFromAppList(*app_list);

chrome/browser/chromeos/drive/drive_app_registry.h

@@ -11,7 +11,6 @@
 
 #include "base/files/file_path.h"
 #include "base/memory/scoped_ptr.h"
-#include "base/memory/scoped_vector.h"
 #include "base/memory/weak_ptr.h"
 #include "google_apis/drive/gdata_errorcode.h"
 #include "google_apis/drive/gdata_wapi_parser.h"
@@ -23,7 +22,7 @@ class AppList;
 
 namespace drive {
 
-class JobScheduler;
+class DriveServiceInterface;
 
 // Data structure that defines Drive app. See
 // https://chrome.google.com/webstore/category/collection/drive_apps for
@@ -47,21 +46,22 @@ struct DriveAppInfo {
   google_apis::InstalledApp::IconList document_icons;
   // App name.
   std::string app_name;
-  // URL for opening a new file in the app.
+  // URL for opening a new file in the app. Empty if the app does not support
+  // new file creation.
   GURL create_url;
 };
 
 // Keeps the track of installed drive applications in-memory.
 class DriveAppRegistry {
  public:
-  explicit DriveAppRegistry(JobScheduler* scheduler);
+  explicit DriveAppRegistry(DriveServiceInterface* scheduler);
   ~DriveAppRegistry();
 
   // Returns a list of Drive app information for the |file_extension| with
   // |mime_type|.
   void GetAppsForFile(const base::FilePath::StringType& file_extension,
                       const std::string& mime_type,
-                      ScopedVector<DriveAppInfo>* apps) const;
+                      std::vector<DriveAppInfo>* apps) const;
 
   // Updates this registry by fetching the data from the server.
   void Update();
@@ -84,7 +84,7 @@ class DriveAppRegistry {
   DriveAppFileSelectorMap extension_map_;
   DriveAppFileSelectorMap mimetype_map_;
 
-  JobScheduler* scheduler_;
+  DriveServiceInterface* drive_service_;
 
   bool is_updating_;
 

chrome/browser/chromeos/drive/drive_app_registry_unittest.cc

@@ -5,10 +5,7 @@
 #include "chrome/browser/chromeos/drive/drive_app_registry.h"
 
 #include "base/files/file_path.h"
-#include "base/prefs/testing_pref_service.h"
 #include "base/run_loop.h"
-#include "chrome/browser/chromeos/drive/job_scheduler.h"
-#include "chrome/browser/chromeos/drive/test_util.h"
 #include "chrome/browser/drive/fake_drive_service.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "google_apis/drive/drive_api_parser.h"
@@ -21,28 +18,20 @@ namespace drive {
 class DriveAppRegistryTest : public testing::Test {
  protected:
   virtual void SetUp() OVERRIDE {
-    pref_service_.reset(new TestingPrefServiceSimple);
-    test_util::RegisterDrivePrefs(pref_service_->registry());
-
     fake_drive_service_.reset(new FakeDriveService);
     fake_drive_service_->LoadAppListForDriveApi("drive/applist.json");
 
-    scheduler_.reset(new JobScheduler(pref_service_.get(),
-                                      fake_drive_service_.get(),
-                                      base::MessageLoopProxy::current().get()));
-
-    web_apps_registry_.reset(new DriveAppRegistry(scheduler_.get()));
+    apps_registry_.reset(new DriveAppRegistry(fake_drive_service_.get()));
   }
 
-  bool VerifyApp(const ScopedVector<DriveAppInfo>& list,
+  bool VerifyApp(const std::vector<DriveAppInfo>& list,
                  const std::string& app_id,
                  const std::string& app_name) {
     bool found = false;
-    for (ScopedVector<DriveAppInfo>::const_iterator it = list.begin();
-         it != list.end(); ++it) {
-      const DriveAppInfo* app = *it;
-      if (app_id == app->app_id) {
-        EXPECT_EQ(app_name, app->app_name);
+    for (size_t i = 0; i < list.size(); ++i) {
+      const DriveAppInfo& app = list[i];
+      if (app_id == app.app_id) {
+        EXPECT_EQ(app_name, app.app_name);
         found = true;
         break;
       }
@@ -52,33 +41,31 @@ class DriveAppRegistryTest : public testing::Test {
   }
 
   content::TestBrowserThreadBundle thread_bundle_;
-  scoped_ptr<TestingPrefServiceSimple> pref_service_;
   scoped_ptr<FakeDriveService> fake_drive_service_;
-  scoped_ptr<JobScheduler> scheduler_;
-  scoped_ptr<DriveAppRegistry> web_apps_registry_;
+  scoped_ptr<DriveAppRegistry> apps_registry_;
 };
 
 TEST_F(DriveAppRegistryTest, LoadAndFindDriveApps) {
-  web_apps_registry_->Update();
+  apps_registry_->Update();
   base::RunLoop().RunUntilIdle();
 
   // Find by primary extension 'exe'.
-  ScopedVector<DriveAppInfo> ext_results;
+  std::vector<DriveAppInfo> ext_results;
   base::FilePath ext_file(FILE_PATH_LITERAL("drive/file.exe"));
-  web_apps_registry_->GetAppsForFile(ext_file.Extension(), "", &ext_results);
+  apps_registry_->GetAppsForFile(ext_file.Extension(), "", &ext_results);
   ASSERT_EQ(1U, ext_results.size());
   VerifyApp(ext_results, "123456788192", "Drive app 1");
 
   // Find by primary MIME type.
-  ScopedVector<DriveAppInfo> primary_app;
-  web_apps_registry_->GetAppsForFile(base::FilePath::StringType(),
+  std::vector<DriveAppInfo> primary_app;
+  apps_registry_->GetAppsForFile(base::FilePath::StringType(),
       "application/vnd.google-apps.drive-sdk.123456788192", &primary_app);
   ASSERT_EQ(1U, primary_app.size());
   VerifyApp(primary_app, "123456788192", "Drive app 1");
 
   // Find by secondary MIME type.
-  ScopedVector<DriveAppInfo> secondary_app;
-  web_apps_registry_->GetAppsForFile(
+  std::vector<DriveAppInfo> secondary_app;
+  apps_registry_->GetAppsForFile(
       base::FilePath::StringType(), "text/html", &secondary_app);
   ASSERT_EQ(1U, secondary_app.size());
   VerifyApp(secondary_app, "123456788192", "Drive app 1");
@@ -90,22 +77,22 @@ TEST_F(DriveAppRegistryTest, UpdateFromAppList) {
   scoped_ptr<google_apis::AppList> app_list(
       google_apis::AppList::CreateFrom(*app_info_value));
 
-  web_apps_registry_->UpdateFromAppList(*app_list);
+  apps_registry_->UpdateFromAppList(*app_list);
 
   // Confirm that something was loaded from applist.json.
-  ScopedVector<DriveAppInfo> ext_results;
+  std::vector<DriveAppInfo> ext_results;
   base::FilePath ext_file(FILE_PATH_LITERAL("drive/file.exe"));
-  web_apps_registry_->GetAppsForFile(ext_file.Extension(), "", &ext_results);
+  apps_registry_->GetAppsForFile(ext_file.Extension(), "", &ext_results);
   ASSERT_EQ(1U, ext_results.size());
 }
 
 TEST_F(DriveAppRegistryTest, MultipleUpdate) {
   // Call Update().
-  web_apps_registry_->Update();
+  apps_registry_->Update();
 
   // Call Update() again.
   // This call should be ignored because there is already an ongoing update.
-  web_apps_registry_->Update();
+  apps_registry_->Update();
 
   // The app list should be loaded only once.
   base::RunLoop().RunUntilIdle();

chrome/browser/chromeos/drive/drive_integration_service.cc

@@ -253,7 +253,7 @@ DriveIntegrationService::DriveIntegrationService(
       cache_root_directory_.Append(kCacheFileDirectory),
       blocking_task_runner_.get(),
       NULL /* free_disk_space_getter */));
-  drive_app_registry_.reset(new DriveAppRegistry(scheduler_.get()));
+  drive_app_registry_.reset(new DriveAppRegistry(drive_service_.get()));
 
   resource_metadata_.reset(new internal::ResourceMetadata(
       metadata_storage_.get(), blocking_task_runner_));

chrome/browser/chromeos/extensions/file_manager/private_api_drive.cc

@@ -144,7 +144,7 @@ void FileBrowserPrivateGetDriveEntryPropertiesFunction::OnGetFileInfo(
 
   // Get drive WebApps that can accept this file. We just need to extract the
   // doc icon for the drive app, which is set as default.
-  ScopedVector<drive::DriveAppInfo> drive_apps;
+  std::vector<drive::DriveAppInfo> drive_apps;
   app_registry->GetAppsForFile(file_path_.Extension(),
                                file_specific_info.content_mime_type(),
                                &drive_apps);
@@ -158,11 +158,11 @@ void FileBrowserPrivateGetDriveEntryPropertiesFunction::OnGetFileInfo(
     file_manager::file_tasks::ParseTaskID(default_task_id, &default_task);
     DCHECK(default_task_id.empty() || !default_task.app_id.empty());
     for (size_t i = 0; i < drive_apps.size(); ++i) {
-      const drive::DriveAppInfo* app_info = drive_apps[i];
-      if (default_task.app_id == app_info->app_id) {
+      const drive::DriveAppInfo& app_info = drive_apps[i];
+      if (default_task.app_id == app_info.app_id) {
         // The drive app is set as default. Files.app should use the doc icon.
         const GURL doc_icon =
-            drive::util::FindPreferredIcon(app_info->document_icons,
+            drive::util::FindPreferredIcon(app_info.document_icons,
                                            drive::util::kPreferredIconSize);
         properties_->custom_icon_url.reset(new std::string(doc_icon.spec()));
       }

chrome/browser/chromeos/file_manager/file_tasks.cc

@@ -330,23 +330,21 @@ void FindDriveAppTasks(
     if (!drive::util::IsUnderDriveMountPoint(file_path))
       return;
 
-    ScopedVector<drive::DriveAppInfo> app_info_list;
+    std::vector<drive::DriveAppInfo> app_info_list;
     drive_app_registry.GetAppsForFile(file_path.Extension(),
                                       mime_type,
                                       &app_info_list);
 
     if (is_first) {
       // For the first file, we store all the info.
-      for (size_t j = 0; j < app_info_list.size(); ++j) {
-        const drive::DriveAppInfo& app_info = *app_info_list[j];
-        drive_app_map[app_info.app_id] = app_info;
-      }
+      for (size_t j = 0; j < app_info_list.size(); ++j)
+        drive_app_map[app_info_list[j].app_id] = app_info_list[j];
     } else {
       // For remaining files, take the intersection with the current
       // result, based on the app id.
       std::set<std::string> app_id_set;
       for (size_t j = 0; j < app_info_list.size(); ++j)
-        app_id_set.insert(app_info_list[j]->app_id);
+        app_id_set.insert(app_info_list[j].app_id);
       for (DriveAppInfoMap::iterator iter = drive_app_map.begin();
            iter != drive_app_map.end();) {
         if (app_id_set.count(iter->first) == 0) {