Commit db891942 authored by Finnur Thorarinsson's avatar Finnur Thorarinsson Committed by Commit Bot

Windows Native Notifications: Plumb the Context Menu value through.

This adds support for encoding the launch id for the context menu item,
in a similar way as for the button index (context menu click is treated
by the Action Center as a button click).

This also removes the check for native notifications being enabled
while processing the command line at startup. Main reason for that
is that the notification activator does not enable native notifications
when it passes the command line to Chrome, but native notifications
are already implied anyway (otherwise the notification activator wouldn't
be used). So, we can remove the check.

Bug: 734095
Change-Id: I629dabaab290fba9043114bf2403daba2b8b7f0c
Reviewed-on: https://chromium-review.googlesource.com/952446
Commit-Queue: Finnur Thorarinsson <finnur@chromium.org>
Reviewed-by: default avatarTommy Martino <tmartino@chromium.org>
Reviewed-by: default avatarPeter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#541800}
parent bd5fdccd
......@@ -13,6 +13,7 @@
enum LaunchIdComponents {
NORMAL = 0,
BUTTON_INDEX = 1,
CONTEXT_MENU = 2,
};
NotificationLaunchId::NotificationLaunchId() = default;
......@@ -47,14 +48,22 @@ NotificationLaunchId::NotificationLaunchId(const std::string& input) {
return;
LaunchIdComponents components = static_cast<LaunchIdComponents>(number);
// The final token may contain the separation character.
size_t min_num_tokens;
switch (components) {
case NORMAL:
// type|notification_type|profile_id|incognito|origin|notification_id
min_num_tokens = 6;
break;
case BUTTON_INDEX:
// type|button_index|notification_type|profile_id|incognito|origin|notification_id
min_num_tokens = 7;
break;
case CONTEXT_MENU:
// type|notification_type|profile_id|incognito|origin|notification_id
min_num_tokens = 6;
is_for_context_menu_ = true;
break;
default:
// |components| has an invalid value.
return;
......@@ -97,11 +106,13 @@ std::string NotificationLaunchId::Serialize() const {
// and unsafe for origins -- and should therefore be encoded (as per
// http://www.ietf.org/rfc/rfc1738.txt).
std::string prefix;
if (button_index_ > -1) {
LaunchIdComponents type = is_for_context_menu_
? CONTEXT_MENU
: (button_index_ > -1 ? BUTTON_INDEX : NORMAL);
if (button_index_ > -1)
prefix = base::StringPrintf("|%d", button_index_);
}
return base::StringPrintf(
"%d%s|%d|%s|%d|%s|%s", button_index_ > -1 ? 1 : 0, prefix.c_str(),
"%d%s|%d|%s|%d|%s|%s", type, prefix.c_str(),
static_cast<int>(notification_type_), profile_id_.c_str(), incognito_,
origin_url_.spec().c_str(), notification_id_.c_str());
}
......@@ -34,7 +34,15 @@ class NotificationLaunchId {
std::string Serialize() const;
void set_button_index(int index) { button_index_ = index; }
void set_button_index(int index) {
DCHECK(!is_for_context_menu_);
button_index_ = index;
}
void set_is_for_context_menu() {
DCHECK_EQ(-1, button_index_);
is_for_context_menu_ = true;
}
NotificationHandler::Type notification_type() const {
DCHECK(is_valid());
......@@ -60,6 +68,10 @@ class NotificationLaunchId {
DCHECK(is_valid());
return button_index_;
}
bool is_for_context_menu() const {
DCHECK(is_valid());
return is_for_context_menu_;
}
private:
NotificationHandler::Type notification_type_;
......@@ -68,6 +80,7 @@ class NotificationLaunchId {
bool incognito_ = false;
GURL origin_url_;
int button_index_ = -1;
bool is_for_context_menu_ = false;
bool is_valid_ = false;
};
......
......@@ -10,12 +10,34 @@
#include "testing/gtest/include/gtest/gtest.h"
TEST(NotificationLaunchIdTest, SerializationTests) {
NotificationLaunchId id(NotificationHandler::Type::WEB_PERSISTENT,
"notification_id", "Default", true,
GURL("https://example.com"));
EXPECT_TRUE(id.is_valid());
EXPECT_EQ("0|0|Default|1|https://example.com/|notification_id",
id.Serialize());
{
NotificationLaunchId id(NotificationHandler::Type::WEB_PERSISTENT,
"notification_id", "Default", true,
GURL("https://example.com"));
ASSERT_TRUE(id.is_valid());
EXPECT_EQ("0|0|Default|1|https://example.com/|notification_id",
id.Serialize());
}
{
NotificationLaunchId id(NotificationHandler::Type::WEB_PERSISTENT,
"notification_id", "Default", true,
GURL("https://example.com"));
id.set_button_index(0);
ASSERT_TRUE(id.is_valid());
EXPECT_EQ("1|0|0|Default|1|https://example.com/|notification_id",
id.Serialize());
}
{
NotificationLaunchId id(NotificationHandler::Type::WEB_PERSISTENT,
"notification_id", "Default", true,
GURL("https://example.com"));
id.set_is_for_context_menu();
ASSERT_TRUE(id.is_valid());
EXPECT_EQ("2|0|Default|1|https://example.com/|notification_id",
id.Serialize());
}
}
TEST(NotificationLaunchIdTest, ParsingTests) {
......@@ -24,8 +46,9 @@ TEST(NotificationLaunchIdTest, ParsingTests) {
std::string encoded = "0|0|Default|1|https://example.com/|notification_id";
NotificationLaunchId id(encoded);
EXPECT_TRUE(id.is_valid());
ASSERT_TRUE(id.is_valid());
EXPECT_EQ(-1, id.button_index());
EXPECT_FALSE(id.is_for_context_menu());
EXPECT_EQ(NotificationHandler::Type::WEB_PERSISTENT,
id.notification_type());
EXPECT_TRUE(id.incognito());
......@@ -39,8 +62,9 @@ TEST(NotificationLaunchIdTest, ParsingTests) {
"0|0|Default|1|https://example.com/|notification_id|Extra|Data";
NotificationLaunchId id(encoded);
EXPECT_TRUE(id.is_valid());
ASSERT_TRUE(id.is_valid());
EXPECT_EQ(-1, id.button_index());
EXPECT_FALSE(id.is_for_context_menu());
EXPECT_EQ(NotificationHandler::Type::WEB_PERSISTENT,
id.notification_type());
EXPECT_TRUE(id.incognito());
......@@ -54,8 +78,9 @@ TEST(NotificationLaunchIdTest, ParsingTests) {
"1|0|0|Default|1|https://example.com/|notification_id";
NotificationLaunchId id(encoded);
EXPECT_TRUE(id.is_valid());
ASSERT_TRUE(id.is_valid());
EXPECT_EQ(0, id.button_index());
EXPECT_FALSE(id.is_for_context_menu());
EXPECT_EQ(NotificationHandler::Type::WEB_PERSISTENT,
id.notification_type());
EXPECT_TRUE(id.incognito());
......@@ -67,18 +92,33 @@ TEST(NotificationLaunchIdTest, ParsingTests) {
// id.
{
std::string encoded =
"1|0|0|Default|1|https://example.com/"
"|notification_id|Extra|Data|";
"1|0|0|Default|1|https://example.com/|notification_id|Extra|Data|";
NotificationLaunchId id(encoded);
EXPECT_TRUE(id.is_valid());
ASSERT_TRUE(id.is_valid());
EXPECT_EQ(0, id.button_index());
EXPECT_FALSE(id.is_for_context_menu());
EXPECT_EQ(NotificationHandler::Type::WEB_PERSISTENT,
id.notification_type());
EXPECT_TRUE(id.incognito());
EXPECT_EQ("Default", id.profile_id());
EXPECT_EQ("notification_id|Extra|Data|", id.notification_id());
}
// Input string for when the context menu item is selected.
{
std::string encoded = "2|0|Default|1|https://example.com/|notification_id";
NotificationLaunchId id(encoded);
ASSERT_TRUE(id.is_valid());
EXPECT_EQ(-1, id.button_index());
EXPECT_TRUE(id.is_for_context_menu());
EXPECT_EQ(NotificationHandler::Type::WEB_PERSISTENT,
id.notification_type());
EXPECT_TRUE(id.incognito());
EXPECT_EQ("Default", id.profile_id());
EXPECT_EQ("notification_id", id.notification_id());
}
}
TEST(NotificationLaunchIdTest, ParsingErrorCases) {
......@@ -102,6 +142,8 @@ TEST(NotificationLaunchIdTest, ParsingErrorCases) {
{"0"},
// Missing all but the component type (type BUTTON_INDEX).
{"1"},
// Missing all but the component type (type CONTEXT_MENU).
{"2"},
};
for (const auto& test_case : cases) {
......
......@@ -740,10 +740,14 @@ bool NotificationPlatformBridgeWin::HandleActivation(
if (!launch_id.is_valid())
return false;
NotificationCommon::Operation operation = launch_id.is_for_context_menu()
? NotificationCommon::SETTINGS
: NotificationCommon::CLICK;
ForwardNotificationOperationOnUiThread(
NotificationCommon::CLICK, launch_id.notification_type(),
launch_id.origin_url(), launch_id.notification_id(),
launch_id.profile_id(), launch_id.incognito(),
operation, launch_id.notification_type(), launch_id.origin_url(),
launch_id.notification_id(), launch_id.profile_id(),
launch_id.incognito(),
/*action_index=*/base::nullopt, /*by_user=*/true);
return true;
......
......@@ -54,6 +54,7 @@ class NotificationPlatformBridgeWin : public NotificationPlatformBridge {
FRIEND_TEST_ALL_PREFIXES(NotificationPlatformBridgeWinTest, Suppress);
FRIEND_TEST_ALL_PREFIXES(NotificationPlatformBridgeWinUITest, GetDisplayed);
FRIEND_TEST_ALL_PREFIXES(NotificationPlatformBridgeWinUITest, HandleEvent);
FRIEND_TEST_ALL_PREFIXES(NotificationPlatformBridgeWinUITest, HandleSettings);
void PostTaskToTaskRunnerThread(base::OnceClosure closure) const;
......
......@@ -189,6 +189,54 @@ IN_PROC_BROWSER_TEST_F(NotificationPlatformBridgeWinUITest, HandleEvent) {
EXPECT_EQ(base::nullopt, last_by_user_);
}
IN_PROC_BROWSER_TEST_F(NotificationPlatformBridgeWinUITest, HandleSettings) {
// This test exercises a feature that is not enabled in older versions of
// Windows.
if (base::win::GetVersion() < base::win::VERSION_WIN8)
return;
const wchar_t kXmlDoc[] =
LR"(<toast launch="0|0|Default|0|https://example.com/|notification_id">
<visual>
<binding template="ToastGeneric">
<text>My Title</text>
<text placement="attribution">example.com</text>
</binding>
</visual>
<actions>
<action content="settings" placement="contextMenu" activationType="foreground" arguments="2|0|Default|0|https://example.com/|notification_id"/>
</actions>
</toast>
)";
MockIToastNotification toast(kXmlDoc, L"tag");
MockIToastActivatedEventArgs args(
L"2|0|Default|0|https://example.com/|notification_id");
base::RunLoop run_loop;
display_service_tester_->SetProcessNotificationOperationDelegate(
base::BindRepeating(&NotificationPlatformBridgeWinUITest::HandleOperation,
base::Unretained(this), run_loop.QuitClosure()));
// Simulate clicks on the toast.
NotificationPlatformBridgeWin* bridge =
static_cast<NotificationPlatformBridgeWin*>(
g_browser_process->notification_platform_bridge());
ASSERT_TRUE(bridge);
bridge->ForwardHandleEventForTesting(NotificationCommon::SETTINGS, &toast,
&args, base::nullopt);
run_loop.Run();
// Validate the click values.
EXPECT_EQ(NotificationCommon::SETTINGS, last_operation_);
EXPECT_EQ(NotificationHandler::Type::WEB_PERSISTENT, last_notification_type_);
EXPECT_EQ(GURL("https://example.com/"), last_origin_);
EXPECT_EQ("notification_id", last_notification_id_);
EXPECT_EQ(base::nullopt, last_action_index_);
EXPECT_EQ(base::nullopt, last_reply_);
EXPECT_EQ(base::nullopt, last_by_user_);
}
IN_PROC_BROWSER_TEST_F(NotificationPlatformBridgeWinUITest, GetDisplayed) {
// This test requires WinRT core functions, which are not available in
// older versions of Windows.
......
......@@ -48,7 +48,6 @@ const char kInputElement[] = "input";
const char kInputId[] = "id";
const char kInputType[] = "type";
const char kStatus[] = "status";
const char kNotificationSettings[] = "notificationSettings";
const char kPlaceholderContent[] = "placeHolderContent";
const char kPlacement[] = "placement";
const char kPlacementAppLogoOverride[] = "appLogoOverride";
......@@ -130,7 +129,7 @@ std::unique_ptr<NotificationTemplateBuilder> NotificationTemplateBuilder::Build(
builder->StartActionsElement();
if (!notification.buttons().empty())
builder->AddActions(notification, launch_id);
builder->AddContextMenu();
builder->AddContextMenu(launch_id);
builder->EndActionsElement();
if (notification.silent())
......@@ -317,12 +316,16 @@ void NotificationTemplateBuilder::AddActions(
WriteActionElement(buttons[i], i, notification.origin_url(), launch_id);
}
void NotificationTemplateBuilder::AddContextMenu() {
void NotificationTemplateBuilder::AddContextMenu(
NotificationLaunchId copied_launch_id) {
std::string notification_settings_msg = l10n_util::GetStringUTF8(
IDS_WIN_NOTIFICATION_SETTINGS_CONTEXT_MENU_ITEM_NAME);
if (context_menu_label_override_)
notification_settings_msg = context_menu_label_override_;
WriteContextMenuElement(notification_settings_msg, kNotificationSettings);
copied_launch_id.set_is_for_context_menu();
WriteContextMenuElement(notification_settings_msg,
copied_launch_id.Serialize());
}
void NotificationTemplateBuilder::StartActionsElement() {
......@@ -343,12 +346,12 @@ void NotificationTemplateBuilder::WriteActionElement(
const message_center::ButtonInfo& button,
int index,
const GURL& origin,
NotificationLaunchId launch_id) {
NotificationLaunchId copied_launch_id) {
xml_writer_->StartElement(kActionElement);
xml_writer_->AddAttribute(kActivationType, kForeground);
xml_writer_->AddAttribute(kContent, base::UTF16ToUTF8(button.title));
launch_id.set_button_index(index);
xml_writer_->AddAttribute(kArguments, launch_id.Serialize());
copied_launch_id.set_button_index(index);
xml_writer_->AddAttribute(kArguments, copied_launch_id.Serialize());
if (!button.icon.IsEmpty()) {
base::FilePath path = image_retainer_->RegisterTemporaryImage(
......
......@@ -123,10 +123,10 @@ class NotificationTemplateBuilder {
void WriteActionElement(const message_center::ButtonInfo& button,
int index,
const GURL& origin,
NotificationLaunchId launch_id);
NotificationLaunchId copied_launch_id);
// Adds context menu actions to the notification sent by |origin|.
void AddContextMenu();
void AddContextMenu(NotificationLaunchId copied_launch_id);
void WriteContextMenuElement(const std::string& content,
const std::string& arguments);
......
......@@ -114,7 +114,7 @@ TEST_F(NotificationTemplateBuilderTest, SimpleToast) {
</binding>
</visual>
<actions>
<action content="settings" placement="contextMenu" activationType="foreground" arguments="notificationSettings"/>
<action content="settings" placement="contextMenu" activationType="foreground" arguments="2|0|Default|0|https://example.com/|notification_id"/>
</actions>
</toast>
)";
......@@ -143,7 +143,7 @@ TEST_F(NotificationTemplateBuilderTest, Buttons) {
<actions>
<action activationType="foreground" content="Button1" arguments="1|0|0|Default|0|https://example.com/|notification_id"/>
<action activationType="foreground" content="Button2" arguments="1|1|0|Default|0|https://example.com/|notification_id"/>
<action content="settings" placement="contextMenu" activationType="foreground" arguments="notificationSettings"/>
<action content="settings" placement="contextMenu" activationType="foreground" arguments="2|0|Default|0|https://example.com/|notification_id"/>
</actions>
</toast>
)";
......@@ -175,7 +175,7 @@ TEST_F(NotificationTemplateBuilderTest, InlineReplies) {
<input id="userResponse" type="text" placeHolderContent="Reply here"/>
<action activationType="foreground" content="Button1" arguments="1|0|0|Default|0|https://example.com/|notification_id"/>
<action activationType="foreground" content="Button2" arguments="1|1|0|Default|0|https://example.com/|notification_id"/>
<action content="settings" placement="contextMenu" activationType="foreground" arguments="notificationSettings"/>
<action content="settings" placement="contextMenu" activationType="foreground" arguments="2|0|Default|0|https://example.com/|notification_id"/>
</actions>
</toast>
)";
......@@ -209,7 +209,7 @@ TEST_F(NotificationTemplateBuilderTest, InlineRepliesDoubleInput) {
<input id="userResponse" type="text" placeHolderContent="Reply here"/>
<action activationType="foreground" content="Button1" arguments="1|0|0|Default|0|https://example.com/|notification_id"/>
<action activationType="foreground" content="Button2" arguments="1|1|0|Default|0|https://example.com/|notification_id"/>
<action content="settings" placement="contextMenu" activationType="foreground" arguments="notificationSettings"/>
<action content="settings" placement="contextMenu" activationType="foreground" arguments="2|0|Default|0|https://example.com/|notification_id"/>
</actions>
</toast>
)";
......@@ -241,7 +241,7 @@ TEST_F(NotificationTemplateBuilderTest, InlineRepliesTextTypeNotFirst) {
<input id="userResponse" type="text" placeHolderContent="Reply here"/>
<action activationType="foreground" content="Button1" arguments="1|0|0|Default|0|https://example.com/|notification_id"/>
<action activationType="foreground" content="Button2" arguments="1|1|0|Default|0|https://example.com/|notification_id"/>
<action content="settings" placement="contextMenu" activationType="foreground" arguments="notificationSettings"/>
<action content="settings" placement="contextMenu" activationType="foreground" arguments="2|0|Default|0|https://example.com/|notification_id"/>
</actions>
</toast>
)";
......@@ -264,7 +264,7 @@ TEST_F(NotificationTemplateBuilderTest, Silent) {
</binding>
</visual>
<actions>
<action content="settings" placement="contextMenu" activationType="foreground" arguments="notificationSettings"/>
<action content="settings" placement="contextMenu" activationType="foreground" arguments="2|0|Default|0|https://example.com/|notification_id"/>
</actions>
<audio silent="true"/>
</toast>
......@@ -293,7 +293,7 @@ TEST_F(NotificationTemplateBuilderTest, RequireInteraction) {
</visual>
<actions>
<action activationType="foreground" content="Button1" arguments="1|0|0|Default|0|https://example.com/|notification_id"/>
<action content="settings" placement="contextMenu" activationType="foreground" arguments="notificationSettings"/>
<action content="settings" placement="contextMenu" activationType="foreground" arguments="2|0|Default|0|https://example.com/|notification_id"/>
</actions>
</toast>
)";
......@@ -317,7 +317,7 @@ TEST_F(NotificationTemplateBuilderTest, NullTimestamp) {
</binding>
</visual>
<actions>
<action content="settings" placement="contextMenu" activationType="foreground" arguments="notificationSettings"/>
<action content="settings" placement="contextMenu" activationType="foreground" arguments="2|0|Default|0|https://example.com/|notification_id"/>
</actions>
</toast>
)";
......@@ -341,7 +341,7 @@ TEST_F(NotificationTemplateBuilderTest, LocalizedContextMenu) {
</binding>
</visual>
<actions>
<action content="%ls" placement="contextMenu" activationType="foreground" arguments="notificationSettings"/>
<action content="%ls" placement="contextMenu" activationType="foreground" arguments="2|0|Default|0|https://example.com/|notification_id"/>
</actions>
</toast>
)";
......@@ -386,7 +386,7 @@ TEST_F(NotificationTemplateBuilderTest, Images) {
<actions>
<input id="userResponse" type="text" placeHolderContent="Reply here"/>
<action activationType="foreground" content="Button1" arguments="1|0|0|Default|0|https://example.com/|notification_id" imageUri="c:\temp\img2.tmp"/>
<action content="settings" placement="contextMenu" activationType="foreground" arguments="notificationSettings"/>
<action content="settings" placement="contextMenu" activationType="foreground" arguments="2|0|Default|0|https://example.com/|notification_id"/>
</actions>
</toast>
)";
......@@ -410,7 +410,7 @@ TEST_F(NotificationTemplateBuilderTest, ContextMessage) {
</binding>
</visual>
<actions>
<action content="settings" placement="contextMenu" activationType="foreground" arguments="notificationSettings"/>
<action content="settings" placement="contextMenu" activationType="foreground" arguments="2|0|Default|0|https://example.com/|notification_id"/>
</actions>
</toast>
)";
......@@ -437,7 +437,7 @@ TEST_F(NotificationTemplateBuilderTest, ExtensionNoContextMessage) {
</binding>
</visual>
<actions>
<action content="settings" placement="contextMenu" activationType="foreground" arguments="notificationSettings"/>
<action content="settings" placement="contextMenu" activationType="foreground" arguments="2|0|Default|0|https://example.com/|notification_id"/>
</actions>
</toast>
)";
......@@ -463,7 +463,7 @@ TEST_F(NotificationTemplateBuilderTest, ProgressBar) {
</binding>
</visual>
<actions>
<action content="settings" placement="contextMenu" activationType="foreground" arguments="notificationSettings"/>
<action content="settings" placement="contextMenu" activationType="foreground" arguments="2|0|Default|0|https://example.com/|notification_id"/>
</actions>
</toast>
)";
......@@ -498,7 +498,7 @@ title4 - message4
</binding>
</visual>
<actions>
<action content="settings" placement="contextMenu" activationType="foreground" arguments="notificationSettings"/>
<action content="settings" placement="contextMenu" activationType="foreground" arguments="2|0|Default|0|https://example.com/|notification_id"/>
</actions>
</toast>
)";
......
......@@ -957,8 +957,7 @@ base::FilePath GetStartupProfilePath(const base::FilePath& user_data_dir,
}
#if defined(OS_WIN)
if (command_line.HasSwitch(switches::kNotificationLaunchId) &&
NotificationPlatformBridgeWin::NativeNotificationEnabled()) {
if (command_line.HasSwitch(switches::kNotificationLaunchId)) {
std::string profile_id =
NotificationPlatformBridgeWin::GetProfileIdFromLaunchId(
command_line.GetSwitchValueASCII(switches::kNotificationLaunchId));
......
......@@ -335,8 +335,7 @@ bool StartupBrowserCreatorImpl::Launch(Profile* profile,
// Launch() call is from notification_helper.exe to process toast activation.
// Delegate to the notification system; do not open a browser window here.
if (command_line_.HasSwitch(switches::kNotificationLaunchId)) {
if (NotificationPlatformBridgeWin::NativeNotificationEnabled() &&
NotificationPlatformBridgeWin::HandleActivation(
if (NotificationPlatformBridgeWin::HandleActivation(
command_line_.GetSwitchValueASCII(
switches::kNotificationLaunchId))) {
RecordLaunchModeHistogram(LM_WIN_PLATFORM_NOTIFICATION);
......
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