[Mojoification] Hook up DisplayNonPersistentNotification()

- When run with --enable-blink-features=NotificationsWithMojo,
notification-showing now goes through Mojo.

- Tested manually that a notification with the expected origin
and title displays when a non-persistent notification is shown.

- All other notification data still needs to be hooked up.

- Notification events are also not yet implemented, so this patch
updates the virtual TestExpectations to skip these/expect timeouts.

Bug: 595685
Change-Id: I3f74f0f434db1e639da1b753554e7c4776b58448
Reviewed-on: https://chromium-review.googlesource.com/788153
Commit-Queue: Peter Beverloo <peter@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Reviewed-by: Anita Woodruff <awdf@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522429}

content/browser/notifications/blink_notification_service_impl.cc

@@ -5,11 +5,13 @@
 #include "content/browser/notifications/blink_notification_service_impl.h"
 
 #include "base/logging.h"
+#include "base/strings/string16.h"
 #include "content/browser/notifications/platform_notification_context_impl.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/platform_notification_service.h"
 #include "content/public/common/content_client.h"
+#include "content/public/common/notification_resources.h"
 #include "third_party/WebKit/public/platform/modules/permissions/permission_status.mojom.h"
 #include "url/gurl.h"
 
@@ -26,22 +28,25 @@ PlatformNotificationService* Service() {
 
 BlinkNotificationServiceImpl::BlinkNotificationServiceImpl(
     PlatformNotificationContextImpl* notification_context,
+    BrowserContext* browser_context,
     ResourceContext* resource_context,
     int render_process_id,
     const url::Origin& origin,
     mojo::InterfaceRequest<blink::mojom::NotificationService> request)
     : notification_context_(notification_context),
+      browser_context_(browser_context),
       resource_context_(resource_context),
       render_process_id_(render_process_id),
       origin_(origin),
       binding_(this, std::move(request)) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(notification_context_);
+  DCHECK(browser_context_);
   DCHECK(resource_context_);
 
   binding_.set_connection_error_handler(base::BindOnce(
       &BlinkNotificationServiceImpl::OnConnectionError,
-      base::Unretained(this) /* the channel is owned by this */));
+      base::Unretained(this) /* the channel is owned by |this| */));
 }
 
 BlinkNotificationServiceImpl::~BlinkNotificationServiceImpl() {
@@ -68,4 +73,22 @@ void BlinkNotificationServiceImpl::OnConnectionError() {
   // |this| has now been deleted.
 }
 
+void BlinkNotificationServiceImpl::DisplayNonPersistentNotification(
+    const base::string16& title) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  if (!Service())
+    return;
+  PlatformNotificationData platform_notification_data;
+  platform_notification_data.title = title;
+  // TODO(crbug.com/595685): Plumb through the rest of the notification data and
+  // the notification resources from blink.
+  // Using base::Unretained is safe because Service() returns a singleton.
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::BindOnce(&PlatformNotificationService::DisplayNotification,
+                     base::Unretained(Service()), browser_context_, "",
+                     origin_.GetURL(), platform_notification_data,
+                     NotificationResources()));
+}
+
 }  // namespace content

content/browser/notifications/blink_notification_service_impl.h

@@ -6,6 +6,7 @@
 #define CONTENT_BROWSER_NOTIFICATIONS_BLINK_NOTIFICATION_SERVICE_IMPL_H_
 
 #include "base/macros.h"
+#include "content/public/browser/browser_context.h"
 #include "mojo/public/cpp/bindings/binding.h"
 #include "mojo/public/cpp/bindings/interface_request.h"
 #include "third_party/WebKit/public/platform/modules/notifications/notification_service.mojom.h"
@@ -23,6 +24,7 @@ class BlinkNotificationServiceImpl : public blink::mojom::NotificationService {
  public:
   BlinkNotificationServiceImpl(
       PlatformNotificationContextImpl* notification_context,
+      BrowserContext* browser_context,
       ResourceContext* resource_context,
       int render_process_id,
       const url::Origin& origin,
@@ -31,6 +33,7 @@ class BlinkNotificationServiceImpl : public blink::mojom::NotificationService {
 
   // blink::mojom::NotificationService implementation.
   void GetPermissionStatus(GetPermissionStatusCallback callback) override;
+  void DisplayNonPersistentNotification(const base::string16& title) override;
 
  private:
   // Called when an error is detected on binding_.
@@ -39,6 +42,8 @@ class BlinkNotificationServiceImpl : public blink::mojom::NotificationService {
   // The notification context that owns this service instance.
   PlatformNotificationContextImpl* notification_context_;
 
+  BrowserContext* browser_context_;
+
   ResourceContext* resource_context_;
 
   int render_process_id_;

content/browser/notifications/platform_notification_context_impl.cc

@@ -142,7 +142,8 @@ void PlatformNotificationContextImpl::CreateServiceOnIO(
     mojo::InterfaceRequest<blink::mojom::NotificationService> request) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   services_.push_back(std::make_unique<BlinkNotificationServiceImpl>(
-      this, resource_context, render_process_id, origin, std::move(request)));
+      this, browser_context_, resource_context, render_process_id, origin,
+      std::move(request)));
 }
 
 void PlatformNotificationContextImpl::RemoveService(

third_party/WebKit/LayoutTests/TestExpectations

@@ -1910,6 +1910,25 @@ crbug.com/711529 http/tests/permissions/test-query.html [ Timeout ]
 crbug.com/711529 http/tests/security/cross-origin-createImageBitmap-structured-clone.html [ Timeout ]
 crbug.com/711529 http/tests/workers/shared-worker-performance-timeline.html [ Timeout ]
 
+# Expected failures during incremental implementation of mojo notification.
+crbug.com/595685 virtual/mojo-notifications/http/tests/notifications/click-dedicated-worker.html [ Skip ]
+crbug.com/595685 virtual/mojo-notifications/http/tests/notifications/click-document.html [ Skip ]
+crbug.com/595685 virtual/mojo-notifications/http/tests/notifications/click-shared-worker.html [ Skip ]
+crbug.com/595685 virtual/mojo-notifications/http/tests/notifications/click-window-focus-document.html [ Skip ]
+crbug.com/595685 virtual/mojo-notifications/http/tests/notifications/close-dedicated-worker.html [ Skip ]
+crbug.com/595685 virtual/mojo-notifications/http/tests/notifications/close-dispatch-asynchronous.html [ Skip ]
+crbug.com/595685 virtual/mojo-notifications/http/tests/notifications/close-document.html [ Skip ]
+crbug.com/595685 virtual/mojo-notifications/http/tests/notifications/close-shared-worker.html [ Skip ]
+crbug.com/595685 virtual/mojo-notifications/http/tests/notifications/request-permission-granted.html [ Timeout ]
+crbug.com/595685 virtual/mojo-notifications/http/tests/notifications/serviceworkerregistration-page-notification-fetch-resources.html [ Timeout ]
+crbug.com/595685 virtual/mojo-notifications/http/tests/notifications/show-dedicated-worker.html [ Timeout ]
+crbug.com/595685 virtual/mojo-notifications/http/tests/notifications/show-document.html [ Timeout ]
+crbug.com/595685 virtual/mojo-notifications/http/tests/notifications/show-shared-worker.html [ Timeout ]
+crbug.com/595685 virtual/mojo-notifications/http/tests/notifications/update-dedicated-worker.html [ Timeout ]
+crbug.com/595685 virtual/mojo-notifications/http/tests/notifications/update-document.html [ Timeout ]
+crbug.com/595685 virtual/mojo-notifications/http/tests/notifications/update-shared-worker.html [ Timeout ]
+
+
 crbug.com/713587 external/wpt/css/css-ui/caret-color-006.html [ Skip ]
 
 # Automatically-added expectations for CSS tests which are temporarily

third_party/WebKit/LayoutTests/http/tests/notifications/notification-creation-detached-context.html

+<!doctype html>
+<html>
+  <head>
+    <title>Notifications: Creating a notification in a detached context should
+      throw a TypeError.</title>
+    <script src="../resources/testharness.js"></script>
+    <script src="../resources/testharnessreport.js"></script>
+    <script src="resources/test-helpers.js"></script>
+  </head>
+  <body>
+    <script>
+      if (window.testRunner) {
+          testRunner.setPermission('notifications', 'granted', location.origin, location.origin);
+          testRunner.setCanOpenWindows();
+      }
+
+      async_test(test => {
+          const remoteWindow = window.open('resources/window-detached-context.html');
+          let remoteNotificationObj = null;
+
+          window.addEventListener('message', test.step_func(event => {
+              switch (event.data) {
+                  case 'opened':
+                      assert_own_property(remoteWindow, 'Notification');
+                      remoteNotificationObj = remoteWindow.Notification;
+                      remoteWindow.close();
+                      break;
+
+                  case 'closed':
+                      try {
+                          new remoteNotificationObj('Title');
+                          assert_unreached('Expected a TypeError but no error thrown.');
+                      } catch (e) {
+                          assert_equals(e.name, 'TypeError');
+                          assert_equals(
+                              e.message, "Failed to construct 'Notification': Illegal invocation.");
+                      }
+                      test.done();
+                      break;
+                  default:
+                      assert_unreached('Unrecognized message from the opened window.');
+                      break;
+              }
+          }));
+
+      }, 'Creating a notification in a detached context should throw.');
+    </script>
+  </body>
+</html>

third_party/WebKit/LayoutTests/http/tests/notifications/request-permission-detached-context.html

@@ -16,7 +16,7 @@
       }
 
       async_test(test => {
-          const remoteWindow = window.open('resources/window-permission-detached-context.html');
+          const remoteWindow = window.open('resources/window-detached-context.html');
           let remoteNotificationObj = null;
 
           window.addEventListener('message', test.step_func(event => {

third_party/WebKit/Source/modules/notifications/Notification.cpp

@@ -118,6 +118,11 @@ Notification* Notification::Create(ExecutionContext* context,
   if (exception_state.HadException())
     return nullptr;
 
+  if (context->IsContextDestroyed()) {
+    exception_state.ThrowTypeError("Illegal invocation.");
+    return nullptr;
+  }
+
   Notification* notification =
       new Notification(context, Type::kNonPersistent, data);
   notification->SchedulePrepareShow();
@@ -186,11 +191,17 @@ void Notification::PrepareShow() {
 void Notification::DidLoadResources(NotificationResourcesLoader* loader) {
   DCHECK_EQ(loader, loader_.Get());
 
-  const SecurityOrigin* origin = GetExecutionContext()->GetSecurityOrigin();
+  ExecutionContext* execution_context = GetExecutionContext();
+  const SecurityOrigin* origin = execution_context->GetSecurityOrigin();
   DCHECK(origin);
 
-  GetWebNotificationManager()->Show(WebSecurityOrigin(origin), data_,
-                                    loader->GetResources(), this);
+  if (RuntimeEnabledFeatures::NotificationsWithMojoEnabled()) {
+    NotificationManager::From(execution_context)
+        ->DisplayNonPersistentNotification(data_.title);
+  } else {
+    GetWebNotificationManager()->Show(WebSecurityOrigin(origin), data_,
+                                      loader->GetResources(), this);
+  }
   loader_.Clear();
 
   state_ = State::kShowing;
@@ -209,7 +220,11 @@ void Notification::close() {
                                               WrapPersistent(this)));
     state_ = State::kClosing;
 
-    GetWebNotificationManager()->Close(this);
+    if (RuntimeEnabledFeatures::NotificationsWithMojoEnabled()) {
+      // TODO(crbug.com/595685): Implement Close path via Mojo.
+    } else {
+      GetWebNotificationManager()->Close(this);
+    }
     return;
   }
 

third_party/WebKit/Source/modules/notifications/NotificationManager.cpp

@@ -108,6 +108,12 @@ void NotificationManager::OnPermissionServiceConnectionError() {
   permission_service_.reset();
 }
 
+void NotificationManager::DisplayNonPersistentNotification(
+    const String& title) {
+  // TODO(crbug.com/595685): Pass the rest of the notification properties here.
+  GetNotificationService()->DisplayNonPersistentNotification(title);
+}
+
 const mojom::blink::NotificationServicePtr&
 NotificationManager::GetNotificationService() {
   if (!notification_service_) {

third_party/WebKit/Source/modules/notifications/NotificationManager.h

@@ -42,6 +42,9 @@ class NotificationManager final
       ScriptState*,
       NotificationPermissionCallback* deprecated_callback);
 
+  // Shows a notification that is not tied to any service worker.
+  void DisplayNonPersistentNotification(const String& title);
+
   virtual void Trace(blink::Visitor*);
 
  private:

third_party/WebKit/public/platform/modules/notifications/notification_service.mojom

@@ -4,6 +4,7 @@
 
 module blink.mojom;
 
+import "mojo/common/string16.mojom";
 import "third_party/WebKit/public/platform/modules/permissions/permission_status.mojom";
 
 // Service through which Blink can request notifications to be shown, closed or
@@ -13,4 +14,8 @@ interface NotificationService {
   // with the interface connection. Required to be synchronous due to the
   // Notification.permission JavaScript getter.
   [Sync] GetPermissionStatus() => (PermissionStatus status);
+
+  // Shows a notification that is not associated with a service worker.
+  // TODO(crbug.com/595685): Pass the rest of the notification data through.
+  DisplayNonPersistentNotification(mojo.common.mojom.String16 title);
 };