Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(3329)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: chrome/browser/ui/views/extensions/extension_install_dialog_view.cc

Issue 313203004: Make ExtensionInstallPrompt::Prompt ref-counted (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Latest master for CQ Created 6 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « chrome/browser/ui/cocoa/extensions/windowed_install_dialog_controller_browsertest.mm ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/ui/views/extensions/extension_install_dialog_view.cc
diff --git a/chrome/browser/ui/views/extensions/extension_install_dialog_view.cc b/chrome/browser/ui/views/extensions/extension_install_dialog_view.cc
index f0c9fc3836d3fc0618cbb57ed58e644a846b9a51..a33e9e2121b9dc60f006bbac553a3a3399edf469 100644
--- a/chrome/browser/ui/views/extensions/extension_install_dialog_view.cc
+++ b/chrome/browser/ui/views/extensions/extension_install_dialog_view.cc
@@ -134,9 +134,10 @@ class ExtensionInstallDialogView : public views::DialogDelegateView,
public views::LinkListener,
public views::ButtonListener {
public:
- ExtensionInstallDialogView(content::PageNavigator* navigator,
- ExtensionInstallPrompt::Delegate* delegate,
- const ExtensionInstallPrompt::Prompt& prompt);
+ ExtensionInstallDialogView(
+ content::PageNavigator* navigator,
+ ExtensionInstallPrompt::Delegate* delegate,
+ scoped_refptr<ExtensionInstallPrompt::Prompt> prompt);
virtual ~ExtensionInstallDialogView();
// Called when one of the child elements has expanded/collapsed.
@@ -175,15 +176,15 @@ class ExtensionInstallDialogView : public views::DialogDelegateView,
bool single_detail_row) const;
bool is_inline_install() const {
- return prompt_.type() == ExtensionInstallPrompt::INLINE_INSTALL_PROMPT;
+ return prompt_->type() == ExtensionInstallPrompt::INLINE_INSTALL_PROMPT;
}
bool is_bundle_install() const {
- return prompt_.type() == ExtensionInstallPrompt::BUNDLE_INSTALL_PROMPT;
+ return prompt_->type() == ExtensionInstallPrompt::BUNDLE_INSTALL_PROMPT;
}
bool is_external_install() const {
- return prompt_.type() == ExtensionInstallPrompt::EXTERNAL_INSTALL_PROMPT;
+ return prompt_->type() == ExtensionInstallPrompt::EXTERNAL_INSTALL_PROMPT;
}
// Updates the histogram that holds installation accepted/aborted data.
@@ -195,7 +196,7 @@ class ExtensionInstallDialogView : public views::DialogDelegateView,
content::PageNavigator* navigator_;
ExtensionInstallPrompt::Delegate* delegate_;
- const ExtensionInstallPrompt::Prompt& prompt_;
+ scoped_refptr<ExtensionInstallPrompt::Prompt> prompt_;
// The scroll view containing all the details for the dialog (including all
// collapsible/expandable sections).
@@ -385,7 +386,7 @@ class ExpandableContainerView : public views::View,
void ShowExtensionInstallDialogImpl(
const ExtensionInstallPrompt::ShowParams& show_params,
ExtensionInstallPrompt::Delegate* delegate,
- const ExtensionInstallPrompt::Prompt& prompt) {
+ scoped_refptr<ExtensionInstallPrompt::Prompt> prompt) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
CreateBrowserModalDialogViews(
new ExtensionInstallDialogView(show_params.navigator, delegate, prompt),
@@ -405,7 +406,7 @@ void CustomScrollableView::Layout() {
ExtensionInstallDialogView::ExtensionInstallDialogView(
content::PageNavigator* navigator,
ExtensionInstallPrompt::Delegate* delegate,
- const ExtensionInstallPrompt::Prompt& prompt)
+ scoped_refptr<ExtensionInstallPrompt::Prompt> prompt)
: navigator_(navigator),
delegate_(delegate),
prompt_(prompt),
@@ -496,9 +497,9 @@ ExtensionInstallDialogView::ExtensionInstallDialogView(
// the dialog depending on the experiment group.
int left_column_width =
- (prompt.ShouldShowPermissions() +
- prompt.GetRetainedFileCount()) > 0 ?
- kPermissionsLeftColumnWidth : kNoPermissionsLeftColumnWidth;
+ (prompt->ShouldShowPermissions() + prompt->GetRetainedFileCount()) > 0
+ ? kPermissionsLeftColumnWidth
+ : kNoPermissionsLeftColumnWidth;
if (is_bundle_install())
left_column_width = kBundleLeftColumnWidth;
if (is_external_install())
@@ -516,8 +517,8 @@ ExtensionInstallDialogView::ExtensionInstallDialogView(
scrollable_, left_column_width, column_set_id, false);
ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
- if (prompt.ShouldShowPermissions() &&
- prompt.experiment()->should_show_expandable_permission_list()) {
+ if (prompt->ShouldShowPermissions() &&
+ prompt->experiment()->should_show_expandable_permission_list()) {
// If the experiment should hide the permission list initially, create a
// simple layout that contains only the header, extension name and icon.
scrollable_header_only_ = new CustomScrollableView();
@@ -534,28 +535,28 @@ ExtensionInstallDialogView::ExtensionInstallDialogView(
// Widen the dialog for experiment with checkboxes so that the information
// label fits the area to the left of the buttons.
- if (prompt.experiment()->show_checkboxes())
+ if (prompt->experiment()->show_checkboxes())
dialog_width += 4 * views::kPanelHorizMargin;
- if (prompt.has_webstore_data()) {
+ if (prompt->has_webstore_data()) {
layout->StartRow(0, column_set_id);
views::View* rating = new views::View();
rating->SetLayoutManager(new views::BoxLayout(
views::BoxLayout::kHorizontal, 0, 0, 0));
layout->AddView(rating);
- prompt.AppendRatingStars(AddResourceIcon, rating);
+ prompt->AppendRatingStars(AddResourceIcon, rating);
const gfx::FontList& small_font_list =
rb.GetFontList(ui::ResourceBundle::SmallFont);
views::Label* rating_count =
- new views::Label(prompt.GetRatingCount(), small_font_list);
+ new views::Label(prompt->GetRatingCount(), small_font_list);
// Add some space between the stars and the rating count.
rating_count->SetBorder(views::Border::CreateEmptyBorder(0, 2, 0, 0));
rating->AddChildView(rating_count);
layout->StartRow(0, column_set_id);
views::Label* user_count =
- new views::Label(prompt.GetUserCount(), small_font_list);
+ new views::Label(prompt->GetUserCount(), small_font_list);
user_count->SetAutoColorReadabilityEnabled(false);
user_count->SetEnabledColor(SK_ColorGRAY);
layout->AddView(user_count);
@@ -569,7 +570,7 @@ ExtensionInstallDialogView::ExtensionInstallDialogView(
}
if (is_bundle_install()) {
- BundleInstaller::ItemList items = prompt.bundle()->GetItemsWithState(
+ BundleInstaller::ItemList items = prompt->bundle()->GetItemsWithState(
BundleInstaller::Item::STATE_PENDING);
for (size_t i = 0; i < items.size(); ++i) {
base::string16 extension_name =
@@ -586,10 +587,10 @@ ExtensionInstallDialogView::ExtensionInstallDialogView(
}
}
- if (prompt.ShouldShowPermissions()) {
+ if (prompt->ShouldShowPermissions()) {
layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
- if (prompt.GetPermissionCount() > 0) {
+ if (prompt->GetPermissionCount() > 0) {
if (is_inline_install()) {
layout->StartRow(0, column_set_id);
layout->AddView(new views::Separator(views::Separator::HORIZONTAL),
@@ -603,31 +604,31 @@ ExtensionInstallDialogView::ExtensionInstallDialogView(
// We need to pass the FontList in the constructor, rather than calling
// SetFontList later, because otherwise SizeToFit mis-judges the width
// of the line.
- permissions_header = new views::Label(
- prompt.GetPermissionsHeading(),
- rb.GetFontList(ui::ResourceBundle::MediumFont));
+ permissions_header =
+ new views::Label(prompt->GetPermissionsHeading(),
+ rb.GetFontList(ui::ResourceBundle::MediumFont));
} else {
- permissions_header = new views::Label(prompt.GetPermissionsHeading());
+ permissions_header = new views::Label(prompt->GetPermissionsHeading());
}
permissions_header->SetMultiLine(true);
permissions_header->SetHorizontalAlignment(gfx::ALIGN_LEFT);
permissions_header->SizeToFit(left_column_width);
layout->AddView(permissions_header);
- for (size_t i = 0; i < prompt.GetPermissionCount(); ++i) {
+ for (size_t i = 0; i < prompt->GetPermissionCount(); ++i) {
layout->AddPaddingRow(0, views::kRelatedControlVerticalSpacing);
layout->StartRow(0, column_set_id);
views::Label* permission_label =
- new views::Label(prompt.GetPermission(i));
+ new views::Label(prompt->GetPermission(i));
const SkColor kTextHighlight = SK_ColorRED;
const SkColor kBackgroundHighlight = SkColorSetRGB(0xFB, 0xF7, 0xA3);
- if (prompt.experiment()->ShouldHighlightText(
- prompt.GetPermission(i))) {
+ if (prompt->experiment()->ShouldHighlightText(
+ prompt->GetPermission(i))) {
permission_label->SetAutoColorReadabilityEnabled(false);
permission_label->SetEnabledColor(kTextHighlight);
- } else if (prompt.experiment()->ShouldHighlightBackground(
- prompt.GetPermission(i))) {
+ } else if (prompt->experiment()->ShouldHighlightBackground(
+ prompt->GetPermission(i))) {
permission_label->SetLineHeight(18);
permission_label->set_background(
views::Background::CreateSolidBackground(kBackgroundHighlight));
@@ -637,18 +638,18 @@ ExtensionInstallDialogView::ExtensionInstallDialogView(
permission_label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
permission_label->SizeToFit(left_column_width);
- if (prompt.experiment()->show_checkboxes()) {
+ if (prompt->experiment()->show_checkboxes()) {
layout->AddView(new CheckboxedView(permission_label, this));
++unchecked_boxes_;
} else {
layout->AddView(new BulletedView(permission_label));
}
// If we have more details to provide, show them in collapsed form.
- if (!prompt.GetPermissionsDetails(i).empty()) {
+ if (!prompt->GetPermissionsDetails(i).empty()) {
layout->StartRow(0, column_set_id);
PermissionDetails details;
details.push_back(
- PrepareForDisplay(prompt.GetPermissionsDetails(i), false));
+ PrepareForDisplay(prompt->GetPermissionsDetails(i), false));
ExpandableContainerView* details_container =
new ExpandableContainerView(
this, base::string16(), details, left_column_width,
@@ -656,10 +657,10 @@ ExtensionInstallDialogView::ExtensionInstallDialogView(
layout->AddView(details_container);
}
- if (prompt.experiment()->should_show_inline_explanations()) {
+ if (prompt->experiment()->should_show_inline_explanations()) {
base::string16 explanation =
- prompt.experiment()->GetInlineExplanation(
- prompt.GetPermission(i));
+ prompt->experiment()->GetInlineExplanation(
+ prompt->GetPermission(i));
if (!explanation.empty()) {
PermissionDetails details;
details.push_back(explanation);
@@ -669,7 +670,7 @@ ExtensionInstallDialogView::ExtensionInstallDialogView(
false, false, true);
// Inline explanations are expanded by default if there is
// no "Show details" link.
- if (!prompt.experiment()->show_details_link())
+ if (!prompt->experiment()->show_details_link())
container->ExpandWithoutAnimation();
layout->StartRow(0, column_set_id);
layout->AddView(container);
@@ -689,13 +690,13 @@ ExtensionInstallDialogView::ExtensionInstallDialogView(
}
}
- if (prompt.GetRetainedFileCount()) {
+ if (prompt->GetRetainedFileCount()) {
// Slide in under the permissions, if there are any. If there are
// either, the retained files prompt stretches all the way to the
// right of the dialog. If there are no permissions, the retained
// files prompt just takes up the left column.
int space_for_files = left_column_width;
- if (prompt.GetPermissionCount()) {
+ if (prompt->GetPermissionCount()) {
space_for_files += kIconSize;
views::ColumnSet* column_set = layout->AddColumnSet(++column_set_id);
column_set->AddColumn(views::GridLayout::FILL,
@@ -710,8 +711,7 @@ ExtensionInstallDialogView::ExtensionInstallDialogView(
layout->StartRow(0, column_set_id);
views::Label* retained_files_header = NULL;
- retained_files_header =
- new views::Label(prompt.GetRetainedFilesHeading());
+ retained_files_header = new views::Label(prompt->GetRetainedFilesHeading());
retained_files_header->SetMultiLine(true);
retained_files_header->SetHorizontalAlignment(gfx::ALIGN_LEFT);
retained_files_header->SizeToFit(space_for_files);
@@ -719,8 +719,8 @@ ExtensionInstallDialogView::ExtensionInstallDialogView(
layout->StartRow(0, column_set_id);
PermissionDetails details;
- for (size_t i = 0; i < prompt.GetRetainedFileCount(); ++i)
- details.push_back(prompt.GetRetainedFile(i));
+ for (size_t i = 0; i < prompt->GetRetainedFileCount(); ++i)
+ details.push_back(prompt->GetRetainedFile(i));
ExpandableContainerView* issue_advice_view =
new ExpandableContainerView(
this, base::string16(), details, space_for_files,
@@ -728,13 +728,13 @@ ExtensionInstallDialogView::ExtensionInstallDialogView(
layout->AddView(issue_advice_view);
}
- DCHECK(prompt.type() >= 0);
+ DCHECK(prompt->type() >= 0);
UMA_HISTOGRAM_ENUMERATION("Extensions.InstallPrompt.Type",
- prompt.type(),
+ prompt->type(),
ExtensionInstallPrompt::NUM_PROMPT_TYPES);
- if (prompt.ShouldShowPermissions()) {
- if (prompt.ShouldShowExplanationText()) {
+ if (prompt->ShouldShowPermissions()) {
+ if (prompt->ShouldShowExplanationText()) {
views::ColumnSet* column_set = layout->AddColumnSet(++column_set_id);
column_set->AddColumn(views::GridLayout::LEADING,
views::GridLayout::FILL,
@@ -746,26 +746,26 @@ ExtensionInstallDialogView::ExtensionInstallDialogView(
layout->AddPaddingRow(0, 2 * views::kRelatedControlVerticalSpacing);
layout->StartRow(0, column_set_id);
- views::Label* explanation = new views::Label(
- prompt.experiment()->GetExplanationText());
+ views::Label* explanation =
+ new views::Label(prompt->experiment()->GetExplanationText());
explanation->SetMultiLine(true);
explanation->SetHorizontalAlignment(gfx::ALIGN_LEFT);
explanation->SizeToFit(left_column_width + kIconSize);
layout->AddView(explanation);
}
- if (prompt.experiment()->should_show_expandable_permission_list() ||
- (prompt.experiment()->show_details_link() &&
- prompt.experiment()->should_show_inline_explanations() &&
- !inline_explanations_.empty())) {
+ if (prompt->experiment()->should_show_expandable_permission_list() ||
+ (prompt->experiment()->show_details_link() &&
+ prompt->experiment()->should_show_inline_explanations() &&
+ !inline_explanations_.empty())) {
// Don't show the "Show details" link if there are retained
// files. These have their own "Show details" links and having
// multiple levels of links is confusing.
- if (prompt.GetRetainedFileCount() == 0) {
+ if (prompt->GetRetainedFileCount() == 0) {
int text_id =
- prompt.experiment()->should_show_expandable_permission_list() ?
- IDS_EXTENSION_PROMPT_EXPERIMENT_SHOW_PERMISSIONS :
- IDS_EXTENSION_PROMPT_EXPERIMENT_SHOW_DETAILS;
+ prompt->experiment()->should_show_expandable_permission_list()
+ ? IDS_EXTENSION_PROMPT_EXPERIMENT_SHOW_PERMISSIONS
+ : IDS_EXTENSION_PROMPT_EXPERIMENT_SHOW_DETAILS;
show_details_link_ = new views::Link(
l10n_util::GetStringUTF16(text_id));
show_details_link_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
@@ -776,7 +776,7 @@ ExtensionInstallDialogView::ExtensionInstallDialogView(
}
}
- if (prompt.experiment()->show_checkboxes()) {
+ if (prompt->experiment()->show_checkboxes()) {
checkbox_info_label_ = new views::Label(
l10n_util::GetStringUTF16(
IDS_EXTENSION_PROMPT_EXPERIMENT_CHECKBOX_INFO));
@@ -833,7 +833,7 @@ views::GridLayout* ExtensionInstallDialogView::CreateLayout(
ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
views::Label* heading = new views::Label(
- prompt_.GetHeading(), rb.GetFontList(ui::ResourceBundle::MediumFont));
+ prompt_->GetHeading(), rb.GetFontList(ui::ResourceBundle::MediumFont));
heading->SetMultiLine(true);
heading->SetHorizontalAlignment(gfx::ALIGN_LEFT);
heading->SizeToFit(left_column_width);
@@ -841,7 +841,7 @@ views::GridLayout* ExtensionInstallDialogView::CreateLayout(
if (!is_bundle_install()) {
// Scale down to icon size, but allow smaller icons (don't scale up).
- const gfx::ImageSkia* image = prompt_.icon().ToImageSkia();
+ const gfx::ImageSkia* image = prompt_->icon().ToImageSkia();
gfx::Size size(image->width(), image->height());
if (size.width() > kIconSize || size.height() > kIconSize)
size = gfx::Size(kIconSize, kIconSize);
@@ -857,13 +857,13 @@ views::GridLayout* ExtensionInstallDialogView::CreateLayout(
if (is_inline_install()) {
// Also span the rating, user_count and store_link rows.
icon_row_span = 4;
- } else if (prompt_.ShouldShowPermissions()) {
- size_t permission_count = prompt_.GetPermissionCount();
+ } else if (prompt_->ShouldShowPermissions()) {
+ size_t permission_count = prompt_->GetPermissionCount();
// Also span the permission header and each of the permission rows (all
// have a padding row above it). This also works for the 'no special
// permissions' case.
icon_row_span = 3 + permission_count * 2;
- } else if (prompt_.GetRetainedFileCount()) {
+ } else if (prompt_->GetRetainedFileCount()) {
// Also span the permission header and the retained files container.
icon_row_span = 4;
}
@@ -900,7 +900,7 @@ void ExtensionInstallDialogView::ViewHierarchyChanged(
}
int ExtensionInstallDialogView::GetDialogButtons() const {
- int buttons = prompt_.GetDialogButtons();
+ int buttons = prompt_->GetDialogButtons();
// Simply having just an OK button is *not* supported. See comment on function
// GetDialogButtons in dialog_delegate.h for reasons.
DCHECK_GT(buttons & ui::DIALOG_BUTTON_CANCEL, 0);
@@ -911,11 +911,11 @@ base::string16 ExtensionInstallDialogView::GetDialogButtonLabel(
ui::DialogButton button) const {
switch (button) {
case ui::DIALOG_BUTTON_OK:
- return prompt_.GetAcceptButtonLabel();
+ return prompt_->GetAcceptButtonLabel();
case ui::DIALOG_BUTTON_CANCEL:
- return prompt_.HasAbortButtonLabel() ?
- prompt_.GetAbortButtonLabel() :
- l10n_util::GetStringUTF16(IDS_CANCEL);
+ return prompt_->HasAbortButtonLabel()
+ ? prompt_->GetAbortButtonLabel()
+ : l10n_util::GetStringUTF16(IDS_CANCEL);
default:
NOTREACHED();
return base::string16();
@@ -943,7 +943,7 @@ ui::ModalType ExtensionInstallDialogView::GetModalType() const {
}
base::string16 ExtensionInstallDialogView::GetWindowTitle() const {
- return prompt_.GetDialogTitle();
+ return prompt_->GetDialogTitle();
}
void ExtensionInstallDialogView::LinkClicked(views::Link* source,
@@ -952,7 +952,7 @@ void ExtensionInstallDialogView::LinkClicked(views::Link* source,
UpdateLinkActionHistogram(LINK_CLICKED);
// Show details link is used to either reveal whole permission list or to
// reveal inline explanations.
- if (prompt_.experiment()->should_show_expandable_permission_list()) {
+ if (prompt_->experiment()->should_show_expandable_permission_list()) {
gfx::Rect bounds = GetWidget()->GetWindowBoundsInScreen();
int spacing = bounds.height() -
scrollable_header_only_->GetPreferredSize().height();
@@ -968,7 +968,7 @@ void ExtensionInstallDialogView::LinkClicked(views::Link* source,
show_details_link_->SetVisible(false);
} else {
GURL store_url(extension_urls::GetWebstoreItemDetailURLPrefix() +
- prompt_.extension()->id());
+ prompt_->extension()->id());
OpenURLParams params(
store_url, Referrer(), NEW_FOREGROUND_TAB,
content::PAGE_TRANSITION_LINK,
@@ -1017,7 +1017,7 @@ void ExtensionInstallDialogView::Layout() {
}
// Disable accept button if there are unchecked boxes and
// the experiment is on.
- if (prompt_.experiment()->show_checkboxes())
+ if (prompt_->experiment()->show_checkboxes())
GetDialogClientView()->ok_button()->SetEnabled(unchecked_boxes_ == 0);
DialogDelegateView::Layout();
@@ -1043,13 +1043,13 @@ void ExtensionInstallDialogView::ButtonPressed(views::Button* sender,
void ExtensionInstallDialogView::UpdateInstallResultHistogram(bool accepted)
const {
- if (prompt_.type() == ExtensionInstallPrompt::INSTALL_PROMPT)
+ if (prompt_->type() == ExtensionInstallPrompt::INSTALL_PROMPT)
UMA_HISTOGRAM_BOOLEAN("Extensions.InstallPrompt.Accepted", accepted);
}
void ExtensionInstallDialogView::UpdateLinkActionHistogram(int action_type)
const {
- if (prompt_.experiment()->should_show_expandable_permission_list()) {
+ if (prompt_->experiment()->should_show_expandable_permission_list()) {
// The clickable link in the UI is "Show Permissions".
UMA_HISTOGRAM_ENUMERATION(
"Extensions.InstallPromptExperiment.ShowPermissions",
« no previous file with comments | « chrome/browser/ui/cocoa/extensions/windowed_install_dialog_controller_browsertest.mm ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698