MacPWAs: Create NSAlerts in the appropriate process

Split the Cocoa-facing parts of JavaScriptAppModalDialogCocoa into a
separate views_bridge_mac::AlertBridge class, implementing a mojo
interface of the same name.

The mojo interface has a single method, Show, which has a single
callback, indicating the disposition of the alert.

Make JavaScriptAppModalDialogCocoa instantiate
views_bridge_mac::AlertBridge across mojo either in-process or
out-of-process as needed. Note that this subtly changes the behavior of
JavaScriptAppModalDialogCocoa::AcceptAppModalDialog in that it will no
longer attempt to read the dialog's text when programmatically
quitting (note that this feature may be dead code).

This makes alerts appear in the appropriate process. Note that the
alerts of PWAs and of Chrome are still interconnected. Only one alert
may be present at once, and an active alert will take focus from Chrome
and all PWAs.

Bug: 898604
Change-Id: I37267cf7aa41d8e68cb2d342a5058e257b2d4385
Reviewed-on: https://chromium-review.googlesource.com/c/1390757Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: ccameron <ccameron@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619812}

chrome/browser/ui/BUILD.gn

@@ -22,13 +22,6 @@ import("//rlz/buildflags/buildflags.gni")
 import("//ui/base/ui_features.gni")
 import("//ui/views/features.gni")
 
