[remoting][mobile] Remove TOAST notification appearance

Dialog notifications now support "Don't show again" checkbox, and it is
deemed nontrivial to implement toast with action button on Android, so
the usefulness and maintenance difficulty of having the TOAST appearance
is questionable. This CL removes the TOAST appearance and the appearance
enum altogether.

Bug: 1001291
Change-Id: I2fe082a80fd9f84dbda96faa453ef99d732e9dda
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1848412Reviewed-by: Yuwei Huang <yuweih@chromium.org>
Reviewed-by: Jamie Walch <jamiewalch@chromium.org>
Commit-Queue: Yuwei Huang <yuweih@chromium.org>
Cr-Commit-Position: refs/heads/master@{#703960}

remoting/android/client_java_tmpl.gni

@@ -98,10 +98,7 @@ template("remoting_android_client_java_tmpl") {
 
     annotation_processor_deps = [ "//base/android/jni_generator:jni_processor" ]
 
-    srcjar_deps = [
-      "//remoting/client/jni:jni_enums",
-      "//remoting/client/notification:notification_enums",
-    ]
+    srcjar_deps = [ "//remoting/client/jni:jni_enums" ]
 
     if (defined(invoker.play_services_package)) {
       deps += [

remoting/android/java/src/org/chromium/chromoting/jni/NotificationPresenter.java

@@ -15,7 +15,6 @@ import androidx.annotation.IntDef;
 import org.chromium.base.annotations.CalledByNative;
 import org.chromium.base.annotations.JNINamespace;
 import org.chromium.base.annotations.NativeMethods;
-import org.chromium.chromoting.NotificationAppearance;
 import org.chromium.chromoting.Preconditions;
 import org.chromium.chromoting.R;
 
@@ -66,8 +65,7 @@ public final class NotificationPresenter {
     }
 
     @CalledByNative
-    void onNotificationFetched(@NotificationAppearance int appearance, String messageText,
-            String linkText, String linkUrl) {
+    void onNotificationFetched(String messageText, String linkText, String linkUrl) {
         Preconditions.isTrue(mState == State.FETCHING);
         mState = State.FETCHED;
 

remoting/client/jni/jni_notification_presenter.cc

@@ -57,14 +57,12 @@ void JniNotificationPresenter::OnNotificationFetched(
     Java_NotificationPresenter_onNoNotification(env, java_presenter);
     return;
   }
-  jint j_appearance = static_cast<jint>(notification->appearance);
   auto j_message_text =
       ConvertUTF8ToJavaString(env, notification->message_text);
   auto j_link_text = ConvertUTF8ToJavaString(env, notification->link_text);
   auto j_link_url = ConvertUTF8ToJavaString(env, notification->link_url);
-  Java_NotificationPresenter_onNotificationFetched(env, java_presenter,
-                                                   j_appearance, j_message_text,
-                                                   j_link_text, j_link_url);
+  Java_NotificationPresenter_onNotificationFetched(
+      env, java_presenter, j_message_text, j_link_text, j_link_url);
 }
 
 static jlong JNI_NotificationPresenter_Init(

remoting/client/notification/BUILD.gn

@@ -2,11 +2,6 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
-if (is_android) {
-  import("//build/config/android/config.gni")
-  import("//build/config/android/rules.gni")
-}
-
 source_set("notification") {
   sources = [
     "gstatic_json_fetcher.cc",
@@ -30,14 +25,6 @@ source_set("notification") {
   ]
 }
 
-if (is_android) {
-  java_cpp_enum("notification_enums") {
-    sources = [
-      "notification_message.h",
-    ]
-  }
-}
-
 source_set("unit_tests") {
   testonly = true
 

remoting/client/notification/notification_client.cc

@@ -254,7 +254,6 @@ base::Optional<NotificationMessage> NotificationClient::ParseAndMatchRule(
     const std::string& user_email,
     std::string* out_message_text_filename,
     std::string* out_link_text_filename) {
-  std::string appearance_string;
   std::string target_platform;
   std::string version_spec_string;
   std::string message_id;
@@ -262,8 +261,7 @@ base::Optional<NotificationMessage> NotificationClient::ParseAndMatchRule(
   std::string link_text_filename;
   std::string link_url;
   int percent;
-  if (!FindKeyAndGet(rule, "appearance", &appearance_string) ||
-      !FindKeyAndGet(rule, "target_platform", &target_platform) ||
+  if (!FindKeyAndGet(rule, "target_platform", &target_platform) ||
       !FindKeyAndGet(rule, "version", &version_spec_string) ||
       !FindKeyAndGet(rule, "message_id", &message_id) ||
       !FindKeyAndGet(rule, "message_text", &message_text_filename) ||
@@ -304,14 +302,6 @@ base::Optional<NotificationMessage> NotificationClient::ParseAndMatchRule(
   }
 
   auto message = base::make_optional<NotificationMessage>();
-  if (appearance_string == "TOAST") {
-    message->appearance = NotificationMessage::Appearance::TOAST;
-  } else if (appearance_string == "DIALOG") {
-    message->appearance = NotificationMessage::Appearance::DIALOG;
-  } else {
-    LOG(ERROR) << "Unknown appearance: " << appearance_string;
-    return base::nullopt;
-  }
   message->message_id = message_id;
   message->link_url = link_url;
   message->allow_dont_show_again = false;

remoting/client/notification/notification_client_unittest.cc

@@ -45,8 +45,7 @@ MATCHER(NoMessage, "") {
 }
 
 MATCHER_P(MessageMatches, expected, "") {
-  return arg->appearance == expected.appearance &&
-         arg->message_id == expected.message_id &&
+  return arg->message_id == expected.message_id &&
          arg->message_text == expected.message_text &&
          arg->link_text == expected.link_text &&
          arg->link_url == expected.link_url;
@@ -59,7 +58,6 @@ decltype(auto) ReturnByMove(T t) {
 
 base::Value CreateDefaultRule() {
   base::Value rule(base::Value::Type::DICTIONARY);
-  rule.SetStringKey("appearance", "TOAST");
   rule.SetStringKey("target_platform", "IOS");
   rule.SetStringKey("version", "[75-)");
   rule.SetStringKey("message_id", "test_message");
@@ -80,7 +78,6 @@ base::Value CreateDefaultTranslations(const std::string& text) {
 
 NotificationMessage CreateDefaultNotification() {
   NotificationMessage message;
-  message.appearance = NotificationMessage::Appearance::TOAST;
   message.message_id = "test_message";
   message.message_text = "zh-CN:message";
   message.link_text = "zh-CN:link";

remoting/client/notification/notification_message.h

@@ -10,13 +10,6 @@
 namespace remoting {
 
 struct NotificationMessage final {
-  // GENERATED_JAVA_ENUM_PACKAGE: org.chromium.chromoting
-  // GENERATED_JAVA_CLASS_NAME_OVERRIDE: NotificationAppearance
-  enum class Appearance {
-    TOAST,
-    DIALOG,
-  };
-
   NotificationMessage();
   NotificationMessage(const NotificationMessage&);
   NotificationMessage(NotificationMessage&&);
@@ -25,7 +18,6 @@ struct NotificationMessage final {
   NotificationMessage& operator=(const NotificationMessage&);
   NotificationMessage& operator=(NotificationMessage&&);
 
-  Appearance appearance;
   std::string message_id;
   std::string message_text;
   std::string link_text;

remoting/ios/app/notification_presenter.mm

@@ -9,7 +9,6 @@
 #include "remoting/ios/app/notification_presenter.h"
 
 #import "ios/third_party/material_components_ios/src/components/Dialogs/src/MaterialDialogs.h"
-#import "ios/third_party/material_components_ios/src/components/Snackbar/src/MaterialSnackbar.h"
 #import "remoting/ios/app/notification_dialog_view_controller.h"
 #import "remoting/ios/facade/remoting_authentication.h"
 #import "remoting/ios/facade/remoting_service.h"
@@ -141,53 +140,19 @@ void NotificationPresenter::OnNotificationFetched(
     [RemotingPreferences.instance synchronizeFlags];
   }
 
-  NSURL* url =
-      [NSURL URLWithString:base::SysUTF8ToNSString(notification->link_url)];
-
-  NSString* message_text_nsstring =
-      base::SysUTF8ToNSString(notification->message_text);
-  NSString* link_text_nsstring =
-      base::SysUTF8ToNSString(notification->link_text);
-
-  switch (notification->appearance) {
-    case NotificationMessage::Appearance::TOAST: {
-      MDCSnackbarMessage* message = [MDCSnackbarMessage new];
-      message.text = message_text_nsstring;
-      MDCSnackbarMessageAction* action = [MDCSnackbarMessageAction new];
-      action.handler = ^{
-        [[UIApplication sharedApplication] openURL:url
-                                           options:@{}
-                                 completionHandler:nil];
-      };
-      action.title = link_text_nsstring;
-      message.action = action;
-      message.duration = MDCSnackbarMessageDurationMax;
-      [MDCSnackbarManager showMessage:message];
-      break;
-    }
-    case NotificationMessage::Appearance::DIALOG: {
-      BOOL shouldShowDontShowAgain = notification->allow_dont_show_again &&
-                                     ui_state == SHOWN_AT_LEAST_ONCE;
-      NotificationDialogViewController* dialogVc =
-          [[NotificationDialogViewController alloc]
-                initWithNotificationMessage:*notification
-              shouldShowDontShowAgainToggle:shouldShowDontShowAgain];
-      [dialogVc presentOnTopVCWithCompletion:^(BOOL isDontShowAgainOn) {
-        NotificationUiState new_ui_state = isDontShowAgainOn
-                                               ? USER_CHOSE_DONT_SHOW_AGAIN
-                                               : SHOWN_AT_LEAST_ONCE;
-        [RemotingPreferences.instance
-            setObject:UiStateToNSNumber(new_ui_state)
-              forFlag:RemotingFlagNotificationUiState];
-        [RemotingPreferences.instance synchronizeFlags];
-      }];
-      break;
-    }
-    default:
-      LOG(DFATAL) << "Unknown appearance: "
-                  << static_cast<int>(notification->appearance);
-      break;
-  }
+  BOOL shouldShowDontShowAgain =
+      notification->allow_dont_show_again && ui_state == SHOWN_AT_LEAST_ONCE;
+  NotificationDialogViewController* dialogVc =
+      [[NotificationDialogViewController alloc]
+            initWithNotificationMessage:*notification
+          shouldShowDontShowAgainToggle:shouldShowDontShowAgain];
+  [dialogVc presentOnTopVCWithCompletion:^(BOOL isDontShowAgainOn) {
+    NotificationUiState new_ui_state =
+        isDontShowAgainOn ? USER_CHOSE_DONT_SHOW_AGAIN : SHOWN_AT_LEAST_ONCE;
+    [RemotingPreferences.instance setObject:UiStateToNSNumber(new_ui_state)
+                                    forFlag:RemotingFlagNotificationUiState];
+    [RemotingPreferences.instance synchronizeFlags];
+  }];
 }
 
 }  // namespace remoting