Skip to content

Commit

Permalink
Make sideloaded (externally installed) extensions display webstore info
Browse files Browse the repository at this point in the history
Have sideloaded extensions pull data from the webstore (a la inline install) in
order to give the users a better idea of what extension they are installing.

Images worth 1000 words: http://imgur.com/zlexZeb,VljPXLz,WzT2ZOc#0

XIB changes:
* rename 'app/nibs/ExtensionInstallPromptInline.xib' to 'app/nibs/ExtensionInstallPromptWebstoreData', since the same prompt is now used for inline prompts and for sideloaded extensions when webstore data is available.

BUG=323063

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@247604 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
rdevlin.cronin@chromium.org committed Jan 29, 2014
1 parent 2d7baa5 commit 34b5f7f
Show file tree
Hide file tree
Showing 17 changed files with 195 additions and 83 deletions.
5 changes: 0 additions & 5 deletions chrome/browser/chromeos/app_mode/kiosk_app_data.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,6 @@ namespace {
const char kKeyName[] = "name";
const char kKeyIcon[] = "icon";

// Web store data keys.
const char kManifestKey[] = "manifest";
const char kIconUrlKey[] = "icon_url";
const char kLocalizedNameKey[] = "localized_name";

const char kInvalidWebstoreResponseError[] = "Invalid Chrome Web Store reponse";

// Icon file extension.
Expand Down
8 changes: 4 additions & 4 deletions chrome/browser/chromeos/file_manager/app_installer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,10 @@ AppInstaller::CreateInstallPrompt() const {
new ExtensionInstallPrompt::Prompt(
ExtensionInstallPrompt::INLINE_INSTALL_PROMPT));

prompt->SetInlineInstallWebstoreData(localized_user_count(),
show_user_count(),
average_rating(),
rating_count());
prompt->SetWebstoreData(localized_user_count(),
show_user_count(),
average_rating(),
rating_count());
return prompt.Pass();
}

