Plumb through notification action types and placeholders on Android

ui/message_center/notification.h

 // Copyright (c) 2013 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_MESSAGE_CENTER_NOTIFICATION_H_
 #define UI_MESSAGE_CENTER_NOTIFICATION_H_
 
 #include <stddef.h>
 
 #include <string>
 #include <vector>
 
 #include "base/strings/string16.h"
+#include "base/strings/nullable_string16.h"
 #include "base/time/time.h"
 #include "base/values.h"
 #include "ui/gfx/image/image.h"
 #include "ui/message_center/message_center_export.h"
 #include "ui/message_center/notification_delegate.h"
 #include "ui/message_center/notification_types.h"
 #include "ui/message_center/notifier_settings.h"
 #include "url/gurl.h"
 
 namespace message_center {
 
 struct MESSAGE_CENTER_EXPORT NotificationItem {
   base::string16 title;
   base::string16 message;
 
   NotificationItem(const base::string16& title, const base::string16& message);
 };
 
+enum class NotificationActionType { BUTTON, TEXT };

[awdf, 2016/09/13 14:14:55]

wasn't sure where to put this

[Peter Beverloo, 2016/09/14 16:27:12]

This is fine. Probably should call it ButtonType to be consistent with the other
definitions in this file.

[awdf, 2016/09/22 14:17:12]

Done.

+
 struct MESSAGE_CENTER_EXPORT ButtonInfo {
   base::string16 title;
   gfx::Image icon;
+  message_center::NotificationActionType type;

[Peter Beverloo, 2016/09/14 16:27:12]

nit: no need to prefix with "message_center::", that's the namespace you're in.

[awdf, 2016/09/26 15:33:28]

Done.

+  base::NullableString16 placeholder;

[Peter Beverloo, 2016/09/14 16:27:12]

I think you should see compile errors about this class now needing an
out-of-line move operator. At that point you should apply the rule of three:

http://en.cppreference.com/w/cpp/language/rule_of_three

However, does |placeholder| need to be a NullableString? How is NULL
semantically different from an empty string for the action input's behaviour? If
it's not, this could just be a base::string16.

[awdf, 2016/09/22 14:17:12]

I didn't see any compile errors about this, can you explain why you think I
should?  I made it a NullableString because that's what it is in
PlatformNotificationData before it's converted into this ButtonInfo
(https://cs.chromium.org/chromium/src/content/public/common/platform_notificat...)
- I could convert to a normal string (mapping NULL -> "") there, or change it in
PlatformNotificationData too, so convert at the point where that's constructed
from a WebNotificationAction - it uses a WebString there - might the WebString
be NULL if not specified in the javascript?

[Peter Beverloo, 2016/09/22 18:07:55]

I saw compile errors when building this. Maybe run the trybots?

Keeping this a base::NullableString16 SGTM based on our offline chat.

[awdf, 2016/09/26 15:33:28]

Done.

 
   ButtonInfo(const base::string16& title);
 };
 
 class MESSAGE_CENTER_EXPORT RichNotificationData {
  public:
   RichNotificationData();
   RichNotificationData(const RichNotificationData& other);
   ~RichNotificationData();
 
@@ skipping 240 matching lines @@
   bool is_read_;  // True if this has been seen in the message center.
 
   // A proxy object that allows access back to the JavaScript object that
   // represents the notification, for firing events.
   scoped_refptr<NotificationDelegate> delegate_;
 };
 
 }  // namespace message_center
 
 #endif  // UI_MESSAGE_CENTER_NOTIFICATION_H_