Update the PushEvent to have a nullable PushMessageData

The specification now allows a PushMessage to have valid non-empty data,
valid empty data, and null data. To support that, the content layer now uses
a PushEventPayload which can be null.

This also adds and updates relevant unit tests for each of the layers.

BUG=578036

Review URL: https://codereview.chromium.org/1636483002

Cr-Commit-Position: refs/heads/master@{#371839}

chrome/browser/push_messaging/push_messaging_service_impl.cc

@@ -217,14 +217,16 @@ void PushMessagingServiceImpl::OnMessage(const std::string& app_id,
       g_browser_process->rappor_service(),
       "PushMessaging.MessageReceived.Origin", app_identifier.origin());
 
-  std::string data;
+  // The payload of a push message can be valid with content, valid with empty
+  // content, or null. Only set the payload data if it is non-null.
+  content::PushEventPayload payload;
   if (AreMessagePayloadsEnabled() && message.decrypted)
-    data = message.raw_data;
+    payload.setData(message.raw_data);
 
   // Dispatch the message to the appropriate Service Worker.
   content::BrowserContext::DeliverPushMessage(
       profile_, app_identifier.origin(),
-      app_identifier.service_worker_registration_id(), data,
+      app_identifier.service_worker_registration_id(), payload,
       base::Bind(&PushMessagingServiceImpl::DeliverMessageCallback,
                  weak_factory_.GetWeakPtr(), app_identifier.app_id(),
                  app_identifier.origin(),
@@ -235,7 +237,7 @@ void PushMessagingServiceImpl::OnMessage(const std::string& app_id,
   if (!message_dispatched_callback_for_testing_.is_null()) {
     message_dispatched_callback_for_testing_.Run(
         app_id, app_identifier.origin(),
-        app_identifier.service_worker_registration_id(), data);
+        app_identifier.service_worker_registration_id(), payload);
   }
 }
 

chrome/browser/push_messaging/push_messaging_service_impl.h

@@ -24,6 +24,7 @@
 #include "components/keyed_service/core/keyed_service.h"
 #include "content/public/browser/push_messaging_service.h"
 #include "content/public/common/permission_status.mojom.h"
+#include "content/public/common/push_event_payload.h"
 #include "content/public/common/push_messaging_status.h"
 #include "third_party/WebKit/public/platform/modules/push_messaging/WebPushPermissionStatus.h"
 
@@ -213,7 +214,7 @@ class PushMessagingServiceImpl : public content::PushMessagingService,
       base::Callback<void(const std::string& app_id,
                           const GURL& origin,
                           int64_t service_worker_registration_id,
-                          const std::string& message_data)>;
+                          const content::PushEventPayload& payload)>;
 
   void SetMessageDispatchedCallbackForTesting(
       const MessageDispatchedCallback& callback) {

chrome/browser/push_messaging/push_messaging_service_unittest.cc

@@ -27,6 +27,7 @@
 #include "components/gcm_driver/fake_gcm_client_factory.h"
 #include "components/gcm_driver/gcm_profile_service.h"
 #include "content/public/common/content_switches.h"
+#include "content/public/common/push_event_payload.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -123,15 +124,15 @@ class PushMessagingServiceTest : public ::testing::Test {
   void DidDispatchMessage(std::string* app_id_out,
                           GURL* origin_out,
                           int64_t* service_worker_registration_id_out,
-                          std::string* message_data_out,
+                          content::PushEventPayload* payload_out,
                           const std::string& app_id,
                           const GURL& origin,
                           int64_t service_worker_registration_id,
-                          const std::string& message_data) {
+                          const content::PushEventPayload& payload) {
     *app_id_out = app_id;
     *origin_out = origin;
     *service_worker_registration_id_out = service_worker_registration_id;
-    *message_data_out = message_data;
+    *payload_out = payload;
   }
 
  protected:
@@ -193,17 +194,17 @@ TEST_F(PushMessagingServiceTest, PayloadEncryptionTest) {
 
   ASSERT_FALSE(app_identifier.is_null());
 
-  std::string app_id, message_data;
+  std::string app_id;
   GURL dispatched_origin;
   int64_t service_worker_registration_id;
+  content::PushEventPayload payload;
 
   // (5) Observe message dispatchings from the Push Messaging service, and then
   // dispatch the |message| on the GCM driver as if it had actually been
   // received by Google Cloud Messaging.
-  push_service->SetMessageDispatchedCallbackForTesting(
-      base::Bind(&PushMessagingServiceTest::DidDispatchMessage,
-                 base::Unretained(this), &app_id, &dispatched_origin,
-                 &service_worker_registration_id, &message_data));
+  push_service->SetMessageDispatchedCallbackForTesting(base::Bind(
+      &PushMessagingServiceTest::DidDispatchMessage, base::Unretained(this),
+      &app_id, &dispatched_origin, &service_worker_registration_id, &payload));
 
   gcm::FakeGCMProfileService* fake_profile_service =
       static_cast<gcm::FakeGCMProfileService*>(
@@ -220,5 +221,6 @@ TEST_F(PushMessagingServiceTest, PayloadEncryptionTest) {
   EXPECT_EQ(origin, dispatched_origin);
   EXPECT_EQ(service_worker_registration_id, kTestServiceWorkerId);
 
-  EXPECT_EQ(kTestPayload, message_data);
+  EXPECT_FALSE(payload.is_null);
+  EXPECT_EQ(kTestPayload, payload.data);
 }

content/browser/browser_context.cc

@@ -255,11 +255,12 @@ void BrowserContext::DeliverPushMessage(
     BrowserContext* browser_context,
     const GURL& origin,
     int64_t service_worker_registration_id,
-    const std::string& data,
+    const PushEventPayload& payload,
     const base::Callback<void(PushDeliveryStatus)>& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
-  PushMessagingRouter::DeliverMessage(
-      browser_context, origin, service_worker_registration_id, data, callback);
+  PushMessagingRouter::DeliverMessage(browser_context, origin,
+                                      service_worker_registration_id, payload,
+                                      callback);
 }
 
 // static

content/browser/devtools/protocol/service_worker_handler.cc

@@ -23,6 +23,7 @@
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/storage_partition.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/common/push_event_payload.h"
 #include "content/public/common/push_messaging_status.h"
 #include "url/gurl.h"
 
@@ -437,9 +438,12 @@ Response ServiceWorkerHandler::DeliverPushMessage(
   int64_t id = 0;
   if (!base::StringToInt64(registration_id, &id))
     return CreateInvalidVersionIdErrorResponse();
+  PushEventPayload payload;
+  if (data.size() > 0)
+    payload.setData(data);
   BrowserContext::DeliverPushMessage(
       render_frame_host_->GetProcess()->GetBrowserContext(), GURL(origin), id,
-      data, base::Bind(&PushDeliveryNoOp));
+      payload, base::Bind(&PushDeliveryNoOp));
   return Response::OK();
 }
 

content/browser/push_messaging/push_messaging_router.cc

@@ -15,6 +15,7 @@
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/storage_partition.h"
+#include "content/public/common/push_event_payload.h"
 
 namespace content {
 
@@ -36,7 +37,7 @@ void PushMessagingRouter::DeliverMessage(
     BrowserContext* browser_context,
     const GURL& origin,
     int64_t service_worker_registration_id,
-    const std::string& data,
+    const PushEventPayload& payload,
     const DeliverMessageCallback& deliver_message_callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   StoragePartition* partition =
@@ -47,15 +48,15 @@ void PushMessagingRouter::DeliverMessage(
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
       base::Bind(&PushMessagingRouter::FindServiceWorkerRegistration, origin,
-                 service_worker_registration_id, data, deliver_message_callback,
-                 service_worker_context));
+                 service_worker_registration_id, payload,
+                 deliver_message_callback, service_worker_context));
 }
 
 // static
 void PushMessagingRouter::FindServiceWorkerRegistration(
     const GURL& origin,
     int64_t service_worker_registration_id,
-    const std::string& data,
+    const PushEventPayload& payload,
     const DeliverMessageCallback& deliver_message_callback,
     scoped_refptr<ServiceWorkerContextWrapper> service_worker_context) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
@@ -64,12 +65,12 @@ void PushMessagingRouter::FindServiceWorkerRegistration(
   service_worker_context->FindReadyRegistrationForId(
       service_worker_registration_id, origin,
       base::Bind(&PushMessagingRouter::FindServiceWorkerRegistrationCallback,
-                 data, deliver_message_callback));
+                 payload, deliver_message_callback));
 }
 
 // static
 void PushMessagingRouter::FindServiceWorkerRegistrationCallback(
-    const std::string& data,
+    const PushEventPayload& payload,
     const DeliverMessageCallback& deliver_message_callback,
     ServiceWorkerStatusCode service_worker_status,
     const scoped_refptr<ServiceWorkerRegistration>&
@@ -91,8 +92,8 @@ void PushMessagingRouter::FindServiceWorkerRegistrationCallback(
   // service worker.
   version->RunAfterStartWorker(
       base::Bind(&PushMessagingRouter::DeliverMessageToWorker,
-                 make_scoped_refptr(version), service_worker_registration, data,
-                 deliver_message_callback),
+                 make_scoped_refptr(version), service_worker_registration,
+                 payload, deliver_message_callback),
       base::Bind(&PushMessagingRouter::DeliverMessageEnd,
                  deliver_message_callback, service_worker_registration));
 }
