summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorfinnur@chromium.org <finnur@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2012-10-23 22:30:43 +0000
committerfinnur@chromium.org <finnur@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2012-10-23 22:30:43 +0000
commitb0def91df95c61f7c6384d0a001f09a701fe0063 (patch)
tree04ceb72d2384a5268ac268697dad399d830295a8
parent8e603cd97952bda6c22a07c46cdbe01b090a1ce2 (diff)
downloadchromium_src-b0def91df95c61f7c6384d0a001f09a701fe0063.zip
chromium_src-b0def91df95c61f7c6384d0a001f09a701fe0063.tar.gz
chromium_src-b0def91df95c61f7c6384d0a001f09a701fe0063.tar.bz2
Make sure sideload wipeout doesn't interfere with the tests.
Make sure it does not run on other platforms besides Windows, where it was intended (for now). Fix the problem with bubble appearing to the right of the browser. Make sure the bubble does not appear on next run after explicitly dismissing it. Enable sideload wipeout by default. BUG=154624 TEST=None Review URL: https://codereview.chromium.org/11232060 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@163705 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r--chrome/browser/extensions/extension_browsertest.cc5
-rw-r--r--chrome/browser/extensions/extension_browsertest.h4
-rw-r--r--chrome/browser/extensions/extension_service.cc4
-rw-r--r--chrome/browser/extensions/extension_service_unittest.cc21
-rw-r--r--chrome/browser/ui/views/extensions/disabled_extensions_view.cc44
-rw-r--r--chrome/browser/ui/views/extensions/disabled_extensions_view.h5
-rw-r--r--chrome/browser/ui/views/toolbar_view.cc21
-rw-r--r--chrome/browser/ui/views/toolbar_view.h7
-rw-r--r--chrome/common/extensions/feature_switch.cc1
9 files changed, 74 insertions, 38 deletions
diff --git a/chrome/browser/extensions/extension_browsertest.cc b/chrome/browser/extensions/extension_browsertest.cc
index f63ce8f..9736df5 100644
--- a/chrome/browser/extensions/extension_browsertest.cc
+++ b/chrome/browser/extensions/extension_browsertest.cc
@@ -43,6 +43,7 @@
using extensions::Extension;
using extensions::ExtensionCreator;
+using extensions::FeatureSwitch;
ExtensionBrowserTest::ExtensionBrowserTest()
: loaded_(false),
@@ -53,7 +54,9 @@ ExtensionBrowserTest::ExtensionBrowserTest()
target_visible_page_action_count_(-1),
current_channel_(chrome::VersionInfo::CHANNEL_DEV),
override_prompt_for_external_extensions_(
- extensions::FeatureSwitch::prompt_for_external_extensions(), false) {
+ FeatureSwitch::prompt_for_external_extensions(), false),
+ override_sideload_wipeout_(
+ FeatureSwitch::sideload_wipeout(), false) {
EXPECT_TRUE(temp_dir_.CreateUniqueTempDir());
}
diff --git a/chrome/browser/extensions/extension_browsertest.h b/chrome/browser/extensions/extension_browsertest.h
index 3fd53a2..bc434a4 100644
--- a/chrome/browser/extensions/extension_browsertest.h
+++ b/chrome/browser/extensions/extension_browsertest.h
@@ -254,6 +254,10 @@ class ExtensionBrowserTest : virtual public InProcessBrowserTest,
// Disable external install UI.
extensions::FeatureSwitch::ScopedOverride
override_prompt_for_external_extensions_;
+
+ // Disable the sideload wipeout UI.
+ extensions::FeatureSwitch::ScopedOverride
+ override_sideload_wipeout_;
};
#endif // CHROME_BROWSER_EXTENSIONS_EXTENSION_BROWSERTEST_H_
diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc
index 7c6f632..c601859 100644
--- a/chrome/browser/extensions/extension_service.cc
+++ b/chrome/browser/extensions/extension_service.cc
@@ -2190,8 +2190,10 @@ void ExtensionService::MaybeWipeout(
if (done)
return;
+ Extension::Type type = extension->GetType();
int disable_reasons = extension_prefs_->GetDisableReasons(extension->id());
- if (disable_reasons == Extension::DISABLE_NONE) {
+ if (disable_reasons == Extension::DISABLE_NONE &&
+ type == Extension::TYPE_EXTENSION) {
Extension::Location location = extension->location();
if (location == Extension::EXTERNAL_REGISTRY ||
(location == Extension::INTERNAL && !extension->from_webstore())) {
diff --git a/chrome/browser/extensions/extension_service_unittest.cc b/chrome/browser/extensions/extension_service_unittest.cc
index 80fc681..3d0bb36 100644
--- a/chrome/browser/extensions/extension_service_unittest.cc
+++ b/chrome/browser/extensions/extension_service_unittest.cc
@@ -111,6 +111,7 @@ using extensions::Extension;
using extensions::ExtensionCreator;
using extensions::ExtensionPrefs;
using extensions::ExtensionSystem;
+using extensions::FeatureSwitch;
using extensions::PermissionSet;
namespace keys = extension_manifest_keys;
@@ -518,8 +519,9 @@ class ExtensionServiceTest
ExtensionServiceTest()
: installed_(NULL),
override_external_install_prompt_(
- extensions::FeatureSwitch::prompt_for_external_extensions(),
- false) {
+ FeatureSwitch::prompt_for_external_extensions(), false),
+ override_sideload_wipeout_(
+ FeatureSwitch::sideload_wipeout(), false) {
registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_LOADED,
content::NotificationService::AllSources());
registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_UNLOADED,
@@ -1007,7 +1009,8 @@ class ExtensionServiceTest
extensions::ExtensionList loaded_;
std::string unloaded_id_;
const Extension* installed_;
- extensions::FeatureSwitch::ScopedOverride override_external_install_prompt_;
+ FeatureSwitch::ScopedOverride override_external_install_prompt_;
+ FeatureSwitch::ScopedOverride override_sideload_wipeout_;
private:
content::NotificationRegistrar registrar_;
@@ -5606,8 +5609,8 @@ TEST_F(ExtensionSourcePriorityTest, InstallExternalBlocksSyncRequest) {
#if ENABLE_EXTERNAL_INSTALL_UI
// Test that installing an external extension displays a GlobalError.
TEST_F(ExtensionServiceTest, ExternalInstallGlobalError) {
- extensions::FeatureSwitch::ScopedOverride prompt(
- extensions::FeatureSwitch::prompt_for_external_extensions(), true);
+ FeatureSwitch::ScopedOverride prompt(
+ FeatureSwitch::prompt_for_external_extensions(), true);
InitializeEmptyExtensionService();
MockExtensionProvider* provider =
@@ -5653,8 +5656,8 @@ TEST_F(ExtensionServiceTest, ExternalInstallGlobalError) {
// Test that external extensions are initially disabled, and that enabling
// them clears the prompt.
TEST_F(ExtensionServiceTest, ExternalInstallInitiallyDisabled) {
- extensions::FeatureSwitch::ScopedOverride prompt(
- extensions::FeatureSwitch::prompt_for_external_extensions(), true);
+ FeatureSwitch::ScopedOverride prompt(
+ FeatureSwitch::prompt_for_external_extensions(), true);
InitializeEmptyExtensionService();
MockExtensionProvider* provider =
@@ -5681,8 +5684,8 @@ TEST_F(ExtensionServiceTest, ExternalInstallInitiallyDisabled) {
// Test that installing multiple external extensions works.
TEST_F(ExtensionServiceTest, ExternalInstallMultiple) {
- extensions::FeatureSwitch::ScopedOverride prompt(
- extensions::FeatureSwitch::prompt_for_external_extensions(), true);
+ FeatureSwitch::ScopedOverride prompt(
+ FeatureSwitch::prompt_for_external_extensions(), true);
InitializeEmptyExtensionService();
MockExtensionProvider* provider =
diff --git a/chrome/browser/ui/views/extensions/disabled_extensions_view.cc b/chrome/browser/ui/views/extensions/disabled_extensions_view.cc
index a5b016e..82d0722 100644
--- a/chrome/browser/ui/views/extensions/disabled_extensions_view.cc
+++ b/chrome/browser/ui/views/extensions/disabled_extensions_view.cc
@@ -9,9 +9,12 @@
#include "base/utf_string_conversions.h"
#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/extensions/extension_system.h"
+#include "chrome/browser/prefs/pref_service.h"
+#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/singleton_tabs.h"
#include "chrome/common/extensions/feature_switch.h"
+#include "chrome/common/pref_names.h"
#include "chrome/common/url_constants.h"
#include "content/public/browser/user_metrics.h"
#include "grit/generated_resources.h"
@@ -43,6 +46,9 @@ const int kHeadlineMessagePadding = 4;
const int kHeadlineRowPadding = 10;
const int kMessageBubblePadding = 11;
+// How often to show the disabled extension (sideload wipeout) bubble.
+const int kShowSideloadWipeoutBubbleMax = 3;
+
// How many extensions to show in the bubble (max).
const int kMaxExtensionsToShow = 7;
@@ -52,21 +58,25 @@ const int kMaxExtensionsToShow = 7;
// DisabledExtensionsView
// static
-bool DisabledExtensionsView::MaybeShow(Browser* browser,
+void DisabledExtensionsView::MaybeShow(Browser* browser,
views::View* anchor_view) {
-#if !defined(OS_WIN)
- // We are targeting registry-installed extensions, which is Windows-specific,
- // and extensions marked internal and not from the web store, which are mostly
- // problematic on Windows.
- return false;
-#endif
-
if (!extensions::FeatureSwitch::sideload_wipeout()->IsEnabled())
- return false;
+ return;
static bool done_showing_ui = false;
if (done_showing_ui)
- return false; // Only show the bubble once per launch.
+ return; // Only show the bubble once per launch.
+
+ // A pref that counts how often the bubble has been shown.
+ IntegerPrefMember sideload_wipeout_bubble_shown;
+
+ sideload_wipeout_bubble_shown.Init(
+ prefs::kExtensionsSideloadWipeoutBubbleShown,
+ browser->profile()->GetPrefs(), NULL);
+ int bubble_shown_count = sideload_wipeout_bubble_shown.GetValue();
+ if (bubble_shown_count >= kShowSideloadWipeoutBubbleMax)
+ return;
+ sideload_wipeout_bubble_shown.SetValue(++bubble_shown_count);
// Fetch all disabled extensions.
ExtensionService* extension_service =
@@ -82,10 +92,7 @@ bool DisabledExtensionsView::MaybeShow(Browser* browser,
bubble_delegate->StartFade(true);
done_showing_ui = true;
- return true;
}
-
- return false;
}
DisabledExtensionsView::DisabledExtensionsView(
@@ -109,6 +116,14 @@ DisabledExtensionsView::DisabledExtensionsView(
DisabledExtensionsView::~DisabledExtensionsView() {
}
+void DisabledExtensionsView::DontShowBubbleAgain() {
+ IntegerPrefMember sideload_wipeout_bubble_shown;
+ sideload_wipeout_bubble_shown.Init(
+ prefs::kExtensionsSideloadWipeoutBubbleShown,
+ browser_->profile()->GetPrefs(), NULL);
+ sideload_wipeout_bubble_shown.SetValue(kShowSideloadWipeoutBubbleMax);
+}
+
void DisabledExtensionsView::ButtonPressed(views::Button* sender,
const ui::Event& event) {
if (sender == settings_button_) {
@@ -123,11 +138,11 @@ void DisabledExtensionsView::ButtonPressed(views::Button* sender,
} else if (sender == dismiss_button_) {
content::RecordAction(UserMetricsAction("DisabledExtension_Dismiss"));
- // No action required. Close will happen below.
} else {
NOTREACHED();
}
+ DontShowBubbleAgain();
GetWidget()->Close();
}
@@ -141,6 +156,7 @@ void DisabledExtensionsView::LinkClicked(
content::PAGE_TRANSITION_LINK,
false));
+ DontShowBubbleAgain();
GetWidget()->Close();
}
diff --git a/chrome/browser/ui/views/extensions/disabled_extensions_view.h b/chrome/browser/ui/views/extensions/disabled_extensions_view.h
index 19ea744..f49a340 100644
--- a/chrome/browser/ui/views/extensions/disabled_extensions_view.h
+++ b/chrome/browser/ui/views/extensions/disabled_extensions_view.h
@@ -31,7 +31,7 @@ class DisabledExtensionsView : public views::BubbleDelegateView,
// Show the Disabled Extension bubble, if needed. Returns true if the bubble
// was shown.
- static bool MaybeShow(Browser* browser, views::View* anchor_view);
+ static void MaybeShow(Browser* browser, views::View* anchor_view);
protected:
// views::BubbleDelegateView overrides:
@@ -40,6 +40,9 @@ class DisabledExtensionsView : public views::BubbleDelegateView,
private:
virtual ~DisabledExtensionsView();
+ // Makes sure the bubble is not shown again.
+ void DontShowBubbleAgain();
+
// views::ButtonListener implementation.
virtual void ButtonPressed(views::Button* sender,
const ui::Event& event) OVERRIDE;
diff --git a/chrome/browser/ui/views/toolbar_view.cc b/chrome/browser/ui/views/toolbar_view.cc
index e859b1a..5b56d2f 100644
--- a/chrome/browser/ui/views/toolbar_view.cc
+++ b/chrome/browser/ui/views/toolbar_view.cc
@@ -111,9 +111,6 @@ const int kSearchTopButtonSpacing = 3;
const int kSearchTopLocationBarSpacing = 2;
const int kSearchToolbarSpacing = 5;
-// How often to show the disabled extension (sideload wipeout) bubble.
-const int kShowSideloadWipeoutBubbleMax = 3;
-
gfx::ImageSkia* kPopupBackgroundEdge = NULL;
// The omnibox border has some additional shadow, so we use less vertical
@@ -217,6 +214,8 @@ ToolbarView::ToolbarView(Browser* browser)
}
ToolbarView::~ToolbarView() {
+ GetWidget()->RemoveObserver(this);
+
// NOTE: Don't remove the command observers here. This object gets destroyed
// after the Browser (which owns the CommandUpdater), so the CommandUpdater is
// already gone.
@@ -227,6 +226,8 @@ ToolbarView::~ToolbarView() {
void ToolbarView::Init(views::View* location_bar_parent,
views::View* popup_parent_view) {
+ GetWidget()->AddObserver(this);
+
back_ = new views::ButtonDropDown(this, new BackForwardMenuModel(
browser_, BackForwardMenuModel::BACKWARD_MENU));
back_->set_triggerable_event_flags(ui::EF_LEFT_MOUSE_BUTTON |
@@ -313,9 +314,6 @@ void ToolbarView::Init(views::View* location_bar_parent,
location_bar_->Init(popup_parent_view);
show_home_button_.Init(prefs::kShowHomeButton,
browser_->profile()->GetPrefs(), this);
- sideload_wipeout_bubble_shown_.Init(
- prefs::kExtensionsSideloadWipeoutBubbleShown,
- browser_->profile()->GetPrefs(), NULL);
browser_actions_->Init();
@@ -326,11 +324,6 @@ void ToolbarView::Init(views::View* location_bar_parent,
forward_->SetTooltipText(
l10n_util::GetStringUTF16(IDS_ACCNAME_TOOLTIP_FORWARD));
}
-
- int bubble_shown_count = sideload_wipeout_bubble_shown_.GetValue();
- if (bubble_shown_count < kShowSideloadWipeoutBubbleMax &&
- DisabledExtensionsView::MaybeShow(browser_, app_menu_))
- sideload_wipeout_bubble_shown_.SetValue(++bubble_shown_count);
}
void ToolbarView::Update(WebContents* tab, bool should_restore_state) {
@@ -593,6 +586,12 @@ void ToolbarView::ButtonPressed(views::Button* sender,
chrome::ExecuteCommandWithDisposition(browser_, command, disposition);
}
+void ToolbarView::OnWidgetVisibilityChanged(views::Widget* widget,
+ bool visible) {
+ if (visible)
+ DisabledExtensionsView::MaybeShow(browser_, app_menu_);
+}
+
////////////////////////////////////////////////////////////////////////////////
// ToolbarView, content::NotificationObserver implementation:
diff --git a/chrome/browser/ui/views/toolbar_view.h b/chrome/browser/ui/views/toolbar_view.h
index e2912c0..a934525 100644
--- a/chrome/browser/ui/views/toolbar_view.h
+++ b/chrome/browser/ui/views/toolbar_view.h
@@ -47,7 +47,8 @@ class ToolbarView : public views::AccessiblePaneView,
public chrome::search::SearchModelObserver,
public content::NotificationObserver,
public CommandObserver,
- public views::ButtonListener {
+ public views::ButtonListener,
+ public views::WidgetObserver {
public:
// The view class name.
static const char kViewClassName[];
@@ -136,6 +137,10 @@ class ToolbarView : public views::AccessiblePaneView,
virtual void ButtonPressed(views::Button* sender,
const ui::Event& event) OVERRIDE;
+ // Overridden from views::WidgetObserver:
+ virtual void OnWidgetVisibilityChanged(views::Widget* widget,
+ bool visible) OVERRIDE;
+
// Overridden from content::NotificationObserver:
virtual void Observe(int type,
const content::NotificationSource& source,
diff --git a/chrome/common/extensions/feature_switch.cc b/chrome/common/extensions/feature_switch.cc
index 11c6b6a..8086cc0 100644
--- a/chrome/common/extensions/feature_switch.cc
+++ b/chrome/common/extensions/feature_switch.cc
@@ -31,6 +31,7 @@ class CommonSwitches {
script_bubble(
switches::kScriptBubble,
FeatureSwitch::DEFAULT_DISABLED),
+ // TODO(finnur): When enabling this, only enable for OS_WIN.
sideload_wipeout(
switches::kSideloadWipeout,
FeatureSwitch::DEFAULT_DISABLED),