Expand Down
19 changes: 11 additions & 8 deletions chrome/browser/extensions/extension_install_prompt.cc
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,8 @@ ExtensionInstallPrompt::Prompt::Prompt(PromptType type)
bundle_(NULL),
average_rating_(0.0),
rating_count_(0),
show_user_count_(false) {
show_user_count_(false),
has_webstore_data_(false) {
}

ExtensionInstallPrompt::Prompt::~Prompt() {
Expand Down Expand Up @@ -253,16 +254,17 @@ void ExtensionInstallPrompt::Prompt::SetUserNameFromProfile(Profile* profile) {
}
}

void ExtensionInstallPrompt::Prompt::SetInlineInstallWebstoreData(
void ExtensionInstallPrompt::Prompt::SetWebstoreData(
const std::string& localized_user_count,
bool show_user_count,
double average_rating,
int rating_count) {
CHECK_EQ(INLINE_INSTALL_PROMPT, type_);
CHECK(type_ == INLINE_INSTALL_PROMPT || type_ == EXTERNAL_INSTALL_PROMPT);
localized_user_count_ = localized_user_count;
show_user_count_ = show_user_count;
average_rating_ = average_rating;
rating_count_ = rating_count;
has_webstore_data_ = true;
}

base::string16 ExtensionInstallPrompt::Prompt::GetDialogTitle() const {
Expand Down Expand Up @@ -387,7 +389,7 @@ bool ExtensionInstallPrompt::Prompt::ShouldShowPermissions() const {
void ExtensionInstallPrompt::Prompt::AppendRatingStars(
StarAppender appender, void* data) const {
CHECK(appender);
CHECK_EQ(INLINE_INSTALL_PROMPT, type_);
CHECK(type_ == INLINE_INSTALL_PROMPT || type_ == EXTERNAL_INSTALL_PROMPT);
int rating_integer = floor(average_rating_);
double rating_fractional = average_rating_ - rating_integer;

Expand All @@ -414,13 +416,13 @@ void ExtensionInstallPrompt::Prompt::AppendRatingStars(
}

base::string16 ExtensionInstallPrompt::Prompt::GetRatingCount() const {
CHECK_EQ(INLINE_INSTALL_PROMPT, type_);
CHECK(type_ == INLINE_INSTALL_PROMPT || type_ == EXTERNAL_INSTALL_PROMPT);
return l10n_util::GetStringFUTF16(IDS_EXTENSION_RATING_COUNT,
base::IntToString16(rating_count_));
}

base::string16 ExtensionInstallPrompt::Prompt::GetUserCount() const {
CHECK_EQ(INLINE_INSTALL_PROMPT, type_);
CHECK(type_ == INLINE_INSTALL_PROMPT || type_ == EXTERNAL_INSTALL_PROMPT);

if (show_user_count_) {
return l10n_util::GetStringFUTF16(IDS_EXTENSION_USER_COUNT,
Expand Down Expand Up @@ -647,12 +649,13 @@ void ExtensionInstallPrompt::ConfirmReEnable(Delegate* delegate,
void ExtensionInstallPrompt::ConfirmExternalInstall(
Delegate* delegate,
const Extension* extension,
const ShowDialogCallback& show_dialog_callback) {
const ShowDialogCallback& show_dialog_callback,
const Prompt& prompt) {
DCHECK(ui_loop_ == base::MessageLoop::current());
extension_ = extension;
permissions_ = extension->GetActivePermissions();
delegate_ = delegate;
prompt_.set_type(EXTERNAL_INSTALL_PROMPT);
prompt_ = prompt;
show_dialog_callback_ = show_dialog_callback;

LoadImageIfNeeded();
Expand Down
18 changes: 12 additions & 6 deletions chrome/browser/extensions/extension_install_prompt.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,10 @@ class ExtensionInstallPrompt
void SetIsShowingDetails(DetailsType type,
size_t index,
bool is_showing_details);
void SetInlineInstallWebstoreData(const std::string& localized_user_count,
bool show_user_count,
double average_rating,
int rating_count);
void SetWebstoreData(const std::string& localized_user_count,
bool show_user_count,
double average_rating,
int rating_count);
void SetOAuthIssueAdvice(const IssueAdviceInfo& issue_advice);
void SetUserNameFromProfile(Profile* profile);

Expand Down Expand Up @@ -155,10 +155,11 @@ class ExtensionInstallPrompt
const gfx::Image& icon() const { return icon_; }
void set_icon(const gfx::Image& icon) { icon_ = icon; }

bool has_webstore_data() const { return has_webstore_data_; }

const ExtensionInstallPromptExperiment* experiment() const {
return experiment_;
}

void set_experiment(ExtensionInstallPromptExperiment* experiment) {
experiment_ = experiment;
}
Expand Down Expand Up @@ -202,6 +203,10 @@ class ExtensionInstallPrompt
// false if localized_user_count_ represents the number zero).
bool show_user_count_;

// Whether or not this prompt has been populated with data from the
// webstore.
bool has_webstore_data_;

std::vector<base::FilePath> retained_files_;

scoped_refptr<ExtensionInstallPromptExperiment> experiment_;
Expand Down Expand Up @@ -327,7 +332,8 @@ class ExtensionInstallPrompt
virtual void ConfirmExternalInstall(
Delegate* delegate,
const extensions::Extension* extension,
const ShowDialogCallback& show_dialog_callback);
const ShowDialogCallback& show_dialog_callback,
const Prompt& prompt);

// This is called by the extension permissions API to verify whether an
// extension may be granted additional permissions.
Expand Down
125 changes: 102 additions & 23 deletions chrome/browser/extensions/external_install_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
#include "chrome/browser/extensions/extension_install_ui.h"
#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/extensions/extension_uninstall_dialog.h"
#include "chrome/browser/extensions/webstore_data_fetcher.h"
#include "chrome/browser/extensions/webstore_data_fetcher_delegate.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/browser_finder.h"
Expand Down Expand Up @@ -56,11 +58,10 @@ static const int kMenuCommandId = IDC_EXTERNAL_EXTENSION_ALERT;

class ExternalInstallGlobalError;

// TODO(mpcomplete): Get rid of the refcounting on this class, or document
// why it's necessary. Will do after refactoring to merge back with
// ExtensionDisabledDialogDelegate.
// This class is refcounted to stay alive while we try and pull webstore data.
class ExternalInstallDialogDelegate
: public ExtensionInstallPrompt::Delegate,
public WebstoreDataFetcherDelegate,
public base::RefCountedThreadSafe<ExternalInstallDialogDelegate> {
public:
ExternalInstallDialogDelegate(Browser* browser,
Expand All @@ -80,12 +81,27 @@ class ExternalInstallDialogDelegate
virtual void InstallUIProceed() OVERRIDE;
virtual void InstallUIAbort(bool user_initiated) OVERRIDE;

// WebstoreDataFetcherDelegate:
virtual void OnWebstoreRequestFailure() OVERRIDE;
virtual void OnWebstoreResponseParseSuccess(
scoped_ptr<base::DictionaryValue> webstore_data) OVERRIDE;
virtual void OnWebstoreResponseParseFailure(
const std::string& error) OVERRIDE;

// Show the install dialog to the user.
void ShowInstallUI();

// The UI for showing the install dialog when enabling.
scoped_ptr<ExtensionInstallPrompt> install_ui_;
scoped_ptr<ExtensionInstallPrompt::Prompt> prompt_;

Browser* browser_;
base::WeakPtr<ExtensionService> service_weak_;
const std::string extension_id_;
scoped_ptr<WebstoreDataFetcher> webstore_data_fetcher_;
std::string extension_id_;
bool use_global_error_;

DISALLOW_COPY_AND_ASSIGN(ExternalInstallDialogDelegate);
};

// Only shows a menu item, no bubble. Clicking the menu item shows
Expand All @@ -97,8 +113,6 @@ class ExternalInstallMenuAlert : public GlobalErrorWithStandardBubble,
const Extension* extension);
virtual ~ExternalInstallMenuAlert();

const Extension* extension() const { return extension_; }

// GlobalError implementation.
virtual Severity GetSeverity() OVERRIDE;
virtual bool HasMenuItem() OVERRIDE;
Expand All @@ -123,6 +137,9 @@ class ExternalInstallMenuAlert : public GlobalErrorWithStandardBubble,
ExtensionService* service_;
const Extension* extension_;
content::NotificationRegistrar registrar_;

private:
DISALLOW_COPY_AND_ASSIGN(ExternalInstallMenuAlert);
};

// Shows a menu item and a global error bubble, replacing the install dialog.
Expand Down Expand Up @@ -151,6 +168,9 @@ class ExternalInstallGlobalError : public ExternalInstallMenuAlert {
// manually).
ExternalInstallDialogDelegate* delegate_;
const ExtensionInstallPrompt::Prompt* prompt_;

private:
DISALLOW_COPY_AND_ASSIGN(ExternalInstallGlobalError);
};

static void CreateExternalInstallGlobalError(
Expand Down Expand Up @@ -196,41 +216,100 @@ ExternalInstallDialogDelegate::ExternalInstallDialogDelegate(
bool use_global_error)
: browser_(browser),
service_weak_(service->AsWeakPtr()),
extension_id_(extension->id()) {
extension_id_(extension->id()),
use_global_error_(use_global_error) {
AddRef(); // Balanced in Proceed or Abort.

prompt_.reset(new ExtensionInstallPrompt::Prompt(
ExtensionInstallPrompt::EXTERNAL_INSTALL_PROMPT));

// If we don't have a browser, we can't go to the webstore to fetch data.
// This should only happen in tests.
if (!browser) {
ShowInstallUI();
return;
}

webstore_data_fetcher_.reset(new WebstoreDataFetcher(
this,
browser->profile()->GetRequestContext(),
GURL::EmptyGURL(),
extension->id()));
webstore_data_fetcher_->Start();
}

void ExternalInstallDialogDelegate::OnWebstoreRequestFailure() {
ShowInstallUI();
}

void ExternalInstallDialogDelegate::OnWebstoreResponseParseSuccess(
scoped_ptr<base::DictionaryValue> webstore_data) {
std::string localized_user_count;
double average_rating;
int rating_count;
if (!webstore_data->GetString(kUsersKey, &localized_user_count) ||
!webstore_data->GetDouble(kAverageRatingKey, &average_rating) ||
!webstore_data->GetInteger(kRatingCountKey, &rating_count)) {
// If we don't get a valid webstore response, short circuit, and continue
// to show a prompt without webstore data.
ShowInstallUI();
return;
}

bool show_user_count = true;
webstore_data->GetBoolean(kShowUserCountKey, &show_user_count);

prompt_->SetWebstoreData(localized_user_count,
show_user_count,
average_rating,
rating_count);

ShowInstallUI();
}

void ExternalInstallDialogDelegate::OnWebstoreResponseParseFailure(
const std::string& error) {
ShowInstallUI();
}

void ExternalInstallDialogDelegate::ShowInstallUI() {
const Extension* extension = NULL;
if (!service_weak_.get() ||
!(extension = service_weak_->GetInstalledExtension(extension_id_))) {
return;
}
install_ui_.reset(
ExtensionInstallUI::CreateInstallPromptWithBrowser(browser));
ExtensionInstallUI::CreateInstallPromptWithBrowser(browser_));

const ExtensionInstallPrompt::ShowDialogCallback callback =
use_global_error ?
base::Bind(&CreateExternalInstallGlobalError,
service_weak_, extension_id_) :
ExtensionInstallPrompt::GetDefaultShowDialogCallback();
install_ui_->ConfirmExternalInstall(this, extension, callback);
use_global_error_ ?
base::Bind(&CreateExternalInstallGlobalError,
service_weak_,
extension_id_) :
ExtensionInstallPrompt::GetDefaultShowDialogCallback();

install_ui_->ConfirmExternalInstall(this, extension, callback, *prompt_);
}

ExternalInstallDialogDelegate::~ExternalInstallDialogDelegate() {
}

void ExternalInstallDialogDelegate::InstallUIProceed() {
if (!service_weak_.get())
return;
const Extension* extension =
service_weak_->GetInstalledExtension(extension_id_);
if (!extension)
const Extension* extension = NULL;
if (!service_weak_.get() ||
!(extension = service_weak_->GetInstalledExtension(extension_id_))) {
return;
}
service_weak_->GrantPermissionsAndEnableExtension(extension);
Release();
}

void ExternalInstallDialogDelegate::InstallUIAbort(bool user_initiated) {
if (!service_weak_.get())
return;
const Extension* extension =
service_weak_->GetInstalledExtension(extension_id_);
if (!extension)
const Extension* extension = NULL;
if (!service_weak_.get() ||
!(extension = service_weak_->GetInstalledExtension(extension_id_))) {
return;
}
service_weak_->UninstallExtension(extension_id_, false, NULL);
Release();
}
Expand Down
25 changes: 25 additions & 0 deletions chrome/browser/extensions/webstore_data_fetcher_delegate.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Copyright 2014 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.

#include "chrome/browser/extensions/webstore_data_fetcher_delegate.h"

namespace extensions {

const char WebstoreDataFetcherDelegate::kAverageRatingKey[] = "average_rating";
const char WebstoreDataFetcherDelegate::kIconUrlKey[] = "icon_url";
const char WebstoreDataFetcherDelegate::kIdKey[] = "id";
const char WebstoreDataFetcherDelegate::kInlineInstallNotSupportedKey[] =
"inline_install_not_supported";
const char WebstoreDataFetcherDelegate::kLocalizedDescriptionKey[] =
"localized_description";
const char WebstoreDataFetcherDelegate::kLocalizedNameKey[] = "localized_name";
const char WebstoreDataFetcherDelegate::kManifestKey[] = "manifest";
const char WebstoreDataFetcherDelegate::kRatingCountKey[] = "rating_count";
const char WebstoreDataFetcherDelegate::kRedirectUrlKey[] = "redirect_url";
const char WebstoreDataFetcherDelegate::kShowUserCountKey[] = "show_user_count";
const char WebstoreDataFetcherDelegate::kUsersKey[] = "users";
const char WebstoreDataFetcherDelegate::kVerifiedSiteKey[] = "verified_site";
const char WebstoreDataFetcherDelegate::kVerifiedSitesKey[] = "verified_sites";

} // namespace extensions
15 changes: 15 additions & 0 deletions chrome/browser/extensions/webstore_data_fetcher_delegate.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,21 @@ class WebstoreDataFetcherDelegate {
// Invoked when the web store response parsing is failed.
virtual void OnWebstoreResponseParseFailure(const std::string& error) = 0;

// Keys for indexing the returned webstore data.
static const char kAverageRatingKey[];
static const char kIconUrlKey[];
static const char kIdKey[];
static const char kInlineInstallNotSupportedKey[];
static const char kLocalizedDescriptionKey[];
static const char kLocalizedNameKey[];
static const char kManifestKey[];
static const char kRatingCountKey[];
static const char kRedirectUrlKey[];
static const char kShowUserCountKey[];
static const char kUsersKey[];
static const char kVerifiedSiteKey[];
static const char kVerifiedSitesKey[];

protected:
virtual ~WebstoreDataFetcherDelegate() {}
};
Expand Down
Loading

0 comments on commit 34b5f7f

Please sign in to comment.