Skip to content

Commit

Permalink
Clean up DriveAppRegistry (part 1 of 2).
Browse files Browse the repository at this point in the history 
- Removed unused fields from DriveAppInfo.
  It is easy to recover them later when needed.
- A bit of rewriting for FindPreferredIcon.

BUG=332014
R=hashimoto@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@243465 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
kinaba@chromium.org committed Jan 8, 2014
1 parent 497bfb2 commit ec4a71c
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 102 deletions.
90 changes: 16 additions & 74 deletions chrome/browser/chromeos/drive/drive_app_registry.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,48 +20,19 @@ using content::BrowserThread;

namespace drive {

namespace {

// Webstore URL prefix.
const char kStoreProductUrl[] = "https://chrome.google.com/webstore/";

// Extracts Web store id from its web store URL.
std::string GetWebStoreIdFromUrl(const GURL& url) {
if (!StartsWithASCII(url.spec(), kStoreProductUrl, false)) {
LOG(WARNING) << "Unrecognized product URL " << url.spec();
return std::string();
}

base::FilePath path(url.path());
std::vector<base::FilePath::StringType> components;
path.GetComponents(&components);
DCHECK_LE(2U, components.size()); // Coming from kStoreProductUrl

// Return the last part of the path
return components[components.size() - 1];
}

} // namespace

DriveAppInfo::DriveAppInfo() {
}

DriveAppInfo::DriveAppInfo(
const std::string& app_id,
const google_apis::InstalledApp::IconList& app_icons,
const google_apis::InstalledApp::IconList& document_icons,
const std::string& web_store_id,
const std::string& app_name,
const std::string& object_type,
bool is_primary_selector,
const GURL& create_url)
: app_id(app_id),
app_icons(app_icons),
document_icons(document_icons),
web_store_id(web_store_id),
app_name(app_name),
object_type(object_type),
is_primary_selector(is_primary_selector),
create_url(create_url) {
}

Expand Down Expand Up @@ -108,12 +79,10 @@ void DriveAppRegistry::Update() {

if (is_updating_) // There is already an update in progress.
return;

is_updating_ = true;

scheduler_->GetAppList(
base::Bind(&DriveAppRegistry::UpdateAfterGetAppList,
weak_ptr_factory_.GetWeakPtr()));
scheduler_->GetAppList(base::Bind(&DriveAppRegistry::UpdateAfterGetAppList,
weak_ptr_factory_.GetWeakPtr()));
}

void DriveAppRegistry::UpdateAfterGetAppList(
Expand All @@ -134,20 +103,13 @@ void DriveAppRegistry::UpdateAfterGetAppList(
UpdateFromAppList(*app_list);
}

void DriveAppRegistry::UpdateFromAppList(
const google_apis::AppList& app_list) {
void DriveAppRegistry::UpdateFromAppList(const google_apis::AppList& app_list) {
STLDeleteValues(&app_extension_map_);
STLDeleteValues(&app_mimetypes_map_);

for (size_t i = 0; i < app_list.items().size(); ++i) {
const google_apis::AppResource& app = *app_list.items()[i];

if (app.product_url().is_empty())
continue;
std::string web_store_id = GetWebStoreIdFromUrl(app.product_url());
if (web_store_id.empty())
continue;

google_apis::InstalledApp::IconList app_icons;
google_apis::InstalledApp::IconList document_icons;
for (size_t j = 0; j < app.icons().size(); ++j) {
Expand All @@ -162,43 +124,31 @@ void DriveAppRegistry::UpdateFromAppList(
icon.icon_url()));
}

AddAppSelectorList(web_store_id,
app.name(),
AddAppSelectorList(app.name(),
app_icons,
document_icons,
app.object_type(),
app.application_id(),
true, // primary
app.create_url(),
app.primary_mimetypes(),
&app_mimetypes_map_);
AddAppSelectorList(web_store_id,
app.name(),
AddAppSelectorList(app.name(),
app_icons,
document_icons,
app.object_type(),
app.application_id(),
false, // primary
app.create_url(),
app.secondary_mimetypes(),
&app_mimetypes_map_);
AddAppSelectorList(web_store_id,
app.name(),
AddAppSelectorList(app.name(),
app_icons,
document_icons,
app.object_type(),
app.application_id(),
true, // primary
app.create_url(),
app.primary_file_extensions(),
&app_extension_map_);
AddAppSelectorList(web_store_id,
app.name(),
AddAppSelectorList(app.name(),
app_icons,
document_icons,
app.object_type(),
app.application_id(),
false, // primary
app.create_url(),
app.secondary_file_extensions(),
&app_extension_map_);
Expand All @@ -207,13 +157,10 @@ void DriveAppRegistry::UpdateFromAppList(

// static.
void DriveAppRegistry::AddAppSelectorList(
const std::string& web_store_id,
const std::string& app_name,
const google_apis::InstalledApp::IconList& app_icons,
const google_apis::InstalledApp::IconList& document_icons,
const std::string& object_type,
const std::string& app_id,
bool is_primary_selector,
const GURL& create_url,
const ScopedVector<std::string>& selectors,
DriveAppFileSelectorMap* map) {
Expand All @@ -224,10 +171,7 @@ void DriveAppRegistry::AddAppSelectorList(
*value, new DriveAppInfo(app_id,
app_icons,
document_icons,
web_store_id,
app_name,
object_type,
is_primary_selector,
create_url)));
}
}
Expand All @@ -244,21 +188,19 @@ void DriveAppRegistry::FindAppsForSelector(

namespace util {

GURL FindPreferredIcon(
const google_apis::InstalledApp::IconList& icons,
int preferred_size) {
GURL FindPreferredIcon(const google_apis::InstalledApp::IconList& icons,
int preferred_size) {
if (icons.empty())
return GURL();

google_apis::InstalledApp::IconList sorted_icons = icons;
std::sort(sorted_icons.begin(), sorted_icons.end());
GURL result = sorted_icons.rbegin()->second;
for (google_apis::InstalledApp::IconList::const_reverse_iterator
iter = sorted_icons.rbegin();
iter != sorted_icons.rend() && iter->first >= preferred_size; ++iter) {
result = iter->second;
}
return result;
std::sort(sorted_icons.rbegin(), sorted_icons.rend());

// Go forward while the size is larger or equal to preferred_size.
size_t i = 1;
while (i < sorted_icons.size() && sorted_icons[i].first >= preferred_size)
++i;
return sorted_icons[i - 1].second;
}

} // namespace util
Expand Down
12 changes: 0 additions & 12 deletions chrome/browser/chromeos/drive/drive_app_registry.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,7 @@ struct DriveAppInfo {
DriveAppInfo(const std::string& app_id,
const google_apis::InstalledApp::IconList& app_icons,
const google_apis::InstalledApp::IconList& document_icons,
const std::string& web_store_id,
const std::string& app_name,
const std::string& object_type,
bool is_primary_selector,
const GURL& create_url);
~DriveAppInfo();

Expand All @@ -48,14 +45,8 @@ struct DriveAppInfo {
// Drive document icon URLs for this app, paired with their size (length of
// a side in pixels).
google_apis::InstalledApp::IconList document_icons;
// Web store id/extension id;
std::string web_store_id;
// App name.
std::string app_name;
// Object (file) type description handled by this app.
std::string object_type;
// Is app the primary selector for file (default open action).
bool is_primary_selector;
// URL for opening a new file in the app.
GURL create_url;
};
Expand Down Expand Up @@ -92,13 +83,10 @@ class DriveAppRegistry {
// Helper function for loading Drive application file |selectors| into
// corresponding |map|.
static void AddAppSelectorList(
const std::string& web_store_id,
const std::string& app_name,
const google_apis::InstalledApp::IconList& app_icons,
const google_apis::InstalledApp::IconList& document_icons,
const std::string& object_type,
const std::string& app_id,
bool is_primary_selector,
const GURL& create_url,
const ScopedVector<std::string>& selectors,
DriveAppFileSelectorMap* map);
Expand Down
22 changes: 6 additions & 16 deletions chrome/browser/chromeos/drive/drive_app_registry_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,26 +35,19 @@ class DriveAppRegistryTest : public testing::Test {
}

bool VerifyApp(const ScopedVector<DriveAppInfo>& list,
const std::string& web_store_id,
const std::string& app_id,
const std::string& app_name,
const std::string& object_type,
bool is_primary) {
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 (web_store_id == app->web_store_id) {
EXPECT_EQ(app_id, app->app_id);
if (app_id == app->app_id) {
EXPECT_EQ(app_name, app->app_name);
EXPECT_EQ(object_type, app->object_type);
EXPECT_EQ(is_primary, app->is_primary_selector);
found = true;
break;
}
}
EXPECT_TRUE(found) << "Unable to find app with web_store_id "
<< web_store_id;
EXPECT_TRUE(found) << "Unable to find app with app_id " << app_id;
return found;
}

Expand All @@ -74,24 +67,21 @@ TEST_F(DriveAppRegistryTest, LoadAndFindDriveApps) {
base::FilePath ext_file(FILE_PATH_LITERAL("drive/file.exe"));
web_apps_registry_->GetAppsForFile(ext_file.Extension(), "", &ext_results);
ASSERT_EQ(1U, ext_results.size());
VerifyApp(ext_results, "abcdefghabcdefghabcdefghabcdefgh", "123456788192",
"Drive app 1", "", true);
VerifyApp(ext_results, "123456788192", "Drive app 1");

// Find by primary MIME type.
ScopedVector<DriveAppInfo> primary_app;
web_apps_registry_->GetAppsForFile(base::FilePath::StringType(),
"application/vnd.google-apps.drive-sdk.123456788192", &primary_app);
ASSERT_EQ(1U, primary_app.size());
VerifyApp(primary_app, "abcdefghabcdefghabcdefghabcdefgh", "123456788192",
"Drive app 1", "", true);
VerifyApp(primary_app, "123456788192", "Drive app 1");

// Find by secondary MIME type.
ScopedVector<DriveAppInfo> secondary_app;
web_apps_registry_->GetAppsForFile(
base::FilePath::StringType(), "text/html", &secondary_app);
ASSERT_EQ(1U, secondary_app.size());
VerifyApp(secondary_app, "abcdefghabcdefghabcdefghabcdefgh", "123456788192",
"Drive app 1", "", false);
VerifyApp(secondary_app, "123456788192", "Drive app 1");
}

TEST_F(DriveAppRegistryTest, UpdateFromAppList) {
Expand Down

0 comments on commit ec4a71c

Please sign in to comment.