Improve monitor compatibility error, move feedback link into button

ash/display/display_util.cc

Index: ash/display/display_util.cc
diff --git a/ash/display/display_util.cc b/ash/display/display_util.cc
index 1b86878302b57fe8d5f00983fa00b520c7831150..258d5e1a14e232641097a02f760b1205a82c1366 100644
--- a/ash/display/display_util.cc
+++ b/ash/display/display_util.cc
@@ -16,11 +16,11 @@
 #include "ash/host/ash_window_tree_host.h"
 #include "ash/shell.h"
 #include "base/strings/string_number_conversions.h"
-#include "grit/ash_resources.h"
+#include "base/strings/utf_string_conversions.h"
+#include "grit/ash_strings.h"
 #include "ui/aura/env.h"
 #include "ui/aura/window_tree_host.h"
 #include "ui/base/l10n/l10n_util.h"
-#include "ui/base/resource/resource_bundle.h"
 #include "ui/display/display.h"
 #include "ui/display/manager/display_manager.h"
 #include "ui/display/manager/managed_display_info.h"
@@ -34,7 +34,9 @@
 #include "ui/wm/core/coordinate_conversion.h"
 
 #if defined(OS_CHROMEOS)
+#include "ash/resources/vector_icons/vector_icons.h"
 #include "base/sys_info.h"
+#include "ui/gfx/paint_vector_icon.h"
 #endif
 
 namespace ash {
@@ -42,6 +44,14 @@ namespace {
 
 const char kDisplayErrorNotificationId[] = "chrome://settings/display/error";
 
+#if defined(OS_CHROMEOS)
+// TODO(glevin): These are for new MD vector icons, but are using pre-MD color
+// scheme. When we switch to all MD icons for notifications, these should be
+// updated to use MD color scheme.
+const SkColor kDisplayIconColor = SkColorSetRGB(0xBD, 0xBD, 0xBD);
+const SkColor kFeedbackIconColor = SkColorSetRGB(0x96, 0x96, 0x98);
+#endif
+
 // A notification delegate that will start the feedback app when the notication
 // is clicked.
 class DisplayErrorNotificationDelegate
@@ -50,9 +60,8 @@ class DisplayErrorNotificationDelegate
   DisplayErrorNotificationDelegate() = default;
 
   // message_center::NotificationDelegate:
-  bool HasClickedListener() override { return true; }
-
-  void Click() override {
+  void ButtonClick(int index) override {
+    DCHECK_EQ(0, index);
     WmShell::Get()->new_window_controller()->OpenFeedbackPage();
   }
 
@@ -163,25 +172,33 @@ void MoveCursorTo(AshWindowTreeHost* ash_host,
 
 
 #if defined(OS_CHROMEOS)
-void ShowDisplayErrorNotification(int message_id) {
+void ShowDisplayErrorNotification(base::string16& message,
+                                  bool allow_feedback) {
   // Always remove the notification to make sure the notification appears
   // as a popup in any situation.
   message_center::MessageCenter::Get()->RemoveNotification(
       kDisplayErrorNotificationId, false /* by_user */);
 
-  ui::ResourceBundle& bundle = ui::ResourceBundle::GetSharedInstance();
+  message_center::RichNotificationData optional_field;

[Evan Stade, 2016/12/12 23:44:17]

nit: this variable name strikes me as a little odd. Perhaps |details|?

[Greg Levin, 2016/12/13 01:48:59]

I was just going for consistency by copying some existing code.  Turns out the
consistency boat has sailed.  Right now we have

data                    15
optional                 8
optional_fields          7
rich_notification_data   6
rich_data                2
notification_data        1
options                  1

(see
https://cs.chromium.org/search/?q=%22message_center::RichNotificationData+%22...)

How does "data" sound?  :-P

[Evan Stade, 2016/12/13 01:56:13]

data is fine. |RichNotificationDetails| would be more idiomatic than
|RichNotificationData| but it is what it is.

+  if (allow_feedback) {
+    message_center::ButtonInfo send_button(
+        l10n_util::GetStringUTF16(IDS_ASH_DISPLAY_FAILURE_SEND_FEEDBACK));
+    send_button.icon = gfx::Image(
+        CreateVectorIcon(kNotificationFeedbackButtonIcon, kFeedbackIconColor));
+    optional_field.buttons.push_back(send_button);
+  }
+
   std::unique_ptr<message_center::Notification> notification(
       new message_center::Notification(
           message_center::NOTIFICATION_TYPE_SIMPLE, kDisplayErrorNotificationId,
           base::string16(),  // title
-          l10n_util::GetStringUTF16(message_id),
-          bundle.GetImageNamed(IDR_AURA_NOTIFICATION_DISPLAY),
+          message, gfx::Image(CreateVectorIcon(kNotificationDisplayErrorIcon,
+                                               kDisplayIconColor)),
           base::string16(),  // display_source
           GURL(), message_center::NotifierId(
                       message_center::NotifierId::SYSTEM_COMPONENT,
                       system_notifier::kNotifierDisplayError),
-          message_center::RichNotificationData(),
-          new DisplayErrorNotificationDelegate));
+          optional_field, new DisplayErrorNotificationDelegate));
   message_center::MessageCenter::Get()->AddNotification(
       std::move(notification));
 }