diff options
9 files changed, 80 insertions, 898 deletions
diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd index ad0dfd4..90b5d28 100644 --- a/chrome/app/generated_resources.grd +++ b/chrome/app/generated_resources.grd @@ -5254,56 +5254,6 @@ Keep your key file in a safe place. You will need it to create new versions of y Hide Details </message> - <!-- Strings for the extensions permission dialog experiment --> - <!-- TODO(meacer): Remove these once the experiments are completed. --> - <message name="IDS_EXTENSION_PROMPT_EXPERIMENT_EXPLANATION1"> - Do you trust this extension to use these privileges safely? - </message> - <message name="IDS_EXTENSION_PROMPT_EXPERIMENT_EXPLANATION2"> - Once installed, this extension could potentially use these privileges to do malicious things to your web browsing experience. Are you sure you want to install this extension? - </message> - <message name="IDS_EXTENSION_PROMPT_EXPERIMENT_EXPLANATION3"> - Are you sure you want to install this extension given that it requires these privileges? - </message> - <message name="IDS_EXTENSION_PROMPT_EXPERIMENT_EXPLANATION4"> - Make sure that these privileges make sense for what you think the extension needs to do. If they do not, click Cancel. - </message> - <message name="IDS_EXTENSION_PROMPT_EXPERIMENT_EXPLANATION5"> - Do you trust this extension to perform these actions? - </message> - - <message name="IDS_EXTENSION_PROMPT_EXPERIMENT_INSTALL_BUTTON_TRUST" desc="Text for the install button on the extension install prompt (for experiment group 1)"> - Yes, I trust this extension! - </message> - <message name="IDS_EXTENSION_PROMPT_EXPERIMENT_INSTALL_BUTTON_YES" desc="Text for the install button on the extension install prompt (for experiment group 2)"> - Yes, install - </message> - <message name="IDS_EXTENSION_PROMPT_EXPERIMENT_INSTALL_BUTTON_SURE" desc="Text for the install button on the extension install prompt (for experiment group 3)"> - Yes, I'm sure - </message> - <message name="IDS_EXTENSION_PROMPT_EXPERIMENT_INSTALL_BUTTON_TRUST2" desc="Text for the install button on the extension install prompt (for experiment group 4)"> - Yes, I trust it - </message> - <message name="IDS_EXTENSION_PROMPT_EXPERIMENT_INSTALL_BUTTON_NOPE" desc="Text for the cancel button on the extension install prompt"> - Nope, cancel - </message> - <message name="IDS_EXTENSION_PROMPT_EXPERIMENT_SHOW_DETAILS" desc="The label of the button which displays permission info when clicked"> - Show details - </message> - <message name="IDS_EXTENSION_PROMPT_EXPERIMENT_SHOW_PERMISSIONS" desc="The label of the button which displays permission list when clicked"> - Show permissions - </message> - <message name="IDS_EXTENSION_PROMPT_EXPERIMENT_CHECKBOX_INFO" desc="Text for the label that explains that user should check all checkboxes to process for installation."> - Please check all boxes to proceed. - </message> - - <message name="IDS_EXTENSION_PROMPT_WARNING_FULL_ACCESS_EXPLANATION" desc="Extra explanation text for full access permission"> - This extension can read and change all data on your computer and all websites including Google, Facebook, Yahoo, etc. - </message> - <message name="IDS_EXTENSION_PROMPT_WARNING_ALL_HOSTS_EXPLANATION" desc="Extra explanation text for all sites permission"> - This extension can read and change your information on all websites including Google, Facebook, Yahoo, etc. - </message> - <!-- Global error messages for extensions --> <message name="IDS_EXTENSION_WARNINGS_WRENCH_MENU_ITEM" desc="The wrench menu item indicating that extensions caused problems."> Extension error diff --git a/chrome/browser/extensions/extension_install_prompt.cc b/chrome/browser/extensions/extension_install_prompt.cc index 45047b8..e8dea3cb 100644 --- a/chrome/browser/extensions/extension_install_prompt.cc +++ b/chrome/browser/extensions/extension_install_prompt.cc @@ -381,11 +381,6 @@ int ExtensionInstallPrompt::Prompt::GetDialogButtons() const { return kButtons[type_]; } -bool ExtensionInstallPrompt::Prompt::ShouldShowExplanationText() const { - return type_ == INSTALL_PROMPT && extension_->is_extension() && - experiment_.get() && experiment_->text_only(); -} - bool ExtensionInstallPrompt::Prompt::HasAcceptButtonLabel() const { if (type_ == POST_INSTALL_PERMISSIONS_PROMPT) return ShouldDisplayRevokeButton(); @@ -419,21 +414,15 @@ base::string16 ExtensionInstallPrompt::Prompt::GetAcceptButtonLabel() const { } return l10n_util::GetStringUTF16(id); } - if (ShouldShowExplanationText()) - return experiment_->GetOkButtonText(); return l10n_util::GetStringUTF16(kAcceptButtonIds[type_]); } bool ExtensionInstallPrompt::Prompt::HasAbortButtonLabel() const { - if (ShouldShowExplanationText()) - return true; return kAbortButtonIds[type_] > 0; } base::string16 ExtensionInstallPrompt::Prompt::GetAbortButtonLabel() const { CHECK(HasAbortButtonLabel()); - if (ShouldShowExplanationText()) - return experiment_->GetCancelButtonText(); return l10n_util::GetStringUTF16(kAbortButtonIds[type_]); } @@ -909,11 +898,6 @@ void ExtensionInstallPrompt::LoadImageIfNeeded() { } void ExtensionInstallPrompt::ShowConfirmation() { - if (prompt_->type() == INSTALL_PROMPT) - prompt_->set_experiment(ExtensionInstallPromptExperiment::Find()); - else - prompt_->set_experiment(ExtensionInstallPromptExperiment::ControlGroup()); - scoped_refptr<const PermissionSet> permissions_to_display; if (custom_permissions_.get()) { permissions_to_display = custom_permissions_; diff --git a/chrome/browser/extensions/extension_install_prompt.h b/chrome/browser/extensions/extension_install_prompt.h index 8bb83fa..56b774c 100644 --- a/chrome/browser/extensions/extension_install_prompt.h +++ b/chrome/browser/extensions/extension_install_prompt.h @@ -15,7 +15,6 @@ #include "base/memory/scoped_ptr.h" #include "base/memory/weak_ptr.h" #include "base/strings/string16.h" -#include "chrome/browser/extensions/extension_install_prompt_experiment.h" #include "extensions/common/url_pattern.h" #include "third_party/skia/include/core/SkBitmap.h" #include "ui/gfx/image/image.h" @@ -141,7 +140,6 @@ class ExtensionInstallPrompt base::string16 GetRetainedDevicesHeading() const; bool ShouldShowPermissions() const; - bool ShouldShowExplanationText() const; // Getters for webstore metadata. Only populated when the type is // INLINE_INSTALL_PROMPT. @@ -200,13 +198,6 @@ class ExtensionInstallPrompt bool has_webstore_data() const { return has_webstore_data_; } - const ExtensionInstallPromptExperiment* experiment() const { - return experiment_.get(); - } - void set_experiment(ExtensionInstallPromptExperiment* experiment) { - experiment_ = experiment; - } - private: friend class base::RefCountedThreadSafe<Prompt>; @@ -271,8 +262,6 @@ class ExtensionInstallPrompt std::vector<base::FilePath> retained_files_; std::vector<base::string16> retained_device_messages_; - scoped_refptr<ExtensionInstallPromptExperiment> experiment_; - DISALLOW_COPY_AND_ASSIGN(Prompt); }; diff --git a/chrome/browser/extensions/extension_install_prompt_experiment.cc b/chrome/browser/extensions/extension_install_prompt_experiment.cc deleted file mode 100644 index 499435a..0000000 --- a/chrome/browser/extensions/extension_install_prompt_experiment.cc +++ /dev/null @@ -1,250 +0,0 @@ -// Copyright 2013 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 "base/basictypes.h" -#include "base/logging.h" -#include "base/metrics/field_trial.h" -#include "base/strings/string_number_conversions.h" -#include "base/strings/string_split.h" -#include "base/strings/stringprintf.h" -#include "chrome/browser/extensions/extension_install_prompt_experiment.h" -#include "chrome/grit/generated_resources.h" -#include "ui/base/l10n/l10n_util.h" - -namespace { - -const char kExperimentName[] = "ExtensionPermissionDialog"; -const char kGroupPrefix[] = "Group"; - -// Flags for groups. Not all combinations make sense. -// Refer to the UI screens at http://goo.gl/f2KzPj for those that do. -enum GroupFlag { - // No changes (Control group). - NONE = 0, - // Indicates that the experiment is text only. A text only experiment - // only adds an explanation text at the bottom of the permission dialog and - // modifies the text on accept/cancel buttons. - TEXT_ONLY = 1 << 0, - // Indicates that the experiment shows inline explanations for permissions. - INLINE_EXPLANATIONS = 1 << 1, - // Indicates that the experiment highlights permission text color. - SHOULD_HIGHLIGHT_TEXT = 1 << 2, - // Indicates that the experiment highlights permission text background. - SHOULD_HIGHLIGHT_BACKGROUND = 1 << 3, - // Indicates that the experiment highlights all permissions. - SHOULD_HIGHLIGHT_ALL_PERMISSIONS = 1 << 4, - // Indicates that the experiment puts a "show details" link in the UI. - SHOULD_SHOW_DETAILS_LINK = 1 << 5, - // Indicates that the experiment hides the permissions by default and the list - // can be expanded. - EXPANDABLE_PERMISSION_LIST = 1 << 6, - // Indicates that the experiment shows checkboxes for each permission. - SHOULD_SHOW_CHECKBOXES = 1 << 7 -}; - -// Flags for the actual experiment groups. These flags define what kind of -// UI changes each experiment group does. An experiment group may change -// multiple aspects of the extension install dialog (e.g. one of the groups -// show a details link and inline explanations for permissions). The control -// group doesn't change the UI. Text only groups add a text warning to the UI, -// with the text changing depending on the group number. Groups with inline -// explanations show detailed explanations for a subset of permissions. Some -// groups highlight the foreground or the background of permission texts. -// The flags reflect the UI screens at http://goo.gl/f2KzPj. -const unsigned int kGroupFlags[] = { - // Control group doesn't change the UI. - NONE, - // Adds "Do you trust this extension to use these privileges safely" text. - TEXT_ONLY, - // Adds "Extension can be malicious" text. - TEXT_ONLY, - // Adds "Are you sure you want to install" text. - TEXT_ONLY, - // Adds "Make sure these privileges make sense for this extension" text. - TEXT_ONLY, - // Adds "Do you trust this extension to perform these actions" text. - TEXT_ONLY, - // Adds inline explanations displayed by default. - INLINE_EXPLANATIONS, - // Adds expandable inline explanations with a "Show Details" link. - SHOULD_SHOW_DETAILS_LINK | INLINE_EXPLANATIONS, - // Adds expandable permission list with a "Show Permissions" link. - SHOULD_SHOW_DETAILS_LINK | EXPANDABLE_PERMISSION_LIST, - // Highlights text for risky permissions. - SHOULD_HIGHLIGHT_TEXT, - // Highlights background for risky permissions. - SHOULD_HIGHLIGHT_BACKGROUND, - // Highlights background for all permissions - SHOULD_HIGHLIGHT_BACKGROUND | SHOULD_HIGHLIGHT_ALL_PERMISSIONS, - // Displays checkboxes for all permissions. - SHOULD_SHOW_CHECKBOXES -}; - -const size_t kGroupCount = arraysize(kGroupFlags); - -// Parameters for text only experiments. -const struct TextParams { - int text_id; - int ok_text_id; - int cancel_text_id; -} kTextParams[] = { - { - IDS_EXTENSION_PROMPT_EXPERIMENT_EXPLANATION1, - IDS_EXTENSION_PROMPT_EXPERIMENT_INSTALL_BUTTON_TRUST, - IDS_CANCEL, - }, - { - IDS_EXTENSION_PROMPT_EXPERIMENT_EXPLANATION2, - IDS_EXTENSION_PROMPT_EXPERIMENT_INSTALL_BUTTON_YES, - IDS_EXTENSION_PROMPT_EXPERIMENT_INSTALL_BUTTON_NOPE - }, - { - IDS_EXTENSION_PROMPT_EXPERIMENT_EXPLANATION3, - IDS_EXTENSION_PROMPT_EXPERIMENT_INSTALL_BUTTON_SURE, - IDS_EXTENSION_PROMPT_EXPERIMENT_INSTALL_BUTTON_NOPE - }, - { - IDS_EXTENSION_PROMPT_EXPERIMENT_EXPLANATION4, - IDS_EXTENSION_PROMPT_INSTALL_BUTTON, - IDS_CANCEL - }, - { - IDS_EXTENSION_PROMPT_EXPERIMENT_EXPLANATION5, - IDS_EXTENSION_PROMPT_EXPERIMENT_INSTALL_BUTTON_TRUST2, - IDS_EXTENSION_PROMPT_EXPERIMENT_INSTALL_BUTTON_NOPE - } -}; - -// Permission warnings in this list have inline explanation texts. -const struct PermissionExplanations { - int warning_msg_id; - int extra_explanation_id; -} kPermissionExplanations[] = { - { - IDS_EXTENSION_PROMPT_WARNING_FULL_ACCESS, - IDS_EXTENSION_PROMPT_WARNING_FULL_ACCESS_EXPLANATION - }, - { - IDS_EXTENSION_PROMPT_WARNING_ALL_HOSTS, - IDS_EXTENSION_PROMPT_WARNING_ALL_HOSTS_EXPLANATION - } -}; - -// Permission warnings in this list are going to be highlighted. -// Note that the matching is done by string comparison, so this list must not -// contain any dynamic strings (e.g. permission for 3 hosts with the host list). -const int kHighlightedWarnings[] = { - IDS_EXTENSION_PROMPT_WARNING_FULL_ACCESS, - IDS_EXTENSION_PROMPT_WARNING_ALL_HOSTS, - IDS_EXTENSION_PROMPT_WARNING_BOOKMARKS, - IDS_EXTENSION_PROMPT_WARNING_CONTENT_SETTINGS, - IDS_EXTENSION_PROMPT_WARNING_HISTORY_WRITE, - IDS_EXTENSION_PROMPT_WARNING_INPUT, - IDS_EXTENSION_PROMPT_WARNING_MANAGEMENT, - IDS_EXTENSION_PROMPT_WARNING_HISTORY_READ, - IDS_EXTENSION_PROMPT_WARNING_DEBUGGER -}; - -bool IsImportantWarning(const base::string16& message) { - for (size_t i = 0; i < arraysize(kHighlightedWarnings); ++i) { - if (message == l10n_util::GetStringUTF16(kHighlightedWarnings[i])) - return true; - } - return false; -} - -} // namespace - -ExtensionInstallPromptExperiment::ExtensionInstallPromptExperiment( - unsigned int group_id, unsigned int flags) - : group_id_(group_id), - flags_(flags) { -} - -ExtensionInstallPromptExperiment::~ExtensionInstallPromptExperiment() { -} - -// static -ExtensionInstallPromptExperiment* - ExtensionInstallPromptExperiment::ControlGroup() { - return new ExtensionInstallPromptExperiment(0, kGroupFlags[0]); -} - -// static -ExtensionInstallPromptExperiment* - ExtensionInstallPromptExperiment::Find() { - base::FieldTrial* trial = base::FieldTrialList::Find(kExperimentName); - // Default is control group. - unsigned int group_id = 0; - if (trial) { - std::vector<std::string> tokens; - base::SplitString(trial->group_name().c_str(), '_', &tokens); - if (tokens.size() == 2 && tokens[0] == kGroupPrefix) { - base::StringToUint(tokens[1], &group_id); - if (group_id >= kGroupCount) - group_id = 0; - } - } - return new ExtensionInstallPromptExperiment(group_id, kGroupFlags[group_id]); -} - -base::string16 ExtensionInstallPromptExperiment::GetExplanationText() const { - DCHECK(group_id_ > 0 && group_id_ - 1 < arraysize(kTextParams)); - return l10n_util::GetStringUTF16(kTextParams[group_id_ - 1].text_id); -} - -base::string16 ExtensionInstallPromptExperiment::GetOkButtonText() const { - DCHECK(group_id_ > 0 && group_id_ - 1 < arraysize(kTextParams)); - return l10n_util::GetStringUTF16(kTextParams[group_id_ - 1].ok_text_id); -} - -base::string16 ExtensionInstallPromptExperiment::GetCancelButtonText() const { - DCHECK(group_id_ > 0 && group_id_ - 1 < arraysize(kTextParams)); - return l10n_util::GetStringUTF16(kTextParams[group_id_ - 1].cancel_text_id); -} - -bool ExtensionInstallPromptExperiment::text_only() const { - return (flags_ & TEXT_ONLY) != 0; -} - -bool ExtensionInstallPromptExperiment::ShouldHighlightText( - const base::string16& message) const { - return (flags_ & SHOULD_HIGHLIGHT_TEXT) != 0 && IsImportantWarning(message); -} - -bool ExtensionInstallPromptExperiment::ShouldHighlightBackground( - const base::string16& message) const { - return (flags_ & SHOULD_HIGHLIGHT_BACKGROUND) != 0 && - ((flags_ & SHOULD_HIGHLIGHT_ALL_PERMISSIONS) != 0 || - IsImportantWarning(message)); -} - -bool ExtensionInstallPromptExperiment::show_details_link() const { - return (flags_ & SHOULD_SHOW_DETAILS_LINK) != 0; -} - -bool ExtensionInstallPromptExperiment::show_checkboxes() const { - return (flags_ & SHOULD_SHOW_CHECKBOXES) != 0; -} - -bool ExtensionInstallPromptExperiment::should_show_expandable_permission_list() - const { - return (flags_ & EXPANDABLE_PERMISSION_LIST) != 0; -} - -bool ExtensionInstallPromptExperiment::should_show_inline_explanations() const { - return (flags_ & INLINE_EXPLANATIONS) != 0; -} - -base::string16 ExtensionInstallPromptExperiment::GetInlineExplanation( - const base::string16& message) const { - for (size_t i = 0; i < arraysize(kPermissionExplanations); ++i) { - if (message == l10n_util::GetStringUTF16( - kPermissionExplanations[i].warning_msg_id)) { - return l10n_util::GetStringUTF16( - kPermissionExplanations[i].extra_explanation_id); - } - } - return base::string16(); -} diff --git a/chrome/browser/extensions/extension_install_prompt_experiment.h b/chrome/browser/extensions/extension_install_prompt_experiment.h deleted file mode 100644 index 630c1d1..0000000 --- a/chrome/browser/extensions/extension_install_prompt_experiment.h +++ /dev/null @@ -1,81 +0,0 @@ -// Copyright 2013 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. - -#ifndef CHROME_BROWSER_EXTENSIONS_EXTENSION_INSTALL_PROMPT_EXPERIMENT_H_ -#define CHROME_BROWSER_EXTENSIONS_EXTENSION_INSTALL_PROMPT_EXPERIMENT_H_ - -#include "base/memory/ref_counted.h" - -// Represents a permission dialog experiment. -// TODO(meacer): Remove this class once the ExtensionPermissionDialog -// experiment is completed (http://crbug.com/308748). -class ExtensionInstallPromptExperiment - : public base::RefCounted<ExtensionInstallPromptExperiment> { - public: - ExtensionInstallPromptExperiment(unsigned int group_id, unsigned int flags); - - // Returns an experiment instance configured by the server. The ownership of - // the returned pointer is passed to the caller. - static ExtensionInstallPromptExperiment* Find(); - - // Returns an experiment instance for the control group. The ownership of the - // returned pointer is passed to the caller. - static ExtensionInstallPromptExperiment* ControlGroup(); - - // Returns true if this is a text only experiment. A text only experiment - // only adds an explanation text at the bottom of the permission dialog - // and changes the text on the add/cancel buttons. - bool text_only() const; - - // The explanation text to be added for text only experiments. - base::string16 GetExplanationText() const; - - // The text for the accept button for text only experiments. - base::string16 GetOkButtonText() const; - - // The text for the cancel button for text only experiments. - base::string16 GetCancelButtonText() const; - - // Returns true if the text color should be highlighted for the given - // permission message. - bool ShouldHighlightText(const base::string16& message) const; - - // Returns true if the text background should be highlighted for the given - // permission message. - bool ShouldHighlightBackground(const base::string16& message) const; - - // Returns true if there should be a "Show details" link at the bottom of the - // permission dialog. - bool show_details_link() const; - - // Returns true if there should be checkboxes next to permissions for the - // user to click. - bool show_checkboxes() const; - - // Returns true if the permission list should be hidden by default and can - // be expanded when necessary. - bool should_show_expandable_permission_list() const; - - // Returns true if the experiment should show inline explanations for - // permissions. - bool should_show_inline_explanations() const; - - // Returns the inline explanation text for the given permission warning. - // Returns empty string if there is no corresponding inline explanation. - base::string16 GetInlineExplanation(const base::string16& message) const; - - private: - friend class base::RefCounted<ExtensionInstallPromptExperiment>; - ~ExtensionInstallPromptExperiment(); - - // Group id of the experiment. The zeroth group is the control group. - const unsigned int group_id_; - // Bitmask for the changes done to the UI by the experiment. An experiment can - // change multiple parts of the UI. - const unsigned int flags_; - - DISALLOW_COPY_AND_ASSIGN(ExtensionInstallPromptExperiment); -}; - -#endif // CHROME_BROWSER_EXTENSIONS_EXTENSION_INSTALL_PROMPT_EXPERIMENT_H_ 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 75d28c0..325fbdd 100644 --- a/chrome/browser/ui/views/extensions/extension_install_dialog_view.cc +++ b/chrome/browser/ui/views/extensions/extension_install_dialog_view.cc @@ -16,7 +16,6 @@ #include "chrome/browser/browser_process.h" #include "chrome/browser/extensions/api/experience_sampling_private/experience_sampling.h" #include "chrome/browser/extensions/bundle_installer.h" -#include "chrome/browser/extensions/extension_install_prompt_experiment.h" #include "chrome/browser/extensions/extension_install_prompt_show_params.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile_manager.h" @@ -87,20 +86,6 @@ const int kBundleLeftColumnWidth = 300; // this case, so make it wider than normal. const int kExternalInstallLeftColumnWidth = 350; -// Lighter color for labels. -const SkColor kLighterLabelColor = SkColorSetRGB(0x99, 0x99, 0x99); - -// Represents an action on a clickable link created by the install prompt -// experiment. This is used to group the actions in UMA histograms named -// Extensions.InstallPromptExperiment.ShowDetails and -// Extensions.InstallPromptExperiment.ShowPermissions. -enum ExperimentLinkAction { - LINK_SHOWN = 0, - LINK_NOT_SHOWN, - LINK_CLICKED, - NUM_LINK_ACTIONS -}; - void AddResourceIcon(const gfx::ImageSkia* skia_image, void* data) { views::View* parent = static_cast<views::View*>(data); views::ImageView* image_view = new views::ImageView(); @@ -140,34 +125,6 @@ BulletedView::BulletedView(views::View* view) { layout->AddView(view); } -CheckboxedView::CheckboxedView(views::View* view, - views::ButtonListener* listener) { - views::GridLayout* layout = new views::GridLayout(this); - SetLayoutManager(layout); - views::ColumnSet* column_set = layout->AddColumnSet(0); - column_set->AddColumn(views::GridLayout::LEADING, - views::GridLayout::LEADING, - 0, - views::GridLayout::USE_PREF, - 0, // No fixed width. - 0); - column_set->AddColumn(views::GridLayout::LEADING, - views::GridLayout::LEADING, - 0, - views::GridLayout::USE_PREF, - 0, // No fixed width. - 0); - layout->StartRow(0, 0); - views::Checkbox* checkbox = new views::Checkbox(base::string16()); - checkbox->set_listener(listener); - // Alignment needs to be explicitly set again here, otherwise the views are - // not vertically centered. - layout->AddView(checkbox, 1, 1, - views::GridLayout::LEADING, views::GridLayout::CENTER); - layout->AddView(view, 1, 1, - views::GridLayout::LEADING, views::GridLayout::CENTER); -} - void ShowExtensionInstallDialogImpl( ExtensionInstallPromptShowParams* show_params, ExtensionInstallPrompt::Delegate* delegate, @@ -201,10 +158,6 @@ ExtensionInstallDialogView::ExtensionInstallDialogView( prompt_(prompt), scroll_view_(NULL), scrollable_(NULL), - scrollable_header_only_(NULL), - show_details_link_(NULL), - checkbox_info_label_(NULL), - unchecked_boxes_(0), handled_result_(false) { InitView(); } @@ -215,7 +168,7 @@ ExtensionInstallDialogView::~ExtensionInstallDialogView() { } void ExtensionInstallDialogView::InitView() { - // Possible grid layouts without ExtensionPermissionDialog experiment: + // Possible grid layouts: // Inline install // w/ permissions no permissions // +--------------------+------+ +--------------+------+ @@ -247,54 +200,6 @@ void ExtensionInstallDialogView::InitView() { // +--------------------| | // | permission2 | | // +--------------------+------+ - // - // If the ExtensionPermissionDialog is on, the layout is modified depending - // on the experiment group. For text only experiment, a footer is added at the - // bottom of the layouts. For others, inline details are added below some of - // the permissions. - // - // Regular install w/ permissions and footer (experiment): - // +--------------------+------+ - // | heading | icon | - // +--------------------| | - // | permissions_header | | - // +--------------------| | - // | permission1 | | - // +--------------------| | - // | permission2 | | - // +--------------------+------+ - // | footer text | | - // +--------------------+------+ - // - // Regular install w/ permissions and inline explanations (experiment): - // +--------------------+------+ - // | heading | icon | - // +--------------------| | - // | permissions_header | | - // +--------------------| | - // | permission1 | | - // +--------------------| | - // | explanation1 | | - // +--------------------| | - // | permission2 | | - // +--------------------| | - // | explanation2 | | - // +--------------------+------+ - // - // Regular install w/ permissions and inline explanations (experiment): - // +--------------------+------+ - // | heading | icon | - // +--------------------| | - // | permissions_header | | - // +--------------------| | - // |checkbox|permission1| | - // +--------------------| | - // |checkbox|permission2| | - // +--------------------+------+ - // - // Additionally, links or informational text is added to non-client areas of - // the dialog depending on the experiment group. - int left_column_width = (prompt_->ShouldShowPermissions() + prompt_->GetRetainedFileCount()) > 0 ? kPermissionsLeftColumnWidth @@ -316,27 +221,12 @@ void ExtensionInstallDialogView::InitView() { 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 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(); - CreateLayout(scrollable_header_only_, left_column_width, - column_set_id, true); - scroll_view_->SetContents(scrollable_header_only_); - } else { - scroll_view_->SetContents(scrollable_); - } + scroll_view_->SetContents(scrollable_); int dialog_width = left_column_width + 2 * views::kPanelHorizMargin; if (!is_bundle_install()) dialog_width += views::kPanelHorizMargin + kIconSize + kIconOffset; - // 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()) - dialog_width += 4 * views::kPanelHorizMargin; - if (prompt_->has_webstore_data()) { layout->StartRow(0, column_set_id); views::View* rating = new views::View(); @@ -454,8 +344,6 @@ void ExtensionInstallDialogView::InitView() { base::string16(), details, space_for_files_and_devices, - false, - true, false); layout->AddView(issue_advice_view); } @@ -481,8 +369,6 @@ void ExtensionInstallDialogView::InitView() { base::string16(), details, space_for_files_and_devices, - false, - true, false); layout->AddView(issue_advice_view); } @@ -492,73 +378,12 @@ void ExtensionInstallDialogView::InitView() { prompt_->type(), ExtensionInstallPrompt::NUM_PROMPT_TYPES); - if (prompt_->ShouldShowPermissions()) { - if (prompt_->ShouldShowExplanationText()) { - views::ColumnSet* column_set = layout->AddColumnSet(++column_set_id); - column_set->AddColumn(views::GridLayout::LEADING, - views::GridLayout::FILL, - 1, - views::GridLayout::USE_PREF, - 0, - 0); - // Add two rows of space so that the text stands out. - layout->AddPaddingRow(0, 2 * views::kRelatedControlVerticalSpacing); - - layout->StartRow(0, column_set_id); - 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())) { - // 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) { - int text_id = - 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); - show_details_link_->set_listener(this); - UpdateLinkActionHistogram(LINK_SHOWN); - } else { - UpdateLinkActionHistogram(LINK_NOT_SHOWN); - } - } - - if (prompt_->experiment()->show_checkboxes()) { - checkbox_info_label_ = new views::Label( - l10n_util::GetStringUTF16( - IDS_EXTENSION_PROMPT_EXPERIMENT_CHECKBOX_INFO)); - checkbox_info_label_->SetMultiLine(true); - checkbox_info_label_->SetHorizontalAlignment(gfx::ALIGN_LEFT); - checkbox_info_label_->SetAutoColorReadabilityEnabled(false); - checkbox_info_label_->SetEnabledColor(kLighterLabelColor); - } - } - gfx::Size scrollable_size = scrollable_->GetPreferredSize(); scrollable_->SetBoundsRect(gfx::Rect(scrollable_size)); dialog_size_ = gfx::Size( dialog_width, std::min(scrollable_size.height(), kDialogMaxHeight)); - if (scrollable_header_only_) { - gfx::Size header_only_size = scrollable_header_only_->GetPreferredSize(); - scrollable_header_only_->SetBoundsRect(gfx::Rect(header_only_size)); - dialog_size_ = gfx::Size( - dialog_width, std::min(header_only_size.height(), kDialogMaxHeight)); - } - std::string event_name = ExperienceSamplingEvent::kExtensionInstallDialog; event_name.append( ExtensionInstallPrompt::PromptTypeToString(prompt_->type())); @@ -609,30 +434,10 @@ bool ExtensionInstallDialogView::AddPermissions( views::Label* permission_label = new views::Label(prompt_->GetPermission(i, perm_type)); - const SkColor kTextHighlight = SK_ColorRED; - const SkColor kBackgroundHighlight = SkColorSetRGB(0xFB, 0xF7, 0xA3); - if (prompt_->experiment()->ShouldHighlightText( - prompt_->GetPermission(i, perm_type))) { - permission_label->SetAutoColorReadabilityEnabled(false); - permission_label->SetEnabledColor(kTextHighlight); - } else if (prompt_->experiment()->ShouldHighlightBackground( - prompt_->GetPermission(i, perm_type))) { - permission_label->SetLineHeight(18); - permission_label->set_background( - views::Background::CreateSolidBackground(kBackgroundHighlight)); - } - permission_label->SetMultiLine(true); permission_label->SetHorizontalAlignment(gfx::ALIGN_LEFT); - - if (prompt_->experiment()->show_checkboxes()) { - permission_label->SizeToFit(left_column_width); - layout->AddView(new CheckboxedView(permission_label, this)); - ++unchecked_boxes_; - } else { - permission_label->SizeToFit(left_column_width - kBulletWidth); - layout->AddView(new BulletedView(permission_label)); - } + permission_label->SizeToFit(left_column_width - kBulletWidth); + layout->AddView(new BulletedView(permission_label)); // If we have more details to provide, show them in collapsed form. if (!prompt_->GetPermissionsDetails(i, perm_type).empty()) { @@ -645,35 +450,9 @@ bool ExtensionInstallDialogView::AddPermissions( base::string16(), details, left_column_width, - true, - true, - false); + true); layout->AddView(details_container); } - - if (prompt_->experiment()->should_show_inline_explanations()) { - base::string16 explanation = prompt_->experiment()->GetInlineExplanation( - prompt_->GetPermission(i, perm_type)); - if (!explanation.empty()) { - PermissionDetails details; - details.push_back(explanation); - ExpandableContainerView* container = - new ExpandableContainerView(this, - base::string16(), - details, - left_column_width, - false, - false, - true); - // Inline explanations are expanded by default if there is - // no "Show details" link. - if (!prompt_->experiment()->show_details_link()) - container->ExpandWithoutAnimation(); - layout->StartRow(0, column_set_id); - layout->AddView(container); - inline_explanations_.push_back(container); - } - } } return true; } @@ -753,29 +532,6 @@ void ExtensionInstallDialogView::ContentsChanged() { Layout(); } -void ExtensionInstallDialogView::ViewHierarchyChanged( - const ViewHierarchyChangedDetails& details) { - views::DialogDelegateView::ViewHierarchyChanged(details); - // Since we want the links to show up in the same visual row as the accept - // and cancel buttons, which is provided by the framework, we must add the - // buttons to the non-client view, which is the parent of this view. - // Similarly, when we're removed from the view hierarchy, we must take care - // to clean up those items as well. - if (details.child == this) { - if (details.is_add) { - if (show_details_link_) - details.parent->AddChildView(show_details_link_); - if (checkbox_info_label_) - details.parent->AddChildView(checkbox_info_label_); - } else { - if (show_details_link_) - details.parent->RemoveChildView(show_details_link_); - if (checkbox_info_label_) - details.parent->RemoveChildView(checkbox_info_label_); - } - } -} - int ExtensionInstallDialogView::GetDialogButtons() const { int buttons = prompt_->GetDialogButtons(); // Simply having just an OK button is *not* supported. See comment on function @@ -836,85 +592,25 @@ base::string16 ExtensionInstallDialogView::GetWindowTitle() const { void ExtensionInstallDialogView::LinkClicked(views::Link* source, int event_flags) { - if (source == show_details_link_) { - 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()) { - gfx::Rect bounds = GetWidget()->GetWindowBoundsInScreen(); - int spacing = bounds.height() - - scrollable_header_only_->GetPreferredSize().height(); - int content_height = std::min(scrollable_->GetPreferredSize().height(), - kDialogMaxHeight); - bounds.set_height(spacing + content_height); - scroll_view_->SetContents(scrollable_); - GetWidget()->SetBoundsConstrained(bounds); - ContentsChanged(); - } else { - ToggleInlineExplanations(); - } - show_details_link_->SetVisible(false); + GURL store_url(extension_urls::GetWebstoreItemDetailURLPrefix() + + prompt_->extension()->id()); + OpenURLParams params( + store_url, Referrer(), NEW_FOREGROUND_TAB, + ui::PAGE_TRANSITION_LINK, + false); + + if (navigator_) { + navigator_->OpenURL(params); } else { - GURL store_url(extension_urls::GetWebstoreItemDetailURLPrefix() + - prompt_->extension()->id()); - OpenURLParams params( - store_url, Referrer(), NEW_FOREGROUND_TAB, - ui::PAGE_TRANSITION_LINK, - false); - - if (navigator_) { - navigator_->OpenURL(params); - } else { - chrome::ScopedTabbedBrowserDisplayer displayer( - profile_, chrome::GetActiveDesktop()); - displayer.browser()->OpenURL(params); - } - GetWidget()->Close(); + chrome::ScopedTabbedBrowserDisplayer displayer( + profile_, chrome::GetActiveDesktop()); + displayer.browser()->OpenURL(params); } -} - -void ExtensionInstallDialogView::ToggleInlineExplanations() { - for (InlineExplanations::iterator it = inline_explanations_.begin(); - it != inline_explanations_.end(); ++it) - (*it)->ToggleDetailLevel(); + GetWidget()->Close(); } void ExtensionInstallDialogView::Layout() { scroll_view_->SetBounds(0, 0, width(), height()); - - if (show_details_link_ || checkbox_info_label_) { - views::LabelButton* cancel_button = GetDialogClientView()->cancel_button(); - gfx::Rect parent_bounds = parent()->GetContentsBounds(); - // By default, layouts have an inset of kButtonHEdgeMarginNew. In order to - // align the link horizontally with the left side of the contents of the - // layout, put a horizontal margin with this amount. - const int horizontal_margin = views::kButtonHEdgeMarginNew; - const int vertical_margin = views::kButtonVEdgeMarginNew; - int y_buttons = parent_bounds.bottom() - - cancel_button->GetPreferredSize().height() - vertical_margin; - int max_width = dialog_size_.width() - cancel_button->width() * 2 - - horizontal_margin * 2 - views::kRelatedButtonHSpacing; - if (show_details_link_) { - gfx::Size link_size = show_details_link_->GetPreferredSize(); - show_details_link_->SetBounds( - horizontal_margin, - y_buttons + (cancel_button->height() - link_size.height()) / 2, - link_size.width(), link_size.height()); - } - if (checkbox_info_label_) { - gfx::Size label_size = checkbox_info_label_->GetPreferredSize(); - checkbox_info_label_->SetBounds( - horizontal_margin, - y_buttons + (cancel_button->height() - label_size.height()) / 2, - label_size.width(), label_size.height()); - checkbox_info_label_->SizeToFit(max_width); - } - } - // Disable accept button if there are unchecked boxes and - // the experiment is on. - if (prompt_->experiment()->show_checkboxes()) - GetDialogClientView()->ok_button()->SetEnabled(unchecked_boxes_ == 0); - DialogDelegateView::Layout(); } @@ -922,51 +618,18 @@ gfx::Size ExtensionInstallDialogView::GetPreferredSize() const { return dialog_size_; } -void ExtensionInstallDialogView::ButtonPressed(views::Button* sender, - const ui::Event& event) { - if (std::string(views::Checkbox::kViewClassName) == sender->GetClassName()) { - views::Checkbox* checkbox = static_cast<views::Checkbox*>(sender); - if (checkbox->checked()) - --unchecked_boxes_; - else - ++unchecked_boxes_; - - GetDialogClientView()->ok_button()->SetEnabled(unchecked_boxes_ == 0); - checkbox_info_label_->SetVisible(unchecked_boxes_ > 0); - } -} - void ExtensionInstallDialogView::UpdateInstallResultHistogram(bool accepted) const { 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()) { - // The clickable link in the UI is "Show Permissions". - UMA_HISTOGRAM_ENUMERATION( - "Extensions.InstallPromptExperiment.ShowPermissions", - action_type, - NUM_LINK_ACTIONS); - } else { - // The clickable link in the UI is "Show Details". - UMA_HISTOGRAM_ENUMERATION( - "Extensions.InstallPromptExperiment.ShowDetails", - action_type, - NUM_LINK_ACTIONS); - } -} - // ExpandableContainerView::DetailsView ---------------------------------------- ExpandableContainerView::DetailsView::DetailsView(int horizontal_space, - bool parent_bulleted, - bool lighter_color) + bool parent_bulleted) : layout_(new views::GridLayout(this)), - state_(0), - lighter_color_(lighter_color) { + state_(0) { SetLayoutManager(layout_); views::ColumnSet* column_set = layout_->AddColumnSet(0); // If the parent is using bullets for its items, then a padding of one unit @@ -992,10 +655,6 @@ void ExpandableContainerView::DetailsView::AddDetail( new views::Label(PrepareForDisplay(detail, false)); detail_label->SetMultiLine(true); detail_label->SetHorizontalAlignment(gfx::ALIGN_LEFT); - if (lighter_color_) { - detail_label->SetEnabledColor(kLighterLabelColor); - detail_label->SetAutoColorReadabilityEnabled(false); - } layout_->AddView(detail_label); } @@ -1017,13 +676,11 @@ ExpandableContainerView::ExpandableContainerView( const base::string16& description, const PermissionDetails& details, int horizontal_space, - bool parent_bulleted, - bool show_expand_link, - bool lighter_color_details) + bool parent_bulleted) : owner_(owner), details_view_(NULL), - more_details_(NULL), slide_animation_(this), + more_details_(NULL), arrow_toggle_(NULL), expanded_(false) { views::GridLayout* layout = new views::GridLayout(this); @@ -1049,8 +706,7 @@ ExpandableContainerView::ExpandableContainerView( if (details.empty()) return; - details_view_ = new DetailsView(horizontal_space, parent_bulleted, - lighter_color_details); + details_view_ = new DetailsView(horizontal_space, parent_bulleted); layout->StartRow(0, column_set_id); layout->AddView(details_view_); @@ -1058,60 +714,57 @@ ExpandableContainerView::ExpandableContainerView( for (size_t i = 0; i < details.size(); ++i) details_view_->AddDetail(details[i]); - // TODO(meacer): Remove show_expand_link when the experiment is completed. - if (show_expand_link) { - views::Link* link = new views::Link( - l10n_util::GetStringUTF16(IDS_EXTENSIONS_SHOW_DETAILS)); + views::Link* link = new views::Link( + l10n_util::GetStringUTF16(IDS_EXTENSIONS_SHOW_DETAILS)); + + // Make sure the link width column is as wide as needed for both Show and + // Hide details, so that the arrow doesn't shift horizontally when we + // toggle. + int link_col_width = + views::kRelatedControlHorizontalSpacing + + std::max(gfx::GetStringWidth( + l10n_util::GetStringUTF16(IDS_EXTENSIONS_HIDE_DETAILS), + link->font_list()), + gfx::GetStringWidth( + l10n_util::GetStringUTF16(IDS_EXTENSIONS_SHOW_DETAILS), + link->font_list())); + + column_set = layout->AddColumnSet(++column_set_id); + // Padding to the left of the More Details column. If the parent is using + // bullets for its items, then a padding of one unit will make the child + // item (which has no bullet) look like a sibling of its parent. Therefore + // increase the indentation by one more unit to show that it is in fact a + // child item (with no missing bullet) and not a sibling. + column_set->AddPaddingColumn( + 0, views::kRelatedControlHorizontalSpacing * (parent_bulleted ? 2 : 1)); + // The More Details column. + column_set->AddColumn(views::GridLayout::LEADING, + views::GridLayout::LEADING, + 0, + views::GridLayout::FIXED, + link_col_width, + link_col_width); + // The Up/Down arrow column. + column_set->AddColumn(views::GridLayout::LEADING, + views::GridLayout::LEADING, + 0, + views::GridLayout::USE_PREF, + 0, + 0); - // Make sure the link width column is as wide as needed for both Show and - // Hide details, so that the arrow doesn't shift horizontally when we - // toggle. - int link_col_width = - views::kRelatedControlHorizontalSpacing + - std::max(gfx::GetStringWidth( - l10n_util::GetStringUTF16(IDS_EXTENSIONS_HIDE_DETAILS), - link->font_list()), - gfx::GetStringWidth( - l10n_util::GetStringUTF16(IDS_EXTENSIONS_SHOW_DETAILS), - link->font_list())); - - column_set = layout->AddColumnSet(++column_set_id); - // Padding to the left of the More Details column. If the parent is using - // bullets for its items, then a padding of one unit will make the child - // item (which has no bullet) look like a sibling of its parent. Therefore - // increase the indentation by one more unit to show that it is in fact a - // child item (with no missing bullet) and not a sibling. - column_set->AddPaddingColumn( - 0, views::kRelatedControlHorizontalSpacing * (parent_bulleted ? 2 : 1)); - // The More Details column. - column_set->AddColumn(views::GridLayout::LEADING, - views::GridLayout::LEADING, - 0, - views::GridLayout::FIXED, - link_col_width, - link_col_width); - // The Up/Down arrow column. - column_set->AddColumn(views::GridLayout::LEADING, - views::GridLayout::LEADING, - 0, - views::GridLayout::USE_PREF, - 0, - 0); + // Add the More Details link. + layout->StartRow(0, column_set_id); + more_details_ = link; + more_details_->set_listener(this); + more_details_->SetHorizontalAlignment(gfx::ALIGN_LEFT); + layout->AddView(more_details_); - // Add the More Details link. - layout->StartRow(0, column_set_id); - more_details_ = link; - more_details_->set_listener(this); - more_details_->SetHorizontalAlignment(gfx::ALIGN_LEFT); - layout->AddView(more_details_); - - // Add the arrow after the More Details link. - ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance(); - arrow_toggle_ = new views::ImageButton(this); - arrow_toggle_->SetImage(views::Button::STATE_NORMAL, - rb.GetImageSkiaNamed(IDR_DOWN_ARROW)); - layout->AddView(arrow_toggle_); - } + // Add the arrow after the More Details link. + ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance(); + arrow_toggle_ = new views::ImageButton(this); + arrow_toggle_->SetImage(views::Button::STATE_NORMAL, + rb.GetImageSkiaNamed(IDR_DOWN_ARROW)); + layout->AddView(arrow_toggle_); } ExpandableContainerView::~ExpandableContainerView() { @@ -1168,11 +821,6 @@ void ExpandableContainerView::ToggleDetailLevel() { slide_animation_.Show(); } -void ExpandableContainerView::ExpandWithoutAnimation() { - expanded_ = true; - details_view_->AnimateToState(1.0); -} - // static ExtensionInstallPrompt::ShowDialogCallback ExtensionInstallPrompt::GetDefaultShowDialogCallback() { diff --git a/chrome/browser/ui/views/extensions/extension_install_dialog_view.h b/chrome/browser/ui/views/extensions/extension_install_dialog_view.h index 93d721c..c904edd 100644 --- a/chrome/browser/ui/views/extensions/extension_install_dialog_view.h +++ b/chrome/browser/ui/views/extensions/extension_install_dialog_view.h @@ -14,7 +14,6 @@ #include "ui/views/window/dialog_delegate.h" typedef std::vector<base::string16> PermissionDetails; -class ExpandableContainerView; class ExtensionInstallPromptShowParams; class Profile; @@ -51,8 +50,7 @@ class CustomScrollableView : public views::View { // Implements the extension installation dialog for TOOLKIT_VIEWS. class ExtensionInstallDialogView : public views::DialogDelegateView, - public views::LinkListener, - public views::ButtonListener { + public views::LinkListener { public: ExtensionInstallDialogView( Profile* profile, @@ -79,18 +77,10 @@ class ExtensionInstallDialogView : public views::DialogDelegateView, base::string16 GetWindowTitle() const override; void Layout() override; gfx::Size GetPreferredSize() const override; - void ViewHierarchyChanged( - const ViewHierarchyChangedDetails& details) override; // views::LinkListener: void LinkClicked(views::Link* source, int event_flags) override; - // views::ButtonListener: - void ButtonPressed(views::Button* sender, const ui::Event& event) override; - - // Experimental: Toggles inline permission explanations with an animation. - void ToggleInlineExplanations(); - // Initializes the dialog view, adding in permissions if they exist. void InitView(); @@ -123,10 +113,6 @@ class ExtensionInstallDialogView : public views::DialogDelegateView, // Updates the histogram that holds installation accepted/aborted data. void UpdateInstallResultHistogram(bool accepted) const; - // Updates the histogram that holds data about whether "Show details" or - // "Show permissions" links were shown and/or clicked. - void UpdateLinkActionHistogram(int action_type) const; - Profile* profile_; content::PageNavigator* navigator_; ExtensionInstallPrompt::Delegate* delegate_; @@ -139,29 +125,9 @@ class ExtensionInstallDialogView : public views::DialogDelegateView, // The container view for the scroll view. CustomScrollableView* scrollable_; - // The container for the simpler view with only the dialog header and the - // extension icon. Used for the experiment where the permissions are - // initially hidden when the dialog shows. - CustomScrollableView* scrollable_header_only_; - // The preferred size of the dialog. gfx::Size dialog_size_; - // Experimental: "Show details" link to expand inline explanations and reveal - // permision dialog. - views::Link* show_details_link_; - - // Experimental: Label for showing information about the checkboxes. - views::Label* checkbox_info_label_; - - // Experimental: Contains pointers to inline explanation views. - typedef std::vector<ExpandableContainerView*> InlineExplanations; - InlineExplanations inline_explanations_; - - // Experimental: Number of unchecked checkboxes in the permission list. - // If this becomes zero, the accept button is enabled, otherwise disabled. - int unchecked_boxes_; - // ExperienceSampling: Track this UI event. scoped_ptr<extensions::ExperienceSamplingEvent> sampling_event_; @@ -181,16 +147,6 @@ class BulletedView : public views::View { DISALLOW_COPY_AND_ASSIGN(BulletedView); }; -// A simple view that prepends a view with a checkbox with the help of a grid -// layout. Used for the permission experiment. -// TODO(meacer): Remove once the experiment is completed. -class CheckboxedView : public views::View { - public: - CheckboxedView(views::View* view, views::ButtonListener* listener); - private: - DISALLOW_COPY_AND_ASSIGN(CheckboxedView); -}; - // A view to display text with an expandable details section. class ExpandableContainerView : public views::View, public views::ButtonListener, @@ -201,9 +157,7 @@ class ExpandableContainerView : public views::View, const base::string16& description, const PermissionDetails& details, int horizontal_space, - bool parent_bulleted, - bool show_expand_link, - bool lighter_color_details); + bool parent_bulleted); ~ExpandableContainerView() override; // views::View: @@ -219,19 +173,11 @@ class ExpandableContainerView : public views::View, void AnimationProgressed(const gfx::Animation* animation) override; void AnimationEnded(const gfx::Animation* animation) override; - // Expand/Collapse the detail section for this ExpandableContainerView. - void ToggleDetailLevel(); - - // Expand the detail section without any animation. - // TODO(meacer): Remove once the experiment is completed. - void ExpandWithoutAnimation(); - private: // A view which displays all the details of an IssueAdviceInfoEntry. class DetailsView : public views::View { public: - explicit DetailsView(int horizontal_space, bool parent_bulleted, - bool lighter_color); + DetailsView(int horizontal_space, bool parent_bulleted); ~DetailsView() override {} // views::View: @@ -246,24 +192,24 @@ class ExpandableContainerView : public views::View, views::GridLayout* layout_; double state_; - // Whether the detail text should be shown with a lighter color. - bool lighter_color_; - DISALLOW_COPY_AND_ASSIGN(DetailsView); }; + // Expand/Collapse the detail section for this ExpandableContainerView. + void ToggleDetailLevel(); + // The dialog that owns |this|. It's also an ancestor in the View hierarchy. ExtensionInstallDialogView* owner_; // A view for showing |issue_advice.details|. DetailsView* details_view_; + gfx::SlideAnimation slide_animation_; + // The 'more details' link shown under the heading (changes to 'hide details' // when the details section is expanded). views::Link* more_details_; - gfx::SlideAnimation slide_animation_; - // The up/down arrow next to the 'more detail' link (points up/down depending // on whether the details section is expanded). views::ImageButton* arrow_toggle_; diff --git a/chrome/browser/ui/views/extensions/extension_install_dialog_view_browsertest.cc b/chrome/browser/ui/views/extensions/extension_install_dialog_view_browsertest.cc index 2de6c33..801bc28 100644 --- a/chrome/browser/ui/views/extensions/extension_install_dialog_view_browsertest.cc +++ b/chrome/browser/ui/views/extensions/extension_install_dialog_view_browsertest.cc @@ -8,7 +8,6 @@ #include "chrome/browser/extensions/extension_browsertest.h" #include "chrome/browser/extensions/extension_icon_manager.h" #include "chrome/browser/extensions/extension_install_prompt.h" -#include "chrome/browser/extensions/extension_install_prompt_experiment.h" #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/tabs/tab_strip_model.h" #include "chrome/browser/ui/webui/extensions/extension_settings_handler.h" @@ -114,7 +113,6 @@ void ExtensionInstallDialogViewTestBase::SetUpOnMainThread() { install_prompt_ = new MockExtensionInstallPrompt(web_contents_); install_prompt_->set_prompt(prompt_.get()); - prompt_->set_experiment(ExtensionInstallPromptExperiment::ControlGroup()); prompt_->set_extension(extension_); scoped_ptr<ExtensionIconManager> icon_manager(new ExtensionIconManager()); diff --git a/chrome/chrome_browser_extensions.gypi b/chrome/chrome_browser_extensions.gypi index a100bd3..6466702 100644 --- a/chrome/chrome_browser_extensions.gypi +++ b/chrome/chrome_browser_extensions.gypi @@ -582,8 +582,6 @@ 'browser/extensions/extension_install_checker.h', 'browser/extensions/extension_install_prompt.cc', 'browser/extensions/extension_install_prompt.h', - 'browser/extensions/extension_install_prompt_experiment.cc', - 'browser/extensions/extension_install_prompt_experiment.h', 'browser/extensions/extension_install_prompt_show_params.cc', 'browser/extensions/extension_install_prompt_show_params.h', 'browser/extensions/extension_install_ui_util.cc', |