Commit 4e501a0a authored by Callistus's avatar Callistus Committed by Commit Bot

Implemented logic to launch Help App's Release Notes instead of the PWA.

In particular, this affects the "See what's new" button on the Settings
About page and the Release Notes notification.

- Also added Notification as an App Launch Source.

Bug: b/159394556
Change-Id: Icc498f4c571098c922a8da29003fc8780d386ec0
Cq-Include-Trybots: chrome/try:linux-chromeos-chrome
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2352459
Commit-Queue: Callistus Tan <callistus@google.com>
Reviewed-by: default avatarRachel Carpenter <carpenterr@chromium.org>
Reviewed-by: default avatarTrent Apted <tapted@chromium.org>
Reviewed-by: default avatarKyle Horimoto <khorimoto@chromium.org>
Reviewed-by: default avatarDominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799521}
parent 388058c7
......@@ -132,6 +132,11 @@ void RecordDefaultAppLaunch(DefaultAppName default_app_name,
base::UmaHistogramEnumeration("Apps.DefaultAppLaunch.FromSharesheet",
default_app_name);
break;
case apps::mojom::LaunchSource::kFromReleaseNotesNotification:
base::UmaHistogramEnumeration(
"Apps.DefaultAppLaunch.FromReleaseNotesNotification",
default_app_name);
break;
}
}
......@@ -164,6 +169,7 @@ void RecordBuiltInAppLaunch(apps::BuiltInAppName built_in_app_name,
case apps::mojom::LaunchSource::kFromTest:
case apps::mojom::LaunchSource::kFromArc:
case apps::mojom::LaunchSource::kFromSharesheet:
case apps::mojom::LaunchSource::kFromReleaseNotesNotification:
break;
}
}
......
......@@ -148,7 +148,7 @@ void BuiltInChromeOsApps::Launch(const std::string& app_id,
} else if (app_id == ash::kReleaseNotesAppId) {
base::RecordAction(
base::UserMetricsAction("ReleaseNotes.SuggestionChipLaunched"));
chrome::LaunchReleaseNotes(profile_);
chrome::LaunchReleaseNotes(profile_, launch_source);
}
}
......
......@@ -133,6 +133,7 @@ ash::ShelfLaunchSource ConvertLaunchSource(
case apps::mojom::LaunchSource::kFromTest:
case apps::mojom::LaunchSource::kFromArc:
case apps::mojom::LaunchSource::kFromSharesheet:
case apps::mojom::LaunchSource::kFromReleaseNotesNotification:
return ash::LAUNCH_FROM_UNKNOWN;
}
}
......@@ -530,6 +531,7 @@ void ExtensionAppsBase::Launch(const std::string& app_id,
case apps::mojom::LaunchSource::kFromTest:
case apps::mojom::LaunchSource::kFromArc:
case apps::mojom::LaunchSource::kFromSharesheet:
case apps::mojom::LaunchSource::kFromReleaseNotesNotification:
break;
}
......
......@@ -201,6 +201,7 @@ apps::mojom::AppLaunchSource GetAppLaunchSource(
case apps::mojom::LaunchSource::kFromFileManager:
return apps::mojom::AppLaunchSource::kSourceFileHandler;
case apps::mojom::LaunchSource::kFromChromeInternal:
case apps::mojom::LaunchSource::kFromReleaseNotesNotification:
return apps::mojom::AppLaunchSource::kSourceChromeInternal;
case apps::mojom::LaunchSource::kFromInstalledNotification:
return apps::mojom::AppLaunchSource::kSourceInstalledNotification;
......
......@@ -257,6 +257,7 @@ void WebAppsBase::Launch(const std::string& app_id,
case apps::mojom::LaunchSource::kFromTest:
case apps::mojom::LaunchSource::kFromArc:
case apps::mojom::LaunchSource::kFromSharesheet:
case apps::mojom::LaunchSource::kFromReleaseNotesNotification:
break;
}
......
......@@ -48,7 +48,8 @@ void ReleaseNotesNotification::HandleClickShowNotification() {
SystemNotificationHelper::GetInstance()->Close(kShowNotificationID);
base::RecordAction(
base::UserMetricsAction("ReleaseNotes.LaunchedNotification"));
chrome::LaunchReleaseNotes(profile_);
chrome::LaunchReleaseNotes(
profile_, apps::mojom::LaunchSource::kFromReleaseNotesNotification);
}
void ReleaseNotesNotification::ShowReleaseNotesNotification() {
......
......@@ -7,6 +7,8 @@
#include "base/test/metrics/histogram_tester.h"
#include "base/test/metrics/user_action_tester.h"
#include "build/branding_buildflags.h"
#include "build/build_config.h"
#include "chrome/browser/apps/app_service/app_launch_params.h"
#include "chrome/browser/apps/app_service/app_service_proxy.h"
#include "chrome/browser/apps/app_service/app_service_proxy_factory.h"
......@@ -19,6 +21,7 @@
#include "chrome/browser/web_applications/system_web_app_manager_browsertest.h"
#include "chromeos/components/help_app_ui/url_constants.h"
#include "chromeos/components/web_applications/test/sandboxed_web_ui_test_base.h"
#include "chromeos/constants/chromeos_features.h"
#include "content/public/test/browser_test.h"
#include "content/public/test/test_navigation_observer.h"
#include "testing/gtest/include/gtest/gtest.h"
......@@ -27,7 +30,16 @@
#include "ui/gfx/geometry/rect.h"
#include "ui/gfx/geometry/size.h"
using HelpAppIntegrationTest = SystemWebAppIntegrationTest;
class HelpAppIntegrationTest : public SystemWebAppIntegrationTest {
public:
HelpAppIntegrationTest() {
scoped_feature_list_.InitWithFeatures(
{chromeos::features::kHelpAppReleaseNotes}, {});
}
private:
base::test::ScopedFeatureList scoped_feature_list_;
};
// Test that the Help App installs and launches correctly. Runs some spot
// checks on the manifest.
......@@ -110,6 +122,68 @@ IN_PROC_BROWSER_TEST_P(HelpAppIntegrationTest, HelpAppV2Incognito) {
chrome::ShowHelp(incognito_browser, chrome::HELP_SOURCE_KEYBOARD));
}
// Test that launching the Help App's release notes opens the app on the Release
// Notes page.
IN_PROC_BROWSER_TEST_P(HelpAppIntegrationTest, HelpAppV2LaunchReleaseNotes) {
WaitForTestSystemAppInstall();
// There should be 1 browser window initially.
EXPECT_EQ(1u, chrome::GetTotalBrowserCount());
const GURL expected_url("chrome://help-app/updates");
content::TestNavigationObserver navigation_observer(expected_url);
navigation_observer.StartWatchingNewWebContents();
chrome::LaunchReleaseNotes(profile(),
apps::mojom::LaunchSource::kFromOtherApp);
#if defined(OS_CHROMEOS) && BUILDFLAG(GOOGLE_CHROME_BRANDING)
// If no navigation happens, then this test will time out due to the wait.
navigation_observer.Wait();
// There should be two browser windows, one regular and one for the newly
// opened app.
EXPECT_EQ(2u, chrome::GetTotalBrowserCount());
// The opened window should be showing the url with attached WebUI.
content::WebContents* web_contents =
chrome::FindLastActive()->tab_strip_model()->GetActiveWebContents();
// The inner frame should be the pathname for the release notes pathname.
EXPECT_EQ("chrome-untrusted://help-app/updates",
SandboxedWebUiAppTestBase::EvalJsInAppFrame(
web_contents, "window.location.href"));
#else
// Nothing should happen on non-branded builds.
EXPECT_EQ(1u, chrome::GetTotalBrowserCount());
#endif
}
// Test that launching the Help App's release notes logs metrics.
IN_PROC_BROWSER_TEST_P(HelpAppIntegrationTest, HelpAppV2ReleaseNotesMetrics) {
WaitForTestSystemAppInstall();
base::UserActionTester user_action_tester;
chrome::LaunchReleaseNotes(profile(),
apps::mojom::LaunchSource::kFromOtherApp);
#if defined(OS_CHROMEOS) && BUILDFLAG(GOOGLE_CHROME_BRANDING)
EXPECT_EQ(1,
user_action_tester.GetActionCount("ReleaseNotes.ShowReleaseNotes"));
#else
EXPECT_EQ(0,
user_action_tester.GetActionCount("ReleaseNotes.ShowReleaseNotes"));
#endif
}
// Test that launching the Help App's release notes doesn't crash an incognito
// browser.
IN_PROC_BROWSER_TEST_P(HelpAppIntegrationTest, HelpAppV2ReleaseNotesIncognito) {
WaitForTestSystemAppInstall();
Browser* incognito_browser = CreateIncognitoBrowser();
EXPECT_NO_FATAL_FAILURE(chrome::LaunchReleaseNotes(
incognito_browser->profile(), apps::mojom::LaunchSource::kFromOtherApp));
}
// Test that the Help App does a navigation on launch even when it was already
// open with the same URL.
IN_PROC_BROWSER_TEST_P(HelpAppIntegrationTest, HelpAppV2NavigateOnRelaunch) {
......
......@@ -151,8 +151,21 @@ void LaunchReleaseNotesInTab(Profile* profile) {
ShowSingletonTab(displayer->browser(), url);
}
void LaunchReleaseNotesImpl(Profile* profile) {
void LaunchReleaseNotesImpl(Profile* profile,
apps::mojom::LaunchSource source) {
base::RecordAction(UserMetricsAction("ReleaseNotes.ShowReleaseNotes"));
// If the flag is enabled, launch the Help app and show the release notes.
if (base::FeatureList::IsEnabled(chromeos::features::kHelpAppReleaseNotes)) {
// Note that AppServiceProxy is null for off-the-record profiles. For more
// context, see https://crbug.com/1112197.
apps::AppServiceProxy* proxy = apps::AppServiceProxyFactory::GetForProfile(
profile->GetOriginalProfile());
proxy->LaunchAppWithUrl(
chromeos::default_web_apps::kHelpAppId, ui::EventFlags::EF_NONE,
GURL("chrome://help-app/updates"), source, display::kDefaultDisplayId);
return;
}
auto* provider = web_app::WebAppProviderBase::GetProviderBase(profile);
if (provider && provider->registrar().IsInstalled(
chromeos::default_web_apps::kReleaseNotesAppId)) {
......@@ -341,9 +354,9 @@ void ShowHelpForProfile(Profile* profile, HelpSource source) {
ShowHelpImpl(NULL, profile, source);
}
void LaunchReleaseNotes(Profile* profile) {
void LaunchReleaseNotes(Profile* profile, apps::mojom::LaunchSource source) {
#if defined(OS_CHROMEOS) && BUILDFLAG(GOOGLE_CHROME_BRANDING)
LaunchReleaseNotesImpl(profile);
LaunchReleaseNotesImpl(profile, source);
#endif
}
......
......@@ -11,6 +11,7 @@
#include "build/build_config.h"
#include "components/content_settings/core/common/content_settings_types.h"
#include "components/services/app_service/public/mojom/types.mojom.h"
#include "url/gurl.h"
#if !defined(OS_ANDROID)
......@@ -96,7 +97,7 @@ void ShowFeedbackPage(const GURL& page_url,
void ShowHelp(Browser* browser, HelpSource source);
void ShowHelpForProfile(Profile* profile, HelpSource source);
void LaunchReleaseNotes(Profile* profile);
void LaunchReleaseNotes(Profile* profile, apps::mojom::LaunchSource source);
void ShowBetaForum(Browser* browser);
void ShowPolicy(Browser* browser);
void ShowSlow(Browser* browser);
......
......@@ -445,6 +445,18 @@ void AboutHandler::HandleCheckInternetConnection(const base::ListValue* args) {
void AboutHandler::HandleLaunchReleaseNotes(const base::ListValue* args) {
DCHECK(args->empty());
// If the flag is enabled, we can always show the release notes since the Help
// app caches it, or can show an appropriate error state (e.g. No internet
// connection).
if (base::FeatureList::IsEnabled(chromeos::features::kHelpAppReleaseNotes)) {
base::RecordAction(
base::UserMetricsAction("ReleaseNotes.LaunchedAboutPage"));
chrome::LaunchReleaseNotes(profile_,
apps::mojom::LaunchSource::kFromOtherApp);
return;
}
// If the flag is disabled, we need connectivity to load the PWA.
chromeos::NetworkStateHandler* network_state_handler =
chromeos::NetworkHandler::Get()->network_state_handler();
const chromeos::NetworkState* network =
......@@ -452,7 +464,8 @@ void AboutHandler::HandleLaunchReleaseNotes(const base::ListValue* args) {
if (network && network->IsOnline()) {
base::RecordAction(
base::UserMetricsAction("ReleaseNotes.LaunchedAboutPage"));
chrome::LaunchReleaseNotes(profile_);
chrome::LaunchReleaseNotes(profile_,
apps::mojom::LaunchSource::kFromOtherApp);
}
}
......
......@@ -192,26 +192,28 @@ struct IconValue {
// should not be re-ordered or removed.
enum LaunchSource {
kUnknown = 0,
kFromAppListGrid = 1, // Grid of apps, not the search box.
kFromAppListGridContextMenu = 2, // Grid of apps; context menu.
kFromAppListQuery = 3, // Query-dependent results (larger icons).
kFromAppListQueryContextMenu = 4, // Query-dependent results; context menu.
kFromAppListRecommendation = 5, // Query-less recommendations (smaller
// icons).
kFromParentalControls = 6, // Parental Controls Settings Section and
// Per App time notification.
kFromShelf = 7, // Shelf.
kFromFileManager = 8, // FileManager.
kFromLink = 9, // Left-licking on links in the browser.
kFromOmnibox = 10, // Enter URL in the Omnibox in the browser.
kFromChromeInternal = 11, // Chrome internal call.
kFromKeyboard = 12, // Keyboard shortcut for opening app.
kFromOtherApp = 13, // Clicking link in another app or webui.
kFromMenu = 14, // Menu.
kFromInstalledNotification = 15, // Installed notification
kFromTest = 16, // Test
kFromArc = 17, // Arc.
kFromSharesheet = 18, // Sharesheet.
kFromAppListGrid = 1, // Grid of apps, not the search box.
kFromAppListGridContextMenu = 2, // Grid of apps; context menu.
kFromAppListQuery = 3, // Query-dependent results (larger icons).
kFromAppListQueryContextMenu = 4, // Query-dependent results; context menu.
kFromAppListRecommendation = 5, // Query-less recommendations (smaller
// icons).
kFromParentalControls = 6, // Parental Controls Settings Section and
// Per App time notification.
kFromShelf = 7, // Shelf.
kFromFileManager = 8, // FileManager.
kFromLink = 9, // Left-licking on links in the browser.
kFromOmnibox = 10, // Enter URL in the Omnibox in the
// browser.
kFromChromeInternal = 11, // Chrome internal call.
kFromKeyboard = 12, // Keyboard shortcut for opening app.
kFromOtherApp = 13, // Clicking link in another app or webui.
kFromMenu = 14, // Menu.
kFromInstalledNotification = 15, // Installed notification
kFromTest = 16, // Test
kFromArc = 17, // Arc.
kFromSharesheet = 18, // Sharesheet.
kFromReleaseNotesNotification = 19, // Release Notes Notification.
};
enum TriState {
......
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