@@ -101,7 +102,7 @@ void PushMessagingRouter::FindServiceWorkerRegistrationCallback(
 void PushMessagingRouter::DeliverMessageToWorker(
     const scoped_refptr<ServiceWorkerVersion>& service_worker,
     const scoped_refptr<ServiceWorkerRegistration>& service_worker_registration,
-    const std::string& data,
+    const PushEventPayload& payload,
     const DeliverMessageCallback& deliver_message_callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   int request_id = service_worker->StartRequest(
@@ -109,7 +110,7 @@ void PushMessagingRouter::DeliverMessageToWorker(
       base::Bind(&PushMessagingRouter::DeliverMessageEnd,
                  deliver_message_callback, service_worker_registration));
   service_worker->DispatchSimpleEvent<ServiceWorkerHostMsg_PushEventFinished>(
-      request_id, ServiceWorkerMsg_PushEvent(request_id, data));
+      request_id, ServiceWorkerMsg_PushEvent(request_id, payload));
 }
 
 // static

content/browser/push_messaging/push_messaging_router.h

@@ -18,6 +18,7 @@
 namespace content {
 
 class BrowserContext;
+struct PushEventPayload;
 class ServiceWorkerContextWrapper;
 class ServiceWorkerRegistration;
 class ServiceWorkerVersion;
@@ -33,7 +34,7 @@ class PushMessagingRouter {
       BrowserContext* browser_context,
       const GURL& origin,
       int64_t service_worker_registration_id,
-      const std::string& data,
+      const PushEventPayload& payload,
       const DeliverMessageCallback& deliver_message_callback);
 
  private:
@@ -42,7 +43,7 @@ class PushMessagingRouter {
   static void FindServiceWorkerRegistration(
       const GURL& origin,
       int64_t service_worker_registration_id,
-      const std::string& data,
+      const PushEventPayload& payload,
       const DeliverMessageCallback& deliver_message_callback,
       scoped_refptr<ServiceWorkerContextWrapper> service_worker_context);
 
@@ -50,7 +51,7 @@ class PushMessagingRouter {
   // |data| on the Service Worker identified by |service_worker_registration|.
   // Must be called on the IO thread.
   static void FindServiceWorkerRegistrationCallback(
-      const std::string& data,
+      const PushEventPayload& payload,
       const DeliverMessageCallback& deliver_message_callback,
       ServiceWorkerStatusCode service_worker_status,
       const scoped_refptr<ServiceWorkerRegistration>&
@@ -62,7 +63,7 @@ class PushMessagingRouter {
       const scoped_refptr<ServiceWorkerVersion>& service_worker,
       const scoped_refptr<ServiceWorkerRegistration>&
           service_worker_registration,
-      const std::string& data,
+      const PushEventPayload& payload,
       const DeliverMessageCallback& deliver_message_callback);
 
   // Gets called asynchronously after the Service Worker has dispatched the push

content/browser/service_worker/embedded_worker_test_helper.cc

@@ -20,6 +20,7 @@
 #include "content/common/service_worker/embedded_worker_messages.h"
 #include "content/common/service_worker/embedded_worker_setup.mojom.h"
 #include "content/common/service_worker/service_worker_messages.h"
+#include "content/public/common/push_event_payload.h"
 #include "content/public/test/mock_render_process_host.h"
 #include "content/public/test/test_browser_context.h"
 #include "mojo/public/cpp/bindings/strong_binding.h"
@@ -224,7 +225,7 @@ void EmbeddedWorkerTestHelper::OnFetchEvent(
 
 void EmbeddedWorkerTestHelper::OnPushEvent(int embedded_worker_id,
                                            int request_id,
-                                           const std::string& data) {
+                                           const PushEventPayload& payload) {
   SimulateSend(new ServiceWorkerHostMsg_PushEventFinished(
       embedded_worker_id, request_id,
       blink::WebServiceWorkerEventResultCompleted));
@@ -372,12 +373,13 @@ void EmbeddedWorkerTestHelper::OnFetchEventStub(
                  request));
 }
 
-void EmbeddedWorkerTestHelper::OnPushEventStub(int request_id,
-                                               const std::string& data) {
+void EmbeddedWorkerTestHelper::OnPushEventStub(
+    int request_id,
+    const PushEventPayload& payload) {
   base::ThreadTaskRunnerHandle::Get()->PostTask(
       FROM_HERE, base::Bind(&EmbeddedWorkerTestHelper::OnPushEvent,
                             weak_factory_.GetWeakPtr(),
-                            current_embedded_worker_id_, request_id, data));
+                            current_embedded_worker_id_, request_id, payload));
 }
 
 void EmbeddedWorkerTestHelper::OnSetupMojoStub(

content/browser/service_worker/embedded_worker_test_helper.h

@@ -30,6 +30,7 @@ class EmbeddedWorkerRegistry;
 class EmbeddedWorkerTestHelper;
 class MessagePortMessageFilter;
 class MockRenderProcessHost;
+struct PushEventPayload;
 class ServiceWorkerContextCore;
 class ServiceWorkerContextWrapper;
 struct ServiceWorkerFetchRequest;
@@ -125,7 +126,7 @@ class EmbeddedWorkerTestHelper : public IPC::Sender,
                             const ServiceWorkerFetchRequest& request);
   virtual void OnPushEvent(int embedded_worker_id,
                            int request_id,
-                           const std::string& data);
+                           const PushEventPayload& payload);
 
   // These functions simulate sending an EmbeddedHostMsg message to the
   // browser.
@@ -152,7 +153,7 @@ class EmbeddedWorkerTestHelper : public IPC::Sender,
   void OnInstallEventStub(int request_id);
   void OnFetchEventStub(int request_id,
                         const ServiceWorkerFetchRequest& request);
-  void OnPushEventStub(int request_id, const std::string& data);
+  void OnPushEventStub(int request_id, const PushEventPayload& payload);
   void OnSetupMojoStub(int thread_id,
                        mojo::InterfaceRequest<mojo::ServiceProvider> services,
                        mojo::ServiceProviderPtr exposed_services);

content/common/service_worker/service_worker_messages.h

@@ -16,6 +16,7 @@
 #include "content/public/common/message_port_types.h"
 #include "content/public/common/navigator_connect_client.h"
 #include "content/public/common/platform_notification_data.h"
+#include "content/public/common/push_event_payload.h"
 #include "ipc/ipc_message_macros.h"
 #include "ipc/ipc_param_traits.h"
 #include "third_party/WebKit/public/platform/WebCircularGeofencingRegion.h"
@@ -133,6 +134,11 @@ IPC_STRUCT_TRAITS_BEGIN(content::NavigatorConnectClient)
   IPC_STRUCT_TRAITS_MEMBER(message_port_id)
 IPC_STRUCT_TRAITS_END()
 
+IPC_STRUCT_TRAITS_BEGIN(content::PushEventPayload)
+  IPC_STRUCT_TRAITS_MEMBER(data)
+  IPC_STRUCT_TRAITS_MEMBER(is_null)
+IPC_STRUCT_TRAITS_END()
+
 //---------------------------------------------------------------------------
 // Messages sent from the child process to the browser.
 
@@ -445,7 +451,7 @@ IPC_MESSAGE_CONTROL4(ServiceWorkerMsg_NotificationClickEvent,
                      int /* action_index */)
 IPC_MESSAGE_CONTROL2(ServiceWorkerMsg_PushEvent,
                      int /* request_id */,
-                     std::string /* data */)
+                     content::PushEventPayload /* data */)
 IPC_MESSAGE_CONTROL4(ServiceWorkerMsg_GeofencingEvent,
                      int /* request_id */,
                      blink::WebGeofencingEventType /* event_type */,

content/public/browser/browser_context.h

@@ -14,6 +14,7 @@
 #include "base/supports_user_data.h"
 #include "content/common/content_export.h"
 #include "content/public/browser/zoom_level_delegate.h"
+#include "content/public/common/push_event_payload.h"
 #include "content/public/common/push_messaging_status.h"
 
 class GURL;
@@ -108,7 +109,7 @@ class CONTENT_EXPORT BrowserContext : public base::SupportsUserData {
       BrowserContext* browser_context,
       const GURL& origin,
       int64_t service_worker_registration_id,
-      const std::string& data,
+      const PushEventPayload& payload,
       const base::Callback<void(PushDeliveryStatus)>& callback);
 
   static void NotifyWillBeDestroyed(BrowserContext* browser_context);

content/public/common/push_event_payload.h

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CONTENT_PUBLIC_COMMON_PUSH_EVENT_PAYLOAD_H_
+#define CONTENT_PUBLIC_COMMON_PUSH_EVENT_PAYLOAD_H_
+
+#include <string>
+
+#include "content/common/content_export.h"
+
+namespace content {
+
+// Structure representing the payload delivered as part of a push message.
+// This struct contains the decrypted information sent from the push
+// service as part of a PushEvent as well as metadata about the information.
+struct CONTENT_EXPORT PushEventPayload {
+  PushEventPayload() : is_null(true) {}
+  ~PushEventPayload() {}
+
+  // Method to both set the data string and update the null status.
+  void setData(const std::string& data_in) {
+    data = data_in;
+    is_null = false;
+  }
+
+  // Data contained in the payload.
+  std::string data;
+
+  // Whether the payload is null or not. Payloads can be valid with non-empty
+  // content, valid with empty content, or null.
+  bool is_null;
+};
+
+}  // namespace content
+
+#endif  // CONTENT_PUBLIC_COMMON_PUSH_EVENT_PAYLOAD_H_

content/renderer/service_worker/service_worker_context_client.cc

@@ -32,6 +32,7 @@
 #include "content/common/mojo/service_registry_impl.h"
 #include "content/common/service_worker/embedded_worker_messages.h"
 #include "content/common/service_worker/service_worker_messages.h"
+#include "content/public/common/push_event_payload.h"
 #include "content/public/common/referrer.h"
 #include "content/public/renderer/content_renderer_client.h"
 #include "content/public/renderer/document_state.h"
@@ -760,10 +761,14 @@ void ServiceWorkerContextClient::OnNotificationClickEvent(
 }
 
 void ServiceWorkerContextClient::OnPushEvent(int request_id,
-                                             const std::string& data) {
+                                             const PushEventPayload& payload) {
   TRACE_EVENT0("ServiceWorker",
                "ServiceWorkerContextClient::OnPushEvent");
-  proxy_->dispatchPushEvent(request_id, blink::WebString::fromUTF8(data));
+  // Only set data to be a valid string if the payload had decrypted data.
+  blink::WebString data;
+  if (!payload.is_null)
+    data.assign(blink::WebString::fromUTF8(payload.data));
+  proxy_->dispatchPushEvent(request_id, data);
 }
 
 void ServiceWorkerContextClient::OnGeofencingEvent(

content/renderer/service_worker/service_worker_context_client.h

@@ -53,6 +53,7 @@ namespace content {
 
 struct NavigatorConnectClient;
 struct PlatformNotificationData;
+struct PushEventPayload;
 struct ServiceWorkerClientInfo;
 class ServiceWorkerProviderContext;
 class ServiceWorkerContextClient;
@@ -192,7 +193,7 @@ class ServiceWorkerContextClient
       int64_t persistent_notification_id,
       const PlatformNotificationData& notification_data,
       int action_index);
-  void OnPushEvent(int request_id, const std::string& data);
+  void OnPushEvent(int request_id, const PushEventPayload& payload);
   void OnGeofencingEvent(int request_id,
                          blink::WebGeofencingEventType event_type,
                          const std::string& region_id,

third_party/WebKit/Source/modules/modules.gypi

@@ -1906,6 +1906,7 @@
       'mediastream/RTCDataChannelTest.cpp',
       'notifications/NotificationDataTest.cpp',
       'presentation/PresentationAvailabilityTest.cpp',
+      'push_messaging/PushMessageDataTest.cpp',
       'serviceworkers/ServiceWorkerContainerTest.cpp',
       'webaudio/AudioBasicProcessorHandlerTest.cpp',
       'webaudio/ConvolverNodeTest.cpp',

third_party/WebKit/Source/modules/push_messaging/PushMessageData.cpp

@@ -18,6 +18,11 @@ namespace blink {
 
 PushMessageData* PushMessageData::create(const String& messageString)
 {
+    // The standard supports both an empty but valid message and a null message.
+    // In case the message is explicitly null, return a null pointer which will
+    // be set in the PushEvent.
+    if (messageString.isNull())
+        return nullptr;
     return PushMessageData::create(ArrayBufferOrArrayBufferViewOrUSVString::fromUSVString(messageString));
 }
 

third_party/WebKit/Source/modules/push_messaging/PushMessageDataTest.cpp

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "modules/push_messaging/PushMessageData.h"
+
+#include "public/platform/WebString.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace blink {
+namespace {
+
+const char kPushMessageData[] = "Push Message valid data string.";
+
+TEST(PushMessageDataTest, ValidPayload)
+{
+    // Create a WebString with the test message, then create a
+    // PushMessageData from that.
+    WebString s(blink::WebString::fromUTF8(kPushMessageData));
+    PushMessageData* data = PushMessageData::create(s);
+
+    ASSERT_NE(data, nullptr);
+    EXPECT_STREQ(kPushMessageData, data->text().utf8().data());
+}
+
+TEST(PushMessageDataTest, ValidEmptyPayload)
+{
+    // Create a WebString with a valid but empty test message, then create
+    // a PushMessageData from that.
+    WebString s("");
+    PushMessageData* data = PushMessageData::create(s);
+
+    ASSERT_NE(data, nullptr);
+    EXPECT_STREQ("", data->text().utf8().data());
+}
+
+TEST(PushMessageDataTest, NullPayload)
+{
+    // Create a PushMessageData with a null payload.
+    WebString s;
+    PushMessageData* data = PushMessageData::create(s);
+
+    EXPECT_EQ(data, nullptr);
+}
+
+} // anonymous namespace
+} // namespace blink