From 3354b1edcc1cb9ef2d307d4afd2e05b5d2c56d2e Mon Sep 17 00:00:00 2001 From: benwells Date: Thu, 11 Dec 2014 17:23:01 -0800 Subject: [PATCH] Enable the new bookmark apps system by default. This change enables the feature previously known as streamlined hosted apps by default. BUG=440670 Review URL: https://codereview.chromium.org/772533005 Cr-Commit-Position: refs/heads/master@{#308017} --- chrome/app/generated_resources.grd | 8 +++--- chrome/browser/about_flags.cc | 8 +++--- chrome/browser/chrome_content_browser_client.cc | 2 +- .../api/management/management_apitest.cc | 18 ------------- chrome/browser/extensions/extension_util.cc | 4 +-- chrome/browser/extensions/extension_util.h | 6 +++++ chrome/browser/ui/browser_browsertest.cc | 31 +++++++--------------- .../startup/startup_browser_creator_browsertest.cc | 12 +++------ .../frame/web_app_left_header_view_ash_unittest.cc | 3 --- chrome/common/chrome_switches.cc | 6 ++--- chrome/common/chrome_switches.h | 2 +- chrome/renderer/web_apps.cc | 4 +-- .../api_test/management/test/launchType.js | 6 ----- tools/metrics/histograms/histograms.xml | 1 + 14 files changed, 38 insertions(+), 73 deletions(-) diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd index f2621cda7b6e..05f6a1b6796d 100644 --- a/chrome/app/generated_resources.grd +++ b/chrome/app/generated_resources.grd @@ -6592,11 +6592,11 @@ Keep your key file in a safe place. You will need it to create new versions of y Do not use Smart Lock on sign-in screen, which allows you to sign in to your Chromebook when in proximity to your phone. - - Enable experimental streamlined hosted apps. + + Disable the new bookmark app system. - - Enables an experimental, more streamlined hosted app experience. + + Disables the new system for creating bookmark apps. Enable experimental ephemeral apps from the webstore. diff --git a/chrome/browser/about_flags.cc b/chrome/browser/about_flags.cc index c599dffacbbb..1de6dc708752 100644 --- a/chrome/browser/about_flags.cc +++ b/chrome/browser/about_flags.cc @@ -1520,11 +1520,11 @@ const Experiment kExperiments[] = { }, #endif { - "enable-streamlined-hosted-apps", - IDS_FLAGS_ENABLE_STREAMLINED_HOSTED_APPS_NAME, - IDS_FLAGS_ENABLE_STREAMLINED_HOSTED_APPS_DESCRIPTION, + "disable-new-bookmark-apps", + IDS_FLAGS_DISABLE_NEW_BOOKMARK_APPS_NAME, + IDS_FLAGS_DISABLE_NEW_BOOKMARK_APPS_DESCRIPTION, kOsWin | kOsCrOS | kOsLinux | kOsMac, - SINGLE_VALUE_TYPE(switches::kEnableStreamlinedHostedApps) + SINGLE_VALUE_TYPE(switches::kDisableNewBookmarkApps) }, { "enable-ephemeral-apps-in-webstore", diff --git a/chrome/browser/chrome_content_browser_client.cc b/chrome/browser/chrome_content_browser_client.cc index ae87f9384723..0b5dd15042e6 100644 --- a/chrome/browser/chrome_content_browser_client.cc +++ b/chrome/browser/chrome_content_browser_client.cc @@ -1391,6 +1391,7 @@ void ChromeContentBrowserClient::AppendExtraCommandLineSwitches( switches::kDisableBundledPpapiFlash, switches::kDisableCastStreamingHWEncoding, switches::kDisableJavaScriptHarmonyShipping, + switches::kDisableNewBookmarkApps, switches::kDisableOutOfProcessPdf, switches::kEnableBenchmarking, switches::kEnableNaCl, @@ -1402,7 +1403,6 @@ void ChromeContentBrowserClient::AppendExtraCommandLineSwitches( switches::kEnableOutOfProcessPdf, switches::kEnablePluginPlaceholderShadowDom, switches::kEnableShowModalDialog, - switches::kEnableStreamlinedHostedApps, switches::kEnableWebBasedSignin, switches::kJavaScriptHarmony, switches::kMessageLoopHistogrammer, diff --git a/chrome/browser/extensions/api/management/management_apitest.cc b/chrome/browser/extensions/api/management/management_apitest.cc index 192ca250105e..7820666cc1df 100644 --- a/chrome/browser/extensions/api/management/management_apitest.cc +++ b/chrome/browser/extensions/api/management/management_apitest.cc @@ -323,21 +323,3 @@ IN_PROC_BROWSER_TEST_F(ExtensionManagementApiTest, LaunchType) { ASSERT_TRUE(RunExtensionSubtest("management/test", "launchType.html")); } - -class ExtensionManagementApiStreamlinedAppsTest - : public ExtensionManagementApiTest { - public: - void SetUpCommandLine(CommandLine* command_line) override { - ExtensionManagementApiTest::SetUpCommandLine(command_line); - command_line->AppendSwitch(switches::kEnableStreamlinedHostedApps); - } -}; - -IN_PROC_BROWSER_TEST_F(ExtensionManagementApiStreamlinedAppsTest, LaunchType) { - LoadExtensions(); - base::FilePath basedir = test_data_dir_.AppendASCII("management"); - LoadNamedExtension(basedir, "packaged_app"); - - ASSERT_TRUE(RunExtensionSubtest("management/test", - "launchType.html?streamlined-hosted-apps")); -} diff --git a/chrome/browser/extensions/extension_util.cc b/chrome/browser/extensions/extension_util.cc index f6ebc4994452..bd8f4b24a06b 100644 --- a/chrome/browser/extensions/extension_util.cc +++ b/chrome/browser/extensions/extension_util.cc @@ -360,8 +360,8 @@ const gfx::ImageSkia& GetDefaultExtensionIcon() { } bool IsStreamlinedHostedAppsEnabled() { - return CommandLine::ForCurrentProcess()->HasSwitch( - switches::kEnableStreamlinedHostedApps); + return !CommandLine::ForCurrentProcess()->HasSwitch( + switches::kDisableNewBookmarkApps); } } // namespace util diff --git a/chrome/browser/extensions/extension_util.h b/chrome/browser/extensions/extension_util.h index 18d34084a9ed..a05e07b2fd78 100644 --- a/chrome/browser/extensions/extension_util.h +++ b/chrome/browser/extensions/extension_util.h @@ -124,6 +124,12 @@ const gfx::ImageSkia& GetDefaultExtensionIcon(); const gfx::ImageSkia& GetDefaultAppIcon(); // Returns true if the experimental streamlined hosted apps feature is enabled. +// +// TODO(benwells): http://crbug.com/441127: Rename this to +// IsNewBookmarkAppsEnabled. +// +// TODO(benwells): http://crbug.com/441128: Remove this entirely once the +// feature is stable. bool IsStreamlinedHostedAppsEnabled(); } // namespace util diff --git a/chrome/browser/ui/browser_browsertest.cc b/chrome/browser/ui/browser_browsertest.cc index bf529cafe4e2..8edda6534669 100644 --- a/chrome/browser/ui/browser_browsertest.cc +++ b/chrome/browser/ui/browser_browsertest.cc @@ -1296,10 +1296,13 @@ IN_PROC_BROWSER_TEST_F(BrowserTest, MAYBE_TabClosingWhenRemovingExtension) { } #if !defined(OS_MACOSX) -// Open with --app-id=, and see that an app window opens. +// Open with --app-id=, and see that an application tab opens by default. IN_PROC_BROWSER_TEST_F(BrowserTest, AppIdSwitch) { ASSERT_TRUE(test_server()->Start()); + // There should be one tab to start with. + ASSERT_EQ(1, browser()->tab_strip_model()->count()); + // Load an app. host_resolver()->AddRule("www.example.com", "127.0.0.1"); ASSERT_TRUE(LoadExtension(test_data_dir_.AppendASCII("app/"))); @@ -1311,26 +1314,15 @@ IN_PROC_BROWSER_TEST_F(BrowserTest, AppIdSwitch) { chrome::startup::IsFirstRun first_run = first_run::IsChromeFirstRun() ? chrome::startup::IS_FIRST_RUN : chrome::startup::IS_NOT_FIRST_RUN; StartupBrowserCreatorImpl launch(base::FilePath(), command_line, first_run); - ASSERT_TRUE(launch.OpenApplicationWindow(browser()->profile(), NULL)); + EXPECT_FALSE(launch.OpenApplicationWindow(browser()->profile(), NULL)); + EXPECT_TRUE(launch.OpenApplicationTab(browser()->profile())); - // Check that the new browser has an app name. - // The launch should have created a new browser. - ASSERT_EQ(2u, chrome::GetBrowserCount(browser()->profile(), + // Check that a new browser wasn't opened. + EXPECT_EQ(1u, chrome::GetBrowserCount(browser()->profile(), browser()->host_desktop_type())); - // Find the new browser. - Browser* new_browser = NULL; - for (chrome::BrowserIterator it; !it.done() && !new_browser; it.Next()) { - if (*it != browser()) - new_browser = *it; - } - ASSERT_TRUE(new_browser); - ASSERT_TRUE(new_browser != browser()); - - // The browser's app_name should include the app's ID. - ASSERT_NE( - new_browser->app_name_.find(extension_app->id()), - std::string::npos) << new_browser->app_name_; + // Check that a new tab was opened. + EXPECT_EQ(2, browser()->tab_strip_model()->count()); } // Open an app window and the dev tools window and ensure that the location @@ -1386,9 +1378,6 @@ IN_PROC_BROWSER_TEST_F(BrowserTest, ShouldShowLocationBar) { // window and a dev tools window, and check that the web app frame feature is // supported correctly. IN_PROC_BROWSER_TEST_F(BrowserTest, ShouldUseWebAppFrame) { - base::CommandLine::ForCurrentProcess()->AppendSwitch( - switches::kEnableStreamlinedHostedApps); - ASSERT_TRUE(test_server()->Start()); // Load a hosted app. diff --git a/chrome/browser/ui/startup/startup_browser_creator_browsertest.cc b/chrome/browser/ui/startup/startup_browser_creator_browsertest.cc index a2a94552c828..fdc0bbb1f15d 100644 --- a/chrome/browser/ui/startup/startup_browser_creator_browsertest.cc +++ b/chrome/browser/ui/startup/startup_browser_creator_browsertest.cc @@ -320,18 +320,14 @@ IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorTest, OpenAppShortcutNoPref) { ASSERT_TRUE(launch.Launch(browser()->profile(), std::vector(), false, browser()->host_desktop_type())); - // No pref was set, so the app should have opened in a window. + // No pref was set, so the app should have opened in a tab in a new window. // The launch should have created a new browser. Browser* new_browser = FindOneOtherBrowser(browser()); ASSERT_TRUE(new_browser); - // Expect an app window. - EXPECT_TRUE(new_browser->is_app()); - - // The browser's app_name should include the app's ID. - EXPECT_NE( - new_browser->app_name_.find(extension_app->id()), - std::string::npos) << new_browser->app_name_; + // It should be a standard tabbed window, not an app window. + EXPECT_FALSE(new_browser->is_app()); + EXPECT_TRUE(new_browser->is_type_tabbed()); } IN_PROC_BROWSER_TEST_F(StartupBrowserCreatorTest, OpenAppShortcutWindowPref) { diff --git a/chrome/browser/ui/views/frame/web_app_left_header_view_ash_unittest.cc b/chrome/browser/ui/views/frame/web_app_left_header_view_ash_unittest.cc index 035400288479..1c59dfab87b0 100644 --- a/chrome/browser/ui/views/frame/web_app_left_header_view_ash_unittest.cc +++ b/chrome/browser/ui/views/frame/web_app_left_header_view_ash_unittest.cc @@ -32,9 +32,6 @@ class WebAppLeftHeaderViewTest : public TestWithBrowserView { // TestWithBrowserView override: void SetUp() override { - CommandLine::ForCurrentProcess()->AppendSwitch( - switches::kEnableStreamlinedHostedApps); - TestWithBrowserView::SetUp(); // Setup a fake toolbar to enable testing. diff --git a/chrome/common/chrome_switches.cc b/chrome/common/chrome_switches.cc index 490727078691..be1cdc9940f3 100644 --- a/chrome/common/chrome_switches.cc +++ b/chrome/common/chrome_switches.cc @@ -280,6 +280,9 @@ const char kDisableIPv6[] = "disable-ipv6"; const char kDisableMinimizeOnSecondLauncherItemClick[] = "disable-minimize-on-second-launcher-item-click"; +// Disables the new bookmark app system. +const char kDisableNewBookmarkApps[] = "disable-new-bookmark-apps"; + // Disables the menu on the NTP for accessing sessions from other devices. const char kDisableNTPOtherSessionsMenu[] = "disable-ntp-other-sessions-menu"; @@ -597,9 +600,6 @@ const char kEnableSSLConnectJobWaiting[] = "enable-ssl-connect-job-waiting"; // proceeds in the background. const char kEnableStaleWhileRevalidate[] = "enable-stale-while-revalidate"; -// Enables an experimental hosted app experience. -const char kEnableStreamlinedHostedApps[] = "enable-streamlined-hosted-apps"; - // Enables the suggestions service. const char kEnableSuggestionsService[] = "enable-suggestions-service"; diff --git a/chrome/common/chrome_switches.h b/chrome/common/chrome_switches.h index ed7773aa56ca..e256704e762d 100644 --- a/chrome/common/chrome_switches.h +++ b/chrome/common/chrome_switches.h @@ -84,6 +84,7 @@ extern const char kDisableExtensions[]; extern const char kDisableIPv6[]; extern const char kDisableJavaScriptHarmonyShipping[]; extern const char kDisableMinimizeOnSecondLauncherItemClick[]; +extern const char kDisableNewBookmarkApps[]; extern const char kDisableNTPOtherSessionsMenu[]; extern const char kDisableOfflineAutoReload[]; extern const char kDisableOfflineAutoReloadVisibleOnly[]; @@ -173,7 +174,6 @@ extern const char kDisableSettingsWindow[]; extern const char kEnableSpdy4[]; extern const char kEnableSSLConnectJobWaiting[]; extern const char kEnableStaleWhileRevalidate[]; -extern const char kEnableStreamlinedHostedApps[]; extern const char kEnableSuggestionsService[]; extern const char kEnableSupervisedUserBlacklist[]; extern const char kEnableSupervisedUserSafeSites[]; diff --git a/chrome/renderer/web_apps.cc b/chrome/renderer/web_apps.cc index 039ee635633f..636d407e5499 100644 --- a/chrome/renderer/web_apps.cc +++ b/chrome/renderer/web_apps.cc @@ -147,8 +147,8 @@ void ParseWebAppFromWebDocument(WebFrame* frame, // "apple-touch-icon-precomposed". if (LowerCaseEqualsASCII(rel, "icon") || LowerCaseEqualsASCII(rel, "shortcut icon") || - (CommandLine::ForCurrentProcess()-> - HasSwitch(switches::kEnableStreamlinedHostedApps) && + (!CommandLine::ForCurrentProcess()-> + HasSwitch(switches::kDisableNewBookmarkApps) && (LowerCaseEqualsASCII(rel, "apple-touch-icon") || LowerCaseEqualsASCII(rel, "apple-touch-icon-precomposed")))) { AddInstallIcon(elem, &app_info->icons); diff --git a/chrome/test/data/extensions/api_test/management/test/launchType.js b/chrome/test/data/extensions/api_test/management/test/launchType.js index 493139207bc1..b2d40f3ce6a1 100644 --- a/chrome/test/data/extensions/api_test/management/test/launchType.js +++ b/chrome/test/data/extensions/api_test/management/test/launchType.js @@ -3,8 +3,6 @@ // found in the LICENSE file. var enabled_app, disabled_app, enabled_extension, packaged_app; -var streamlinedHostedAppsEnabled = - (location.href.indexOf("streamlined-hosted-apps") != -1); var allLaunchTypes = ["OPEN_AS_REGULAR_TAB", "OPEN_AS_PINNED_TAB", "OPEN_AS_WINDOW", @@ -34,10 +32,6 @@ function getAvailableLaunchTypes(app) { types.push("OPEN_AS_REGULAR_TAB"); types.push("OPEN_AS_WINDOW"); - if (!streamlinedHostedAppsEnabled) { - types.push("OPEN_AS_PINNED_TAB"); - types.push("OPEN_FULL_SCREEN"); - } return types; } diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index 6a623e7218a1..e40fd78cf5cd 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml @@ -49496,6 +49496,7 @@ To add a new entry, add it with any value and run test to compute valid value. + -- 2.11.4.GIT