Extract NotificationBuilderBase for holding the arguments.

chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationBuilderBase.java

Index: chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationBuilderBase.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/notifications/StandardNotificationBuilder.java b/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationBuilderBase.java
similarity index 21%
copy from chrome/android/java/src/org/chromium/chrome/browser/notifications/StandardNotificationBuilder.java
copy to chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationBuilderBase.java
index d321e1e91abecccca68dc115ba567cafbe9b009c..57bcfc1c8c4861d239511f95755e370ee65ccdeb 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/notifications/StandardNotificationBuilder.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/notifications/NotificationBuilderBase.java
@@ -1,4 +1,4 @@
-// Copyright 2015 The Chromium Authors. All rights reserved.
+// Copyright 2016 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.
 
@@ -6,95 +6,134 @@
 
 import android.app.Notification;
 import android.app.PendingIntent;
-import android.content.Context;
 import android.graphics.Bitmap;
-import android.support.v4.app.NotificationCompat;
+import android.support.v4.app.NotificationCompat.Action;
 
-/**
- * Builds a notification using the given inputs. Relies on NotificationCompat and
- * NotificationCompat.BigTextStyle to provide a standard layout.
- */
-public class StandardNotificationBuilder implements NotificationBuilder {
-    private final NotificationCompat.Builder mBuilder;
+import org.chromium.base.VisibleForTesting;
 
-    public StandardNotificationBuilder(Context context) {
-        mBuilder = new NotificationCompat.Builder(context);
-    }
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
 
-    @Override
-    public Notification build() {
-        return mBuilder.build();
-    }
+import javax.annotation.Nullable;
+
+/**
+ * Abstract base class for building a notification. Stores all given arguments for later use.
+ */
+public abstract class NotificationBuilderBase implements NotificationBuilder {

[Peter Beverloo, 2016/01/27 11:36:03]

Do we need to keep the distinction between the interface and the abstract class?
Could we merge them together?

Right now, the diagram would look like:

                                                 CustomNotificationBuilder
NotificationBuilder -> NotificationBuilderBase /
                                               \
                                                 StandardNotificationBuilder

[Michael van Ouwerkerk, 2016/01/27 15:04:25]

From a purist perspective, I'd like to keep the interface. From a practical and
resource conservation perspective, we should get rid of it. Done.

+    /**
+     * Maximum length of CharSequence inputs to prevent excessive memory consumption. At current
+     * screen sizes we display about 500 characters at most, so this is a pretty generous limit, and
+     * it matches what the Notification class does.
+     */
+    @VisibleForTesting
+    static final int MAX_CHARSEQUENCE_LENGTH = 5 * 1024;
+
+    /**
+     * The maximum number of action buttons. One is for the settings button, and two more slots are
+     * for developer provided buttons.
+     */
+    private static final int MAX_ACTION_BUTTONS = 3;
+
+    protected CharSequence mTitle;
+    protected CharSequence mBody;
+    protected CharSequence mOrigin;
+    protected CharSequence mTickerText;
+    protected Bitmap mLargeIcon;
+    protected int mSmallIconId;
+    protected PendingIntent mContentIntent;
+    protected PendingIntent mDeleteIntent;
+    protected List<Action> mActions = new ArrayList<>(MAX_ACTION_BUTTONS);
+    protected Action mSettingsAction;
+    protected int mDefaults = Notification.DEFAULT_ALL;
+    protected long[] mVibratePattern;
 
     @Override
-    public NotificationBuilder setTitle(CharSequence title) {
-        mBuilder.setContentTitle(title);
+    public NotificationBuilder setTitle(@Nullable CharSequence title) {
+        mTitle = limitLength(title);
         return this;
     }
 
     @Override
-    public NotificationBuilder setBody(CharSequence body) {
-        mBuilder.setContentText(body).setStyle(new NotificationCompat.BigTextStyle().bigText(body));
+    public NotificationBuilder setBody(@Nullable CharSequence body) {
+        mBody = limitLength(body);
         return this;
     }
 
     @Override
-    public NotificationBuilder setOrigin(CharSequence origin) {
-        mBuilder.setSubText(origin);
+    public NotificationBuilder setOrigin(@Nullable CharSequence origin) {
+        mOrigin = limitLength(origin);
         return this;
     }
 
     @Override
-    public NotificationBuilder setTicker(CharSequence tickerText) {
-        mBuilder.setTicker(tickerText);
+    public NotificationBuilder setTicker(@Nullable CharSequence tickerText) {
+        mTickerText = limitLength(tickerText);
         return this;
     }
 
     @Override
-    public NotificationBuilder setLargeIcon(Bitmap icon) {
-        mBuilder.setLargeIcon(icon);
+    public NotificationBuilder setLargeIcon(@Nullable Bitmap icon) {
+        mLargeIcon = icon;
         return this;
     }
 
     @Override
     public NotificationBuilder setSmallIcon(int iconId) {
-        mBuilder.setSmallIcon(iconId);
+        mSmallIconId = iconId;
         return this;
     }
 
     @Override
-    public NotificationBuilder setContentIntent(PendingIntent intent) {
-        mBuilder.setContentIntent(intent);
+    public NotificationBuilder setContentIntent(@Nullable PendingIntent intent) {
+        mContentIntent = intent;
         return this;
     }
 
     @Override
-    public NotificationBuilder setDeleteIntent(PendingIntent intent) {
-        mBuilder.setDeleteIntent(intent);
+    public NotificationBuilder setDeleteIntent(@Nullable PendingIntent intent) {
+        mDeleteIntent = intent;
         return this;
     }
 
     @Override
-    public NotificationBuilder addAction(int iconId, CharSequence title, PendingIntent intent) {
-        mBuilder.addAction(iconId, title, intent);
+    public NotificationBuilder addAction(
+            int iconId, @Nullable CharSequence title, @Nullable PendingIntent intent) {
+        if (mActions.size() == MAX_ACTION_BUTTONS) {
+            throw new IllegalStateException(
+                    "Cannot add more than " + MAX_ACTION_BUTTONS + " actions.");
+        }
+        mActions.add(new Action(iconId, limitLength(title), intent));
         return this;
     }
 
     @Override
     public NotificationBuilder addSettingsAction(
-            int iconId, CharSequence title, PendingIntent intent) {
-        return addAction(iconId, title, intent);
+            int iconId, @Nullable CharSequence title, @Nullable PendingIntent intent) {
+        mSettingsAction = new Action(iconId, limitLength(title), intent);
+        return this;
     }
 
     @Override
     public NotificationBuilder setDefaults(int defaults) {
-        mBuilder.setDefaults(defaults);
+        mDefaults = defaults;
         return this;
     }
 
     @Override
-    public NotificationBuilder setVibrate(long[] pattern) {
-        mBuilder.setVibrate(pattern);
+    public NotificationBuilder setVibrate(@Nullable long[] pattern) {

[Peter Beverloo, 2016/01/27 11:36:03]

This seems incorrect? You're marking it as @Nullable but are using the |pattern|
member on the next line.

[Michael van Ouwerkerk, 2016/01/27 15:04:25]

Done.

+        mVibratePattern = Arrays.copyOf(pattern, pattern.length);
         return this;
     }
+
+    @Nullable
+    private static CharSequence limitLength(@Nullable CharSequence input) {
+        if (input == null) {
+            return input;
+        }
+        if (input.length() > MAX_CHARSEQUENCE_LENGTH) {
+            return input.subSequence(0, MAX_CHARSEQUENCE_LENGTH);
+        }
+        return input;
+    }
 }