Plumb through notification action types and placeholders on Android

ui/message_center/notification.h

Index: ui/message_center/notification.h
diff --git a/ui/message_center/notification.h b/ui/message_center/notification.h
index 93d255e56daa8439201c0348146a3df69e15ee7c..bfca493e164a5d05f2a59cc764dfd2c0d7ad81f8 100644
--- a/ui/message_center/notification.h
+++ b/ui/message_center/notification.h
@@ -10,6 +10,7 @@
 #include <string>
 #include <vector>
 
+#include "base/strings/nullable_string16.h"

[dewittj, 2016/09/28 02:41:58]

unused?

[awdf, 2016/09/28 11:42:09]

Done, thanks.

Is there an automated tool I can use to clean up unused imports as I edit?

[Peter Beverloo, 2016/09/28 12:00:40]

Unfortunately there isn't. (This is a hard problem because one of the includes
after here might "use" NullableString16 as well.)

 #include "base/strings/string16.h"
 #include "base/time/time.h"
 #include "base/values.h"
@@ -29,11 +30,18 @@ struct MESSAGE_CENTER_EXPORT NotificationItem {
   NotificationItem(const base::string16& title, const base::string16& message);
 };
 
+enum class ButtonType { BUTTON, TEXT };
+
 struct MESSAGE_CENTER_EXPORT ButtonInfo {
   base::string16 title;
   gfx::Image icon;
+  ButtonType type = ButtonType::BUTTON;
+  base::string16 placeholder;
 
-  ButtonInfo(const base::string16& title);
+  explicit ButtonInfo(const base::string16& title);
+  ButtonInfo(const ButtonInfo& other);
+  ~ButtonInfo();
+  ButtonInfo& operator=(const ButtonInfo& other);

[dewittj, 2016/09/28 02:41:58]

any reason not to put =default here instead of .cc file?

[awdf, 2016/09/28 11:42:09]

Nope,  other than I didn't know you could (and I was copying from the example at
https://www.chromium.org/developers/coding-style/chromium-style-checker-errors
for how to fix the "Complex class/struct needs an explicit out-of-line copy
constructor" error).

So in general is it preferred style to always put =default implementations in
the header file then?

[Peter Beverloo, 2016/09/28 12:00:40]

Well, by moving =default to the header file you're inlining them again, so I
would expect this to yield the Clang error in the CL you just CQed.

For this particular class, the compiler-generated constructor has to roughly do
four things:

```
ButtonInfo::ButtonInfo(const base::string16& other_title) {
  title.assign(other_title);  // will malloc()
  new (&icon) gfx::Image();
  type = ButtonType::BUTTON;
  new (&placeholder) base::string16();
}
```

That's three calls and an assign— more if either of the called constructors will
be inlined themselves.

By inlining the constructor, this code will be copied to each function that
creates a new ButtonInfo object. This can very quickly get out of bounds, and
lead to a significant bump in binary size.

By outlining the constructor, i.e. defining =default in the .cc file, each call
site will instead have one call to the constructor, which in turn executes the
code that would otherwise be inlined. This is an abstraction that favors size
over speed, but often at negligible levels.

To give a concrete example, look at the diff in the first two files of the
following CL, that moves a function's definition from the header file (thus
might be inlined) to the definition file. Super trivial, but saved 50KB of
binary size.

https://codereview.chromium.org/331593006/

 };
 
 class MESSAGE_CENTER_EXPORT RichNotificationData {