-config("ui_warnings") {
-  if (is_mac) {
-    # TODO(thakis): Remove this once http://crbug.com/383820 is figured out
-    cflags = [ "-Wno-nonnull" ]
-  }
-}
-
 # Use a static library here because many test binaries depend on this but don't
 # require many files from it. This makes linking more efficient.
 jumbo_split_static_library("ui") {
@@ -347,7 +340,6 @@ jumbo_split_static_library("ui") {
   libs = []
 
   configs += [
-    ":ui_warnings",
     "//build/config:precompiled_headers",
     "//build/config/compiler:wexit_time_destructors",
   ]
@@ -2120,6 +2112,7 @@ jumbo_split_static_library("ui") {
       "//third_party/google_toolbox_for_mac",
       "//third_party/mozilla",
       "//ui/accelerated_widget_mac:accelerated_widget_mac",
+      "//ui/views_bridge_mac",
     ]
     include_dirs = [ "$target_gen_dir" ]
     libs += [

chrome/browser/ui/cocoa/javascript_app_modal_dialog_cocoa.h

@@ -10,7 +10,10 @@
 #include "base/logging.h"
 #include "base/mac/scoped_nsobject.h"
 #include "base/macros.h"
+#include "base/memory/weak_ptr.h"
 #include "components/app_modal/native_app_modal_dialog.h"
+#include "ui/views_bridge_mac/alert.h"
+#include "ui/views_bridge_mac/mojo/alert.mojom.h"
 
 class PopunderPreventer;
 
@@ -18,19 +21,10 @@ namespace app_modal {
 class JavaScriptAppModalDialog;
 }
 
-#if __OBJC__
-@class NSAlert;
-@class JavaScriptAppModalDialogHelper;
-#else
-class NSAlert;
-class JavaScriptAppModalDialogHelper;
-#endif
-
 class JavaScriptAppModalDialogCocoa : public app_modal::NativeAppModalDialog {
  public:
   explicit JavaScriptAppModalDialogCocoa(
       app_modal::JavaScriptAppModalDialog* dialog);
-  ~JavaScriptAppModalDialogCocoa() override;
 
   // Overridden from NativeAppModalDialog:
   int GetAppModalDialogButtons() const override;
@@ -46,17 +40,30 @@ class JavaScriptAppModalDialogCocoa : public app_modal::NativeAppModalDialog {
   }
 
  private:
-  // Returns the NSAlert associated with the modal dialog.
-  NSAlert* GetAlert() const;
+  ~JavaScriptAppModalDialogCocoa() override;
+
+  // Return the parameters to use for the alert.
+  views_bridge_mac::mojom::AlertBridgeInitParamsPtr GetAlertParams();
+
+  // Called when the alert completes. Deletes |this|.
+  void OnAlertFinished(views_bridge_mac::mojom::AlertDisposition disposition,
+                       const base::string16& prompt_text,
+                       bool suppress_js_messages);
+
+  // Called if there is an error connecting to the alert process. Deletes
+  // |this|.
+  void OnConnectionError();
+
+  // Mojo interface to the NSAlert.
+  views_bridge_mac::mojom::AlertBridgePtr alert_bridge_;
 
   std::unique_ptr<app_modal::JavaScriptAppModalDialog> dialog_;
   std::unique_ptr<PopunderPreventer> popunder_preventer_;
 
-  // Created in the constructor and destroyed in the destructor.
-  base::scoped_nsobject<JavaScriptAppModalDialogHelper> helper_;
-
-  bool is_showing_;
+  int num_buttons_ = 0;
+  bool is_showing_ = false;
 
+  base::WeakPtrFactory<JavaScriptAppModalDialogCocoa> weak_factory_;
   DISALLOW_COPY_AND_ASSIGN(JavaScriptAppModalDialogCocoa);
 };
 

ui/views_bridge_mac/BUILD.gn

@@ -4,10 +4,20 @@
 
 import("//mojo/public/tools/bindings/mojom.gni")
 
+config("views_bridge_mac_warnings") {
+  if (is_mac) {
+    # TODO(thakis): Remove this once http://crbug.com/383820 is figured out
+    cflags = [ "-Wno-nonnull" ]
+  }
+}
+
 component("views_bridge_mac") {
   assert(is_mac)
 
+  configs += [ ":views_bridge_mac_warnings" ]
   sources = [
+    "alert.h",
+    "alert.mm",
     "bridged_native_widget_host_helper.h",
     "cocoa_mouse_capture.h",
     "cocoa_mouse_capture.mm",
@@ -17,7 +27,11 @@ component("views_bridge_mac") {
   ]
   defines = [ "VIEWS_BRIDGE_MAC_IMPLEMENTATION" ]
   deps = [
+    ":mojo",
     "//base",
+    "//base:i18n",
+    "//mojo/public/cpp/bindings",
+    "//ui/accelerated_widget_mac",
     "//ui/base",
     "//ui/events",
     "//ui/gfx",
@@ -29,6 +43,7 @@ mojom("mojo") {
   assert(is_mac)
 
   sources = [
+    "mojo/alert.mojom",
     "mojo/bridge_factory.mojom",
     "mojo/bridged_native_widget.mojom",
     "mojo/bridged_native_widget_host.mojom",

ui/views_bridge_mac/alert.h

+// Copyright 2019 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 UI_VIEWS_BRIDGE_MAC_ALERT_H_
+#define UI_VIEWS_BRIDGE_MAC_ALERT_H_
+
+#import <Cocoa/Cocoa.h>
+
+#include "base/mac/scoped_nsobject.h"
+#include "mojo/public/cpp/bindings/binding.h"
+#include "ui/gfx/text_elider.h"
+#include "ui/views_bridge_mac/mojo/alert.mojom.h"
+#include "ui/views_bridge_mac/views_bridge_mac_export.h"
+
+@class AlertBridgeHelper;
+
+namespace views_bridge_mac {
+
+// Class that displays an NSAlert with associated UI as described by the mojo
+// AlertBridge interface.
+class VIEWS_BRIDGE_MAC_EXPORT AlertBridge
+    : public views_bridge_mac::mojom::AlertBridge {
+ public:
+  // Creates a new alert which controls its own lifetime. It will destroy itself
+  // once its NSAlert goes away.
+  AlertBridge(mojom::AlertBridgeRequest bridge_request);
+
+  // Send the specified disposition via the Show callback, then destroy |this|.
+  void SendResultAndDestroy(mojom::AlertDisposition disposition);
+
+  // Called by Cocoa to indicate when the NSAlert is visible (and can be
+  // programmatically updated by Accept, Cancel, and Close).
+  void SetAlertHasShown();
+
+ private:
+  // Private destructor is called only through SendResultAndDestroy.
+  ~AlertBridge() override;
+
+  // Handle being disconnected (e.g, because the alert was programmatically
+  // dismissed).
+  void OnConnectionError();
+
+  // views_bridge_mac::mojom::Alert:
+  void Show(mojom::AlertBridgeInitParamsPtr params,
+            ShowCallback callback) override;
+
+  // The NSAlert's owner and delegate.
+  base::scoped_nsobject<AlertBridgeHelper> helper_;
+
+  // Set once the alert window is showing (needed because showing is done in a
+  // posted task).
+  bool alert_shown_ = false;
+
+  // The callback to make when the dialog has finished running.
+  ShowCallback callback_;
+
+  mojo::Binding<views_bridge_mac::mojom::AlertBridge> mojo_binding_;
+  base::WeakPtrFactory<AlertBridge> weak_factory_;
+  DISALLOW_COPY_AND_ASSIGN(AlertBridge);
+};
+
+}  // namespace views_bridge_mac
+
+#endif  // UI_VIEWS_BRIDGE_MAC_ALERT_H_

ui/views_bridge_mac/bridge_factory_impl.h

@@ -7,6 +7,7 @@
 
 #include "mojo/public/cpp/bindings/associated_binding.h"
 #include "ui/views/views_export.h"
+#include "ui/views_bridge_mac/mojo/alert.mojom.h"
 #include "ui/views_bridge_mac/mojo/bridge_factory.mojom.h"
 #include "ui/views_bridge_mac/mojo/bridged_native_widget.mojom.h"
 #include "ui/views_bridge_mac/mojo/bridged_native_widget_host.mojom.h"
@@ -23,6 +24,7 @@ class VIEWS_EXPORT BridgeFactoryImpl : public mojom::BridgeFactory {
   void BindRequest(mojom::BridgeFactoryAssociatedRequest request);
 
   // mojom::BridgeFactory:
+  void CreateAlert(mojom::AlertBridgeRequest bridge_request) override;
   void CreateBridgedNativeWidget(
       uint64_t bridge_id,
       mojom::BridgedNativeWidgetAssociatedRequest bridge_request,

ui/views_bridge_mac/bridge_factory_impl.mm

@@ -7,6 +7,7 @@
 #include "base/no_destructor.h"
 #include "ui/accelerated_widget_mac/window_resize_helper_mac.h"
 #include "ui/base/cocoa/remote_accessibility_api.h"
+#include "ui/views_bridge_mac/alert.h"
 #include "ui/views_bridge_mac/bridged_native_widget_host_helper.h"
 #include "ui/views_bridge_mac/bridged_native_widget_impl.h"
 
@@ -104,6 +105,11 @@ void BridgeFactoryImpl::BindRequest(
                 ui::WindowResizeHelperMac::Get()->task_runner());
 }
 
+void BridgeFactoryImpl::CreateAlert(mojom::AlertBridgeRequest bridge_request) {
+  // The resulting object manages its own lifetime.
+  ignore_result(new AlertBridge(std::move(bridge_request)));
+}
+
 void BridgeFactoryImpl::CreateBridgedNativeWidget(
     uint64_t bridge_id,
     mojom::BridgedNativeWidgetAssociatedRequest bridge_request,

ui/views_bridge_mac/mojo/alert.mojom

+// Copyright 2019 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.
+
+module views_bridge_mac.mojom;
+
+import "mojo/public/mojom/base/string16.mojom";
+
+struct AlertBridgeInitParams {
+  // The dialog title and text.
+  mojo_base.mojom.String16 title;
+  mojo_base.mojom.String16 message_text;
+
+  // Set if the application icon should be hidden.
+  bool hide_application_icon;
+
+  // Text for the primary button (which is also the default button).
+  mojo_base.mojom.String16 primary_button_text;
+
+  // Text for the secondary (non-default) button. If not set, then there is only
+  // one button for this alert.
+  mojo_base.mojom.String16? secondary_button_text;
+
+  // Default text for the text field. If not set, then there is no text field
+  // for this alert.
+  mojo_base.mojom.String16? text_field_text;
+
+  // The text for the checkbox. If not set, then there is no checkbox for this
+  // alert.
+  mojo_base.mojom.String16? check_box_text;
+};
+
+// Disposition of alert window.
+enum AlertDisposition {
+  // Default button was pressed.
+  PRIMARY_BUTTON,
+  // Secondary button was pressed.
+  SECONDARY_BUTTON,
+  // The window was closed without a selection being made.
+  CLOSE,
+};
+
+interface AlertBridge {
+  // Initialize and show the alert. Return in |disposition| is how the window
+  // was dismissed. Return in |text_field_value| the value of the text field
+  // shown in the alert (if any). Return true in |check_box_value| only if the
+  // alert had a checkbox and it was checked.
+  Show(AlertBridgeInitParams params) =>
+      (AlertDisposition disposition, mojo_base.mojom.String16 text_field_value,
+       bool check_box_value);
+};

ui/views_bridge_mac/mojo/bridge_factory.mojom

@@ -4,11 +4,15 @@
 
 module views_bridge_mac.mojom;
 
+import "ui/views_bridge_mac/mojo/alert.mojom";
 import "ui/views_bridge_mac/mojo/bridged_native_widget.mojom";
 import "ui/views_bridge_mac/mojo/bridged_native_widget_host.mojom";
 
 // The interface through which a bridge is created and connected to its host.
 interface BridgeFactory {
+  // Create a bridge for an NSAlert. The resulting object owns its own lifetime.
+  CreateAlert(AlertBridge& alert_bridge_request);
+
   // Create a bridge for a native widget. The resulting object will be owned by
   // the connection for |host|. Closing that connection will result in deleting
   // the bridge.