Commit 988ef967 authored by Peter Beverloo's avatar Peter Beverloo Committed by Commit Bot

Use base::Optional<> in notification event dispatching code

This updates the use of the |action_index| and |reply| values throughout
the notification event dispatching code to base::Optional<>, reflecting
that neither has to be present.

BUG=

Change-Id: I930cd6a103138e91ffb2f4f0bfdf074c87a0c44b
Reviewed-on: https://chromium-review.googlesource.com/655307Reviewed-by: default avatarJohn Mellor <johnme@chromium.org>
Reviewed-by: default avatarNasko Oskov <nasko@chromium.org>
Commit-Queue: Peter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#501290}
parent bb8ba896
......@@ -38,20 +38,8 @@ void PersistentNotificationHandler::OnClick(
const GURL notification_origin(origin);
DCHECK(notification_origin.is_valid());
// TODO(peter): Convert the notification stack from this point onward to use
// base::Optional<> as well.
int action_index_int = -1 /* no_action */;
if (action_index.has_value())
action_index_int = action_index.value();
base::NullableString16 nullable_reply;
if (reply.has_value())
nullable_reply = base::NullableString16(reply.value(), false /* is_null */);
PlatformNotificationServiceImpl::GetInstance()->OnPersistentNotificationClick(
profile, notification_id, notification_origin, action_index_int,
nullable_reply);
profile, notification_id, notification_origin, action_index, reply);
}
void PersistentNotificationHandler::OpenSettings(Profile* profile) {
......
......@@ -127,8 +127,8 @@ void PlatformNotificationServiceImpl::OnPersistentNotificationClick(
BrowserContext* browser_context,
const std::string& notification_id,
const GURL& origin,
int action_index,
const base::NullableString16& reply) {
const base::Optional<int>& action_index,
const base::Optional<base::string16>& reply) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
blink::mojom::PermissionStatus permission_status =
CheckPermissionOnUIThread(browser_context, origin,
......@@ -142,12 +142,12 @@ void PlatformNotificationServiceImpl::OnPersistentNotificationClick(
return;
}
if (action_index == -1) {
base::RecordAction(
base::UserMetricsAction("Notifications.Persistent.Clicked"));
} else {
if (action_index.has_value()) {
base::RecordAction(base::UserMetricsAction(
"Notifications.Persistent.ClickedActionButton"));
} else {
base::RecordAction(
base::UserMetricsAction("Notifications.Persistent.Clicked"));
}
#if BUILDFLAG(ENABLE_BACKGROUND)
......
......@@ -16,6 +16,7 @@
#include "base/gtest_prod_util.h"
#include "base/macros.h"
#include "base/memory/singleton.h"
#include "base/optional.h"
#include "base/strings/string16.h"
#include "chrome/browser/notifications/notification.h"
#include "chrome/browser/notifications/notification_common.h"
......@@ -28,10 +29,6 @@
class NotificationDelegate;
class ScopedKeepAlive;
namespace base {
class NullableString16;
}
namespace content {
class BrowserContext;
struct NotificationResources;
......@@ -53,11 +50,12 @@ class PlatformNotificationServiceImpl
// To be called when a persistent notification has been clicked on. The
// Service Worker associated with the registration will be started if
// needed, on which the event will be fired. Must be called on the UI thread.
void OnPersistentNotificationClick(content::BrowserContext* browser_context,
const std::string& notification_id,
const GURL& origin,
int action_index,
const base::NullableString16& reply);
void OnPersistentNotificationClick(
content::BrowserContext* browser_context,
const std::string& notification_id,
const GURL& origin,
const base::Optional<int>& action_index,
const base::Optional<base::string16>& reply);
// To be called when a persistent notification has been closed. The data
// associated with the notification has to be pruned from the database in this
......
......@@ -206,27 +206,27 @@ void ReadNotificationDatabaseData(
void DispatchNotificationClickEventOnWorker(
const scoped_refptr<ServiceWorkerVersion>& service_worker,
const NotificationDatabaseData& notification_database_data,
int action_index,
const base::NullableString16& reply,
const base::Optional<int>& action_index,
const base::Optional<base::string16>& reply,
const ServiceWorkerVersion::StatusCallback& callback) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
int request_id = service_worker->StartRequest(
ServiceWorkerMetrics::EventType::NOTIFICATION_CLICK, callback);
base::Optional<base::string16> optional_reply;
if (!reply.is_null())
optional_reply = reply.string();
int action_index_int = -1 /* no value */;
if (action_index.has_value())
action_index_int = action_index.value();
service_worker->event_dispatcher()->DispatchNotificationClickEvent(
notification_database_data.notification_id,
notification_database_data.notification_data, action_index,
optional_reply, service_worker->CreateSimpleEventCallback(request_id));
notification_database_data.notification_data, action_index_int, reply,
service_worker->CreateSimpleEventCallback(request_id));
}
// Dispatches the notification click event on the |service_worker_registration|.
void DoDispatchNotificationClickEvent(
int action_index,
const base::NullableString16& reply,
const base::Optional<int>& action_index,
const base::Optional<base::string16>& reply,
const NotificationDispatchCompleteCallback& dispatch_complete_callback,
const scoped_refptr<PlatformNotificationContext>& notification_context,
const ServiceWorkerRegistration* service_worker_registration,
......@@ -368,8 +368,8 @@ void NotificationEventDispatcherImpl::DispatchNotificationClickEvent(
BrowserContext* browser_context,
const std::string& notification_id,
const GURL& origin,
int action_index,
const base::NullableString16& reply,
const base::Optional<int>& action_index,
const base::Optional<base::string16>& reply,
const NotificationDispatchCompleteCallback& dispatch_complete_callback) {
DispatchNotificationEvent(
browser_context, notification_id, origin,
......
......@@ -25,8 +25,8 @@ class NotificationEventDispatcherImpl : public NotificationEventDispatcher {
BrowserContext* browser_context,
const std::string& notification_id,
const GURL& origin,
int action_index,
const base::NullableString16& reply,
const base::Optional<int>& action_index,
const base::Optional<base::string16>& reply,
const NotificationDispatchCompleteCallback& dispatch_complete_callback)
override;
void DispatchNotificationCloseEvent(
......
......@@ -8,15 +8,13 @@
#include <string>
#include "base/callback_forward.h"
#include "base/optional.h"
#include "base/strings/string16.h"
#include "content/common/content_export.h"
#include "content/public/common/persistent_notification_status.h"
class GURL;
namespace base {
class NullableString16;
}
namespace content {
class BrowserContext;
......@@ -42,8 +40,8 @@ class CONTENT_EXPORT NotificationEventDispatcher {
BrowserContext* browser_context,
const std::string& notification_id,
const GURL& origin,
int action_index,
const base::NullableString16& reply,
const base::Optional<int>& action_index,
const base::Optional<base::string16>& reply,
const NotificationDispatchCompleteCallback&
dispatch_complete_callback) = 0;
......
......@@ -133,8 +133,8 @@ void LayoutTestMessageFilter::OnSetDatabaseQuota(int quota) {
void LayoutTestMessageFilter::OnSimulateWebNotificationClick(
const std::string& title,
int action_index,
const base::NullableString16& reply) {
const base::Optional<int>& action_index,
const base::Optional<base::string16>& reply) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
LayoutTestNotificationManager* manager =
LayoutTestContentBrowserClient::Get()->GetLayoutTestNotificationManager();
......
......@@ -10,6 +10,8 @@
#include "base/files/file_path.h"
#include "base/macros.h"
#include "base/optional.h"
#include "base/strings/string16.h"
#include "content/public/browser/browser_message_filter.h"
#include "third_party/WebKit/public/platform/modules/permissions/permission_status.mojom.h"
......@@ -17,7 +19,6 @@ class GURL;
namespace base {
class DictionaryValue;
class NullableString16;
}
namespace net {
......@@ -59,9 +60,10 @@ class LayoutTestMessageFilter : public BrowserMessageFilter {
void OnGrantWebNotificationPermission(const GURL& origin,
bool permission_granted);
void OnClearWebNotificationPermissions();
void OnSimulateWebNotificationClick(const std::string& title,
int action_index,
const base::NullableString16& reply);
void OnSimulateWebNotificationClick(
const std::string& title,
const base::Optional<int>& action_index,
const base::Optional<base::string16>& reply);
void OnSimulateWebNotificationClose(const std::string& title, bool by_user);
void OnSetPushMessagingPermission(const GURL& origin, bool allowed);
void OnClearPushMessagingPermissions();
......
......@@ -6,6 +6,8 @@
#include <string>
#include <vector>
#include "base/optional.h"
#include "base/strings/string16.h"
#include "content/public/common/common_param_traits_macros.h"
#include "ipc/ipc_message_macros.h"
#include "ipc/ipc_platform_file.h"
......@@ -26,8 +28,8 @@ IPC_MESSAGE_ROUTED1(LayoutTestHostMsg_SetDatabaseQuota,
int /* quota */)
IPC_MESSAGE_ROUTED3(LayoutTestHostMsg_SimulateWebNotificationClick,
std::string /* title */,
int /* action_index */,
base::NullableString16 /* reply */)
base::Optional<int> /* action_index */,
base::Optional<base::string16> /* reply */)
IPC_MESSAGE_ROUTED2(LayoutTestHostMsg_SimulateWebNotificationClose,
std::string /* title */,
bool /* by_user */)
......
......@@ -462,8 +462,8 @@ void BlinkTestRunner::SetDatabaseQuota(int quota) {
void BlinkTestRunner::SimulateWebNotificationClick(
const std::string& title,
int action_index,
const base::NullableString16& reply) {
const base::Optional<int>& action_index,
const base::Optional<base::string16>& reply) {
Send(new LayoutTestHostMsg_SimulateWebNotificationClick(routing_id(), title,
action_index, reply));
}
......
......@@ -12,6 +12,8 @@
#include "base/containers/circular_deque.h"
#include "base/files/file_path.h"
#include "base/macros.h"
#include "base/optional.h"
#include "base/strings/string16.h"
#include "content/public/common/page_state.h"
#include "content/public/renderer/render_view_observer.h"
#include "content/public/renderer/render_view_observer_tracker.h"
......@@ -98,8 +100,8 @@ class BlinkTestRunner : public RenderViewObserver,
void SetDatabaseQuota(int quota) override;
void SimulateWebNotificationClick(
const std::string& title,
int action_index,
const base::NullableString16& reply) override;
const base::Optional<int>& action_index,
const base::Optional<base::string16>& reply) override;
void SimulateWebNotificationClose(const std::string& title,
bool by_user) override;
void SetDeviceScaleFactor(float factor) override;
......
......@@ -253,10 +253,7 @@ class TestRunnerBindings : public gin::Wrappable<TestRunnerBindings> {
void SetWindowIsKey(bool value);
void SetXSSAuditorEnabled(bool enabled);
void ShowWebInspector(gin::Arguments* args);
void SimulateWebNotificationClick(const std::string& title, int action_index);
void SimulateWebNotificationClickWithReply(const std::string& title,
int action_index,
const std::string& reply);
void SimulateWebNotificationClick(gin::Arguments* args);
void SimulateWebNotificationClose(const std::string& title, bool by_user);
void UseUnfortunateSynchronousResizeMode();
void WaitForPolicyDelegate();
......@@ -609,8 +606,6 @@ gin::ObjectTemplateBuilder TestRunnerBindings::GetObjectTemplateBuilder(
.SetMethod("showWebInspector", &TestRunnerBindings::ShowWebInspector)
.SetMethod("simulateWebNotificationClick",
&TestRunnerBindings::SimulateWebNotificationClick)
.SetMethod("simulateWebNotificationClickWithReply",
&TestRunnerBindings::SimulateWebNotificationClickWithReply)
.SetMethod("simulateWebNotificationClose",
&TestRunnerBindings::SimulateWebNotificationClose)
.SetProperty("tooltipText", &TestRunnerBindings::TooltipText)
......@@ -1377,23 +1372,34 @@ void TestRunnerBindings::SetMIDIAccessorResult(bool result) {
}
}
void TestRunnerBindings::SimulateWebNotificationClick(const std::string& title,
int action_index) {
void TestRunnerBindings::SimulateWebNotificationClick(gin::Arguments* args) {
DCHECK_GE(args->Length(), 1);
if (!runner_)
return;
runner_->SimulateWebNotificationClick(title, action_index,
base::NullableString16());
}
void TestRunnerBindings::SimulateWebNotificationClickWithReply(
const std::string& title,
int action_index,
const std::string& reply) {
if (!runner_)
return;
runner_->SimulateWebNotificationClick(
title, action_index,
base::NullableString16(base::UTF8ToUTF16(reply), false /* is_null */));
std::string title;
base::Optional<int> action_index;
base::Optional<base::string16> reply;
args->GetNext(&title);
// Optional |action_index| argument.
if (args->Length() >= 2) {
int action_index_int;
args->GetNext(&action_index_int);
action_index = action_index_int;
}
// Optional |reply| argument.
if (args->Length() >= 3) {
std::string reply_string;
args->GetNext(&reply_string);
reply = base::UTF8ToUTF16(reply_string);
}
runner_->SimulateWebNotificationClick(title, action_index, reply);
}
void TestRunnerBindings::SimulateWebNotificationClose(const std::string& title,
......@@ -2761,8 +2767,8 @@ void TestRunner::SetMIDIAccessorResult(midi::mojom::Result result) {
void TestRunner::SimulateWebNotificationClick(
const std::string& title,
int action_index,
const base::NullableString16& reply) {
const base::Optional<int>& action_index,
const base::Optional<base::string16>& reply) {
delegate_->SimulateWebNotificationClick(title, action_index, reply);
}
......
......@@ -15,6 +15,8 @@
#include "base/containers/circular_deque.h"
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "base/optional.h"
#include "base/strings/string16.h"
#include "content/shell/test_runner/layout_test_runtime_flags.h"
#include "content/shell/test_runner/test_runner_export.h"
#include "content/shell/test_runner/web_test_runner.h"
......@@ -26,10 +28,6 @@
class GURL;
class SkBitmap;
namespace base {
class NullableString16;
}
namespace blink {
class WebContentSettingsClient;
class WebFrame;
......@@ -532,9 +530,10 @@ class TestRunner : public WebTestRunner {
void SetMIDIAccessorResult(midi::mojom::Result result);
// Simulates a click on a Web Notification.
void SimulateWebNotificationClick(const std::string& title,
int action_index,
const base::NullableString16& reply);
void SimulateWebNotificationClick(
const std::string& title,
const base::Optional<int>& action_index,
const base::Optional<base::string16>& reply);
// Simulates closing a Web Notification.
void SimulateWebNotificationClose(const std::string& title, bool by_user);
......
......@@ -11,6 +11,8 @@
#include "base/callback_forward.h"
#include "base/memory/ref_counted.h"
#include "base/optional.h"
#include "base/strings/string16.h"
#include "third_party/WebKit/public/platform/WebString.h"
#include "third_party/WebKit/public/platform/WebURL.h"
#include "third_party/WebKit/public/platform/WebVector.h"
......@@ -150,8 +152,8 @@ class WebTestDelegate {
// Controls Web Notifications.
virtual void SimulateWebNotificationClick(
const std::string& title,
int action_index,
const base::NullableString16& reply) = 0;
const base::Optional<int>& action_index,
const base::Optional<base::string16>& reply) = 0;
virtual void SimulateWebNotificationClose(const std::string& title,
bool by_user) = 0;
......
......@@ -95,8 +95,8 @@ void MockPlatformNotificationService::GetDisplayedNotifications(
void MockPlatformNotificationService::SimulateClick(
const std::string& title,
int action_index,
const base::NullableString16& reply) {
const base::Optional<int>& action_index,
const base::Optional<base::string16>& reply) {
DCHECK_CURRENTLY_ON(BrowserThread::UI);
const auto notification_id_iter = notification_id_map_.find(title);
if (notification_id_iter == notification_id_map_.end())
......@@ -116,8 +116,9 @@ void MockPlatformNotificationService::SimulateClick(
notification.browser_context, notification_id, notification.origin,
action_index, reply, base::Bind(&OnEventDispatchComplete));
} else if (non_persistent_iter != non_persistent_notifications_.end()) {
DCHECK_EQ(action_index, -1) << "Action buttons are only supported for "
"persistent notifications";
DCHECK(!action_index.has_value())
<< "Action buttons are only supported for "
"persistent notifications";
NotificationEventDispatcher::GetInstance()->DispatchNonPersistentClickEvent(
notification_id);
}
......
......@@ -13,14 +13,12 @@
#include "base/callback.h"
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "base/optional.h"
#include "base/strings/string16.h"
#include "content/public/browser/platform_notification_service.h"
#include "third_party/WebKit/public/platform/modules/permissions/permission_status.mojom.h"
#include "url/gurl.h"
namespace base {
class NullableString16;
}
namespace content {
struct NotificationResources;
......@@ -34,12 +32,11 @@ class MockPlatformNotificationService : public PlatformNotificationService {
~MockPlatformNotificationService() override;
// Simulates a click on the notification titled |title|. |action_index|
// indicates which action was clicked, or -1 if the main notification body was
// clicked. |reply| indicates the user reply, if any.
// indicates which action was clicked. |reply| indicates the user reply.
// Must be called on the UI thread.
void SimulateClick(const std::string& title,
int action_index,
const base::NullableString16& reply);
const base::Optional<int>& action_index,
const base::Optional<base::string16>& reply);
// Simulates the closing a notification titled |title|. Must be called on
// the UI thread.
......
......@@ -33,7 +33,7 @@
var notification = new Notification('My Notification');
notification.addEventListener('show', test.step_func(function() {
if (window.testRunner)
testRunner.simulateWebNotificationClick('My Notification', -1 /* action_index */);
testRunner.simulateWebNotificationClick('My Notification');
}));
notification.addEventListener('click', test.step_func(function() {
......
......@@ -12,7 +12,7 @@ async_test(function(test) {
var notification = new Notification('My Notification');
notification.addEventListener('show', function() {
if (testRunner)
testRunner.simulateWebNotificationClick('My Notification', -1 /* action_index */);
testRunner.simulateWebNotificationClick('My Notification');
});
notification.addEventListener('click', function() {
......
......@@ -7,7 +7,7 @@ function supportTestRunnerMessagesOnPort(messagePort)
messagePort.addEventListener('message', function(message) {
if (message.data.type == 'simulateWebNotificationClick')
testRunner.simulateWebNotificationClick(message.data.title, -1 /* action_index */);
testRunner.simulateWebNotificationClick(message.data.title);
});
}
......@@ -85,8 +85,7 @@ function sendCommand(port, data)
}
// Simulates a click on the notification whose title equals |title|. The |actionIndex| specifies
// which action button to activate, where -1 means the notification itself is clicked, not an action
// button.
// which action button to activate, if any.
function simulateNotificationClick(title, actionIndex, port)
{
return new Promise((resolve, reject) => {
......
......@@ -16,6 +16,6 @@ var testRunner = {
simulateWebNotificationClick: function(title, action_index)
{
if (_port)
_port.postMessage({ type: 'simulateWebNotificationClick', title: title, action_index: action_index });
_port.postMessage({ type: 'simulateWebNotificationClick', title, action_index });
}
};
......@@ -33,7 +33,7 @@
});
}).then(function() {
// (2) Simulate a click on the notification that has been displayed.
testRunner.simulateWebNotificationClick(scope, -1 /* action_index */);
testRunner.simulateWebNotificationClick(scope);
workerInfo.port.addEventListener('message', function(event) {
if (typeof event.data != 'object' || typeof event.data.success != 'boolean') {
......
......@@ -47,7 +47,7 @@
// (3) Simulate a click on each button and on the notification body.
for (var i = 0; i < options.actions.length; ++i)
testRunner.simulateWebNotificationClick(scope, i);
testRunner.simulateWebNotificationClick(scope, -1 /* action_index */);
testRunner.simulateWebNotificationClick(scope);
port.addEventListener('message', function(event) {
// (4) Listen for confirmation from the Service Worker that the
......
......@@ -18,7 +18,7 @@
async_test(function(test) {
runNotificationDataReflectionTest(test, {
run: function (scope) {
testRunner.simulateWebNotificationClick(scope, -1 /* action_index */);
testRunner.simulateWebNotificationClick(scope);
},
name: 'click'
});
......
......@@ -38,15 +38,15 @@
var expectedReplies = [];
// (3) Simulate some clicks on the notification, with and without replies.
// (3.1) Simulate a reply to the notification text action.
testRunner.simulateWebNotificationClickWithReply(scope, 0 /* action_index */, 'My reply.');
testRunner.simulateWebNotificationClick(scope, 0 /* action_index */, 'My reply.');
expectedReplies.push('My reply.');
// (3.2) Simulate an empty reply to the notification text action.
testRunner.simulateWebNotificationClickWithReply(scope, 0 /* action_index */, '');
testRunner.simulateWebNotificationClick(scope, 0 /* action_index */, '');
expectedReplies.push('');
// (3.3) Simulate a click on the notification body (reply should be null).
testRunner.simulateWebNotificationClick(scope, -1 /* action_index */);
testRunner.simulateWebNotificationClick(scope);
expectedReplies.push(null);
port.addEventListener('message', function(event) {
......
......@@ -58,7 +58,7 @@ t.step(function() {
if (message.type !== 'click')
return;
if (window.testRunner)
testRunner.simulateWebNotificationClick(message.title, -1 /* action_index */);
testRunner.simulateWebNotificationClick(message.title);
return;
}
......
......@@ -39,7 +39,7 @@ t.step(function() {
if (message.type !== 'click')
return;
if (window.testRunner)
testRunner.simulateWebNotificationClick(message.title, -1 /* action_index */);
testRunner.simulateWebNotificationClick(message.title);
return;
}
......
......@@ -39,7 +39,7 @@ t.step(function() {
if (message.type !== 'click')
return;
if (window.testRunner)
testRunner.simulateWebNotificationClick(message.title, -1 /* action_index */);
testRunner.simulateWebNotificationClick(message.title);
return;
}
......
......@@ -74,7 +74,7 @@ t.step(function() {
if (message.type !== 'click')
return;
if (window.testRunner)
testRunner.simulateWebNotificationClick(message.title, -1 /* action_index */);
testRunner.simulateWebNotificationClick(message.title);
return;
}
......
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