Commit 48844bd2 authored by Archana Simha's avatar Archana Simha Committed by Commit Bot

[Extensions Checkup] Shows the extensions page on startup.

Users in the experiment will be shown the extensions page
alongside the NTP upon startup once.

Design Doc: go/extensionscheckup

Bug: 1019296
Change-Id: Ia9fd02ece790643b320ee3f68d7544349967f6eb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1864828
Commit-Queue: Archana Simha <archanasimha@chromium.org>
Reviewed-by: default avatarDevlin <rdevlin.cronin@chromium.org>
Reviewed-by: default avatarTommy Martino <tmartino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#719431}
parent 55972f22
......@@ -67,9 +67,6 @@ bool ShouldShowExtensionsCheckupOnStartup(content::BrowserContext* context) {
extensions_features::kExtensionsCheckupToolEntryPointParameter) ==
"startup" &&
!prefs->HasUserSeenExtensionsCheckupOnStartup()) {
// Stores a boolean in ExtensionPrefs so we can make sure that the user is
// redirected to the extensions page upon startup once.
prefs->SetUserHasSeenExtensionsCheckupOnStartup(true);
return true;
}
return false;
......
......@@ -22,6 +22,7 @@
#include "chrome/browser/browser_process.h"
#include "chrome/browser/chrome_notification_types.h"
#include "chrome/browser/extensions/extension_browsertest.h"
#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/extensions/launch_util.h"
#include "chrome/browser/first_run/first_run.h"
#include "chrome/browser/infobars/infobar_service.h"
......@@ -66,6 +67,7 @@
#include "content/public/test/test_navigation_observer.h"
#include "content/public/test/test_utils.h"
#include "extensions/browser/extension_registry.h"
#include "extensions/common/extension_features.h"
#include "net/test/embedded_test_server/embedded_test_server.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
......@@ -1020,6 +1022,104 @@ IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorTest,
#endif // !defined(OS_CHROMEOS)
class StartupBrowserCreatorExtensionsCheckupExperimentTest
: public extensions::ExtensionBrowserTest {
public:
// ExtensionsBrowserTest opens about::blank via the command line, and
// command-line tabs supersede all others, except pinned tabs.
StartupBrowserCreatorExtensionsCheckupExperimentTest() {
set_open_about_blank_on_browser_launch(false);
}
void SetUp() override {
// Enable the extensions checkup experiment.
scoped_feature_list_.InitAndEnableFeatureWithParameters(
extensions_features::kExtensionsCheckupTool,
{{extensions_features::kExtensionsCheckupToolEntryPointParameter,
"startup"}});
extensions::ExtensionBrowserTest::SetUp();
}
void AddExtension() {
// Adds a non policy-installed extension to the extension registry.
const Extension* extension =
LoadExtension(test_data_dir_.AppendASCII("good.crx"));
ASSERT_TRUE(extension);
constexpr char kGoodExtensionId[] = "ldnnhddmnhbkjipkidpdiheffobcpfmf";
extensions::ExtensionRegistry* registry =
extensions::ExtensionRegistry::Get(profile());
EXPECT_TRUE(registry->enabled_extensions().GetByID(kGoodExtensionId));
extensions::ExtensionPrefs* prefs =
extensions::ExtensionPrefs::Get(profile());
EXPECT_TRUE(prefs->GetInstalledExtensionInfo(kGoodExtensionId));
}
private:
base::test::ScopedFeatureList scoped_feature_list_;
DISALLOW_COPY_AND_ASSIGN(
StartupBrowserCreatorExtensionsCheckupExperimentTest);
};
// Test that when the extensions checkup experiment is enabled for the startup
// entry point and a user has extensions installed, the user is directed to the
// chrome://extensions page upon startup.
IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorExtensionsCheckupExperimentTest,
PRE_ExtensionsCheckup) {
AddExtension();
}
IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorExtensionsCheckupExperimentTest,
ExtensionsCheckup) {
// The new browser should have exactly two tabs (chrome://extensions page and
// the NTP).
TabStripModel* tab_strip = browser()->tab_strip_model();
content::WebContents* extensions_tab = tab_strip->GetWebContentsAt(0);
// Check that the tab showing the extensions page is the active tab.
EXPECT_EQ(extensions_tab, tab_strip->GetActiveWebContents());
// Check that both the extensions page and the ntp page are shown.
ASSERT_EQ(2, tab_strip->count());
EXPECT_EQ("chrome://extensions/?checkup=shown",
extensions_tab->GetLastCommittedURL());
EXPECT_EQ(chrome::kChromeUINewTabURL,
tab_strip->GetWebContentsAt(1)->GetLastCommittedURL());
// Once the user sees the extensions page upon startup, they should not see it
// again.
Browser* other_browser = CreateBrowser(browser()->profile());
// Make sure we are observing a new browser instance.
EXPECT_NE(other_browser, browser());
TabStripModel* other_tab_strip = other_browser->tab_strip_model();
ASSERT_EQ(1, other_tab_strip->count());
EXPECT_NE("chrome://extensions/?checkup=shown",
other_tab_strip->GetWebContentsAt(0)->GetLastCommittedURL());
}
// Test that when the extensions checkup experiment has been shown and the
// browser is started again, the user is not directed to the
// chrome://extensions page upon startup.
IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorExtensionsCheckupExperimentTest,
PRE_ExtensionsCheckupAlreadyShown) {
AddExtension();
extensions::ExtensionPrefs::Get(profile())
->SetUserHasSeenExtensionsCheckupOnStartup(true);
}
IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorExtensionsCheckupExperimentTest,
ExtensionsCheckupAlreadyShown) {
// The new browser should have exactly one tab (the NTP).
TabStripModel* tab_strip = browser()->tab_strip_model();
ASSERT_EQ(1, tab_strip->count());
EXPECT_EQ(chrome::kChromeUINewTabURL,
tab_strip->GetActiveWebContents()->GetLastCommittedURL());
}
// These tests are not applicable to Chrome OS as neither master_preferences nor
// the onboarding promos exist there.
#if !defined(OS_CHROMEOS)
......
......@@ -32,6 +32,7 @@
#include "chrome/browser/custom_handlers/protocol_handler_registry.h"
#include "chrome/browser/custom_handlers/protocol_handler_registry_factory.h"
#include "chrome/browser/defaults.h"
#include "chrome/browser/extensions/extension_checkup.h"
#include "chrome/browser/infobars/infobar_service.h"
#include "chrome/browser/obsolete_system/obsolete_system.h"
#include "chrome/browser/prefs/session_startup_pref.h"
......@@ -625,10 +626,14 @@ void StartupBrowserCreatorImpl::DetermineURLsAndLaunch(
welcome::IsEnabled(profile_) && welcome::HasModulesToShow(profile_);
#endif // !defined(OS_CHROMEOS)
bool serve_extensions_page =
extensions::ShouldShowExtensionsCheckupOnStartup(profile_);
StartupTabs tabs = DetermineStartupTabs(
StartupTabProviderImpl(), cmd_line_tabs, process_startup,
is_incognito_or_guest, is_post_crash_launch,
has_incompatible_applications, promotional_tabs_enabled, welcome_enabled);
has_incompatible_applications, promotional_tabs_enabled, welcome_enabled,
serve_extensions_page);
// Return immediately if we start an async restore, since the remainder of
// that process is self-contained.
......@@ -680,7 +685,8 @@ StartupTabs StartupBrowserCreatorImpl::DetermineStartupTabs(
bool is_post_crash_launch,
bool has_incompatible_applications,
bool promotional_tabs_enabled,
bool welcome_enabled) {
bool welcome_enabled,
bool serve_extensions_page) {
// Only the New Tab Page or command line URLs may be shown in incognito mode.
// A similar policy exists for crash recovery launches, to prevent getting the
// user stuck in a crash loop.
......@@ -741,10 +747,18 @@ StartupTabs StartupBrowserCreatorImpl::DetermineStartupTabs(
AppendTabs(prefs_tabs, &tabs);
// Potentially add the New Tab Page. Onboarding content is designed to
// replace (and eventually funnel the user to) the NTP. Likewise, URLs
// from preferences are explicitly meant to override showing the NTP.
if (onboarding_tabs.empty() && prefs_tabs.empty())
AppendTabs(provider.GetNewTabPageTabs(command_line_, profile_), &tabs);
// replace (and eventually funnel the user to) the NTP.
if (onboarding_tabs.empty()) {
// Potentially show the extensions page in addition to the NTP if the user
// is part of the extensions checkup experiment and they have not been
// redirected to the extensions page upon startup before.
AppendTabs(provider.GetExtensionCheckupTabs(serve_extensions_page),
&tabs);
// URLs from preferences are explicitly meant to override showing the NTP.
if (prefs_tabs.empty()) {
AppendTabs(provider.GetNewTabPageTabs(command_line_, profile_), &tabs);
}
}
}
// Maybe add any tabs which the user has previously pinned.
......
......@@ -87,6 +87,8 @@ class StartupBrowserCreatorImpl {
DetermineBrowserOpenBehavior_PostCrash);
FRIEND_TEST_ALL_PREFIXES(StartupBrowserCreatorImplTest,
DetermineBrowserOpenBehavior_NotStartup);
FRIEND_TEST_ALL_PREFIXES(StartupBrowserCreatorImplTest,
DetermineStartupTabs_ExtensionCheckupPage);
enum class WelcomeRunType {
NONE, // Do not inject the welcome page for this run.
......@@ -150,7 +152,8 @@ class StartupBrowserCreatorImpl {
bool is_post_crash_launch,
bool has_incompatible_applications,
bool promotional_tabs_enabled,
bool welcome_enabled);
bool welcome_enabled,
bool serve_extensions_page);
// Begins an asynchronous session restore if current state allows it (e.g.,
// this is not process startup) and SessionService indicates that one is
......
......@@ -22,6 +22,7 @@ constexpr uint32_t kPreferencesTabs = 1 << 4;
constexpr uint32_t kNewTabPageTabs = 1 << 5;
constexpr uint32_t kWelcomeBackTab = 1 << 6;
constexpr uint32_t kPostCrashTab = 1 << 7;
constexpr uint32_t kExtensionsCheckupTabs = 1 << 8;
class FakeStartupTabProvider : public StartupTabProvider {
public:
......@@ -92,6 +93,14 @@ class FakeStartupTabProvider : public StartupTabProvider {
return tabs;
}
StartupTabs GetExtensionCheckupTabs(
bool serve_extensions_page) const override {
StartupTabs tabs;
if (options_ & kExtensionsCheckupTabs)
tabs.emplace_back(GURL("https://extensions/"), false);
return tabs;
}
private:
const uint32_t options_;
};
......@@ -110,7 +119,7 @@ TEST(StartupBrowserCreatorImplTest, DetermineStartupTabs) {
chrome::startup::IS_FIRST_RUN);
StartupTabs output = impl.DetermineStartupTabs(
provider, StartupTabs(), true, false, false, false, true, true);
provider, StartupTabs(), true, false, false, false, true, true, false);
ASSERT_EQ(4U, output.size());
EXPECT_EQ("reset-trigger", output[0].url.host());
EXPECT_EQ("onboarding", output[1].url.host());
......@@ -119,7 +128,7 @@ TEST(StartupBrowserCreatorImplTest, DetermineStartupTabs) {
// No extra onboarding content for managed starts.
output = impl.DetermineStartupTabs(provider, StartupTabs(), true, false,
false, false, false, true);
false, false, false, true, false);
ASSERT_EQ(3U, output.size());
EXPECT_EQ("reset-trigger", output[0].url.host());
EXPECT_EQ("prefs", output[1].url.host());
......@@ -127,7 +136,7 @@ TEST(StartupBrowserCreatorImplTest, DetermineStartupTabs) {
// No onboarding if not enabled even if promo is allowed.
output = impl.DetermineStartupTabs(provider, StartupTabs(), true, false,
false, false, true, false);
false, false, true, false, false);
ASSERT_EQ(3U, output.size());
EXPECT_EQ("reset-trigger", output[0].url.host());
EXPECT_EQ("prefs", output[1].url.host());
......@@ -145,7 +154,7 @@ TEST(StartupBrowserCreatorImplTest, DetermineStartupTabs_Incognito) {
chrome::startup::IS_FIRST_RUN);
StartupTabs output = impl.DetermineStartupTabs(
provider, StartupTabs(), true, true, false, false, true, true);
provider, StartupTabs(), true, true, false, false, true, true, false);
ASSERT_EQ(1U, output.size());
// Check for the actual NTP URL, rather than the sentinel returned by the
// fake, because the Provider is ignored entirely when short-circuited by
......@@ -165,7 +174,7 @@ TEST(StartupBrowserCreatorImplTest, DetermineStartupTabs_Crash) {
// Regular Crash Recovery case:
StartupTabs output = impl.DetermineStartupTabs(
provider, StartupTabs(), true, false, true, false, true, true);
provider, StartupTabs(), true, false, true, false, true, true, false);
ASSERT_EQ(1U, output.size());
// Check for the actual NTP URL, rather than the sentinel returned by the
// fake, because the Provider is ignored entirely when short-circuited by
......@@ -174,7 +183,7 @@ TEST(StartupBrowserCreatorImplTest, DetermineStartupTabs_Crash) {
// Crash Recovery case with problem applications:
output = impl.DetermineStartupTabs(provider, StartupTabs(), true, false, true,
true, true, true);
true, true, true, false);
ASSERT_EQ(1U, output.size());
EXPECT_EQ(GURL("https://incompatible-applications"), output[0].url);
}
......@@ -190,7 +199,7 @@ TEST(StartupBrowserCreatorImplTest, DetermineStartupTabs_MasterPrefs) {
chrome::startup::IS_FIRST_RUN);
StartupTabs output = impl.DetermineStartupTabs(
provider, StartupTabs(), true, false, false, false, true, true);
provider, StartupTabs(), true, false, false, false, true, true, false);
ASSERT_EQ(1U, output.size());
EXPECT_EQ("distribution", output[0].url.host());
}
......@@ -208,7 +217,7 @@ TEST(StartupBrowserCreatorImplTest, DetermineStartupTabs_CommandLine) {
StartupTabs cmd_line_tabs = {StartupTab(GURL("https://cmd-line"), false)};
StartupTabs output = impl.DetermineStartupTabs(
provider, cmd_line_tabs, true, false, false, false, true, true);
provider, cmd_line_tabs, true, false, false, false, true, true, false);
ASSERT_EQ(3U, output.size());
EXPECT_EQ("reset-trigger", output[0].url.host());
EXPECT_EQ("cmd-line", output[1].url.host());
......@@ -219,19 +228,19 @@ TEST(StartupBrowserCreatorImplTest, DetermineStartupTabs_CommandLine) {
// Incognito
output = impl.DetermineStartupTabs(provider, cmd_line_tabs, true, true, false,
false, true, true);
false, true, true, false);
ASSERT_EQ(1U, output.size());
EXPECT_EQ("cmd-line", output[0].url.host());
// Crash Recovery
output = impl.DetermineStartupTabs(provider, cmd_line_tabs, true, false, true,
false, true, true);
false, true, true, false);
ASSERT_EQ(1U, output.size());
EXPECT_EQ("cmd-line", output[0].url.host());
// Crash Recovery with incompatible applications.
output = impl.DetermineStartupTabs(provider, cmd_line_tabs, true, false, true,
true, true, true);
true, true, true, false);
ASSERT_EQ(1U, output.size());
EXPECT_EQ("cmd-line", output[0].url.host());
}
......@@ -247,13 +256,28 @@ TEST(StartupBrowserCreatorImplTest, DetermineStartupTabs_NewTabPage) {
StartupTabs output =
impl.DetermineStartupTabs(provider_allows_ntp, StartupTabs(), true, false,
false, false, true, true);
false, false, true, true, false);
ASSERT_EQ(3U, output.size());
EXPECT_EQ("reset-trigger", output[0].url.host());
EXPECT_EQ("new-tab", output[1].url.host());
EXPECT_EQ("pinned", output[2].url.host());
}
// If the user's preferences satisfy the conditions, show the extensions page
// upon startup.
TEST(StartupBrowserCreatorImplTest, DetermineStartupTabs_ExtensionCheckupPage) {
FakeStartupTabProvider provider(kNewTabPageTabs | kExtensionsCheckupTabs);
Creator impl(base::FilePath(),
base::CommandLine(base::CommandLine::NO_PROGRAM),
chrome::startup::IS_FIRST_RUN);
StartupTabs output = impl.DetermineStartupTabs(
provider, StartupTabs(), true, false, false, false, true, true, true);
ASSERT_EQ(2U, output.size());
EXPECT_EQ("extensions", output[0].url.host());
EXPECT_EQ("new-tab", output[1].url.host());
}
// The welcome back page should appear before any other session restore tabs.
TEST(StartupBrowserCreatorImplTest, DetermineStartupTabs_WelcomeBackPage) {
FakeStartupTabProvider provider_allows_ntp(kPinnedTabs | kPreferencesTabs |
......@@ -264,7 +288,7 @@ TEST(StartupBrowserCreatorImplTest, DetermineStartupTabs_WelcomeBackPage) {
StartupTabs output =
impl.DetermineStartupTabs(provider_allows_ntp, StartupTabs(), true, false,
false, false, true, true);
false, false, true, true, false);
ASSERT_EQ(3U, output.size());
EXPECT_EQ("welcome-back", output[0].url.host());
EXPECT_EQ("prefs", output[1].url.host());
......@@ -272,14 +296,14 @@ TEST(StartupBrowserCreatorImplTest, DetermineStartupTabs_WelcomeBackPage) {
// No welcome back for non-startup opens.
output = impl.DetermineStartupTabs(provider_allows_ntp, StartupTabs(), false,
false, false, false, true, true);
false, false, false, true, true, false);
ASSERT_EQ(2U, output.size());
EXPECT_EQ("prefs", output[0].url.host());
EXPECT_EQ("pinned", output[1].url.host());
// No welcome back for managed starts even if first run.
output = impl.DetermineStartupTabs(provider_allows_ntp, StartupTabs(), true,
false, false, false, false, true);
false, false, false, false, true, false);
ASSERT_EQ(2U, output.size());
EXPECT_EQ("prefs", output[0].url.host());
EXPECT_EQ("pinned", output[1].url.host());
......
......@@ -131,6 +131,11 @@ StartupTabs StartupTabProviderImpl::GetPostCrashTabs(
return GetPostCrashTabsForState(has_incompatible_applications);
}
StartupTabs StartupTabProviderImpl::GetExtensionCheckupTabs(
bool serve_extensions_page) const {
return GetExtensionCheckupTabsForState(serve_extensions_page);
}
// static
bool StartupTabProviderImpl::CanShowWelcome(bool is_signin_allowed,
bool is_supervised_user,
......@@ -232,6 +237,19 @@ StartupTabs StartupTabProviderImpl::GetPostCrashTabsForState(
return tabs;
}
// static
StartupTabs StartupTabProviderImpl::GetExtensionCheckupTabsForState(
bool serve_extensions_page) {
StartupTabs tabs;
if (serve_extensions_page) {
tabs.emplace_back(
net::AppendQueryParameter(GURL(chrome::kChromeUIExtensionsURL),
"checkup", "shown"),
false);
}
return tabs;
}
// static
GURL StartupTabProviderImpl::GetWelcomePageUrl(bool use_later_run_variant) {
GURL url(chrome::kChromeUIWelcomeURL);
......
......@@ -60,6 +60,10 @@ class StartupTabProvider {
// applications exist.
virtual StartupTabs GetPostCrashTabs(
bool has_incompatible_applications) const = 0;
// Returns tabs related to the extension checkup promo (if applicable).
virtual StartupTabs GetExtensionCheckupTabs(
bool serve_extensions_page) const = 0;
};
class StartupTabProviderImpl : public StartupTabProvider {
......@@ -129,6 +133,10 @@ class StartupTabProviderImpl : public StartupTabProvider {
static StartupTabs GetPostCrashTabsForState(
bool has_incompatible_applications);
// Determines if the extensions page should be shown.
static StartupTabs GetExtensionCheckupTabsForState(
bool serve_extensions_page);
// Gets the URL for the Welcome page. If |use_later_run_variant| is true, a
// URL parameter will be appended so as to access the variant page used when
// onboarding occurs after the first Chrome execution (e.g., when creating an
......@@ -160,6 +168,8 @@ class StartupTabProviderImpl : public StartupTabProvider {
Profile* profile) const override;
StartupTabs GetPostCrashTabs(
bool has_incompatible_applications) const override;
StartupTabs GetExtensionCheckupTabs(
bool serve_extensions_page) const override;
private:
DISALLOW_COPY_AND_ASSIGN(StartupTabProviderImpl);
......
......@@ -245,3 +245,13 @@ TEST(StartupTabProviderTest, IncognitoProfile) {
StartupTabs output = StartupTabProviderImpl().GetOnboardingTabs(incognito);
EXPECT_TRUE(output.empty());
}
TEST(StartupTabProviderTest, GetNewTabPageTabsForState_ExtensionsCheckup) {
SessionStartupPref pref_default(SessionStartupPref::Type::DEFAULT);
StartupTabs output = StartupTabProviderImpl::GetExtensionCheckupTabsForState(
/*serve_extensions_page=*/true);
ASSERT_EQ(1U, output.size());
EXPECT_EQ("chrome://extensions/?checkup=shown", output[0].url);
}
......@@ -31,8 +31,10 @@
#include "components/pref_registry/pref_registry_syncable.h"
#include "components/prefs/pref_service.h"
#include "components/strings/grit/components_strings.h"
#include "content/public/browser/web_contents.h"
#include "content/public/browser/web_ui.h"
#include "content/public/browser/web_ui_data_source.h"
#include "extensions/browser/extension_prefs.h"
#include "extensions/common/extension_urls.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/base/resource/resource_bundle.h"
......@@ -339,6 +341,17 @@ ExtensionsUI::ExtensionsUI(content::WebUI* web_ui)
source->OverrideContentSecurityPolicyObjectSrc("object-src 'self';");
content::WebUIDataSource::Add(profile, source);
// Stores a boolean in ExtensionPrefs so we can make sure that the user is
// redirected to the extensions page upon startup once. We're using
// GetVisibleURL() because the load hasn't committed and this check isn't used
// for a security decision, however a stronger check will be implemented if we
// decide to invest more in this experiment.
if (web_ui->GetWebContents()->GetVisibleURL().query_piece().starts_with(
"checkup")) {
ExtensionPrefs::Get(profile)->SetUserHasSeenExtensionsCheckupOnStartup(
true);
}
}
ExtensionsUI::~ExtensionsUI() {}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment