Add support for color input datalist on Android

ui/android/java/src/org/chromium/ui/ColorPickerSimple.java

Index: ui/android/java/src/org/chromium/ui/ColorPickerSimple.java
diff --git a/ui/android/java/src/org/chromium/ui/ColorPickerSimple.java b/ui/android/java/src/org/chromium/ui/ColorPickerSimple.java
index 979405a4078fce54e3cd606e6d6987acaaba8c4d..5e06f133070f825e2cc38a3f42e60764fe6ed98b 100644
--- a/ui/android/java/src/org/chromium/ui/ColorPickerSimple.java
+++ b/ui/android/java/src/org/chromium/ui/ColorPickerSimple.java
@@ -11,33 +11,34 @@ import android.graphics.Rect;
 import android.util.AttributeSet;
 import android.view.MotionEvent;
 import android.view.View;
-
+import android.view.ViewGroup;
 
 /**
  * Draws a grid of (predefined) colors and allows the user to choose one of
  * those colors.
  */
 public class ColorPickerSimple extends View {
-    private static final int ROW_COUNT = 2;
-
     private static final int COLUMN_COUNT = 4;
 
-    private static final int GRID_CELL_COUNT = ROW_COUNT * COLUMN_COUNT;
+    private static final int MINIMUM_CELL_HEIGHT = 50;

[newt (away), 2013/08/19 23:18:19]

MINIMUM_CELL_HEIGHT should be measured in DP, and probably should be called
MINIMUM_CELL_HEIGHT_DP for clarity. You can convert DP to pixels like so:

    int minimumCellHeight = Math.round(MINIMUM_CELL_HEIGHT_DP *
getResources().getDisplayMetrics().density);

+    private static final int GRID_LINE_WIDTH = 1;
 
-    private static final int[] COLORS = { Color.RED,
-                                          Color.CYAN,
-                                          Color.BLUE,
-                                          Color.GREEN,
-                                          Color.MAGENTA,
-                                          Color.YELLOW,
-                                          Color.BLACK,
-                                          Color.WHITE
-                                        };
+    private static final int[] DEFAULT_COLORS = { Color.RED,
+                                                  Color.CYAN,
+                                                  Color.BLUE,
+                                                  Color.GREEN,
+                                                  Color.MAGENTA,
+                                                  Color.YELLOW,
+                                                  Color.BLACK,
+                                                  Color.WHITE
+                                                };
 
     private Paint mBorderPaint;
 
     private Rect[] mBounds;
 
+    private int[] mColors;
+
     private Paint[] mPaints;
 
     private OnColorChangedListener mOnColorTouchedListener;
@@ -60,20 +61,30 @@ public class ColorPickerSimple extends View {
 
     /**
      * Initializes the listener and precalculates the grid and color positions.

[newt (away), 2013/08/19 23:18:19]

explain what happens if colors is empty. Also, I'd prefer passing null (instead
of an empty array) to signify "use default colors".

[keishi, 2013/08/26 05:28:54]

Changed to use default colors when passed null.

+     * TODO(keishi): Use colorLabels to improve accessibility.
      *
+     * @param colors The list of colors that should be displayed.
+     * @param colorLabels The label text that describe the colors.
      * @param onColorChangedListener The listener that gets notified when the user touches
      *                               a color.
      */
-    public void init(OnColorChangedListener onColorChangedListener) {
+    public void init(int[] colors,

[Miguel Garcia, 2013/08/19 14:43:27]

I think, somewhere in the whole stack, preferably before we convert it to a bare
array that needs to be mapped into memory. We should implement a limit on the
number of suggestions. Is there precedent for this on desktop?

[keishi, 2013/08/26 05:28:54]

No. I checked the limits for <input type=text> suggestions on other browsers. IE
only shows 16 suggestions. And Firefox has no limit.

I added a 10000 limit in the Blink patch.

[Miguel Garcia, 2013/08/27 14:32:00]

Do we really need 10K? Can we settle for 1K colors? I can't conveive a scenario
where more than 1K datalist element would be required and mapping 10K structs
would be expensive.

On 2013/08/26 05:28:54, keishi1 wrote:

[newt (away), 2013/08/27 16:05:29]

Should we limit the total amount of data sent over IPC (e.g. 1024 strings, max
length 1024 chars each) so we don't cause an out of memory error in the browser
process?

[keishi, 2013/08/29 05:04:11]

Added the 1024 char limit in Blink. I wonder if its the best way to handle the
IPC size limit.

+                     String[] colorLabels,
+                     OnColorChangedListener onColorChangedListener) {
         mOnColorTouchedListener = onColorChangedListener;
 
         // This will get calculated when the layout size is updated.
         mBounds = null;
 
-        mPaints = new Paint[GRID_CELL_COUNT];
-        for (int i = 0; i < GRID_CELL_COUNT; ++i) {
+        if (colors.length == 0)

[newt (away), 2013/08/19 23:18:19]

need curly braces

+            mColors = DEFAULT_COLORS;
+        else
+            mColors = colors;
+
+        mPaints = new Paint[mColors.length];
+        for (int i = 0; i < mColors.length; ++i) {
             Paint newPaint = new Paint();
-            newPaint.setColor(COLORS[i]);
+            newPaint.setColor(mColors[i]);
             mPaints[i] = newPaint;
         }
 
@@ -81,6 +92,10 @@ public class ColorPickerSimple extends View {
         int borderColor = getContext().getResources().getColor(R.color.color_picker_border_color);
         mBorderPaint.setColor(borderColor);
 
+        int rowCount = (int)Math.ceil((double)mColors.length / COLUMN_COUNT);

[newt (away), 2013/08/19 23:18:19]

Calculate mRowCount only once, and store it as a member variable. Also, this can
be done with integer arithmetic:

    mRowCount = (mColors.length + COLUMN_COUNT - 1) / COLUMN_COUNT;

+        int minimumHeight = rowCount * (MINIMUM_CELL_HEIGHT + GRID_LINE_WIDTH) + GRID_LINE_WIDTH;
+        setMinimumHeight(Math.max(minimumHeight, getMinimumHeight()));
+
         // Responds to the user touching the grid and works out which color has been chosen as
         // a result, depending on the X,Y coordinate. Note that we respond to the click event
         // here, but the onClick() method doesn't provide us with the X,Y coordinates, so we
@@ -91,12 +106,13 @@ public class ColorPickerSimple extends View {
             @Override
             public void onClick(View v) {
                 if (mOnColorTouchedListener != null && getWidth() > 0 && getHeight() > 0) {
+                    int rowCount = (int)Math.ceil((double)mColors.length / COLUMN_COUNT);

[newt (away), 2013/08/19 23:18:19]

use mRowCount here and below.

                     int column = mLastTouchedXPosition * COLUMN_COUNT / getWidth();
-                    int row = mLastTouchedYPosition * ROW_COUNT / getHeight();
+                    int row = mLastTouchedYPosition * rowCount / getHeight();
 
                     int colorIndex = (row * COLUMN_COUNT) + column;
-                    if (colorIndex >= 0 && colorIndex < COLORS.length) {
-                        mOnColorTouchedListener.onColorChanged(COLORS[colorIndex]);
+                    if (colorIndex >= 0 && colorIndex < mColors.length) {
+                        mOnColorTouchedListener.onColorChanged(mColors[colorIndex]);
                     }
                 }
             }
@@ -118,12 +134,15 @@ public class ColorPickerSimple extends View {
         canvas.drawColor(Color.WHITE);
 
         // Draw the actual colored rectangles.
-        for (int i = 0; i < GRID_CELL_COUNT; ++i) {
+        for (int i = 0; i < mColors.length; ++i) {

[Miguel Garcia, 2013/08/19 14:43:27]

can you calculate the number of cells and rows at the beginning and store it in
a local variable for readbility?

             canvas.drawRect(mBounds[i], mPaints[i]);
         }
 
         // Draw 1px borders between the rows.
-        for (int i = 0; i < ROW_COUNT - 1; ++i) {
+        int rowCount = (int)Math.ceil((double)mColors.length / COLUMN_COUNT);
+        for (int i = 0; i < rowCount - 1; ++i) {
+          if (i * COLUMN_COUNT >= mBounds.length)

[newt (away), 2013/08/19 23:18:19]

this condition can never be true.

+            break;
           canvas.drawLine(0,
                   mBounds[i * COLUMN_COUNT].bottom + 1,
                   getWidth(),
@@ -133,6 +152,8 @@ public class ColorPickerSimple extends View {
 
         // Draw 1px borders between the columns.
         for (int j = 0; j < COLUMN_COUNT - 1; ++j) {
+          if (j >= mBounds.length)
+            break;
           canvas.drawLine(mBounds[j].right + 1,
                   0,
                   mBounds[j].right + 1,
@@ -171,18 +192,24 @@ public class ColorPickerSimple extends View {
      * edge.
      */
     private void calculateGrid(final int width, final int height) {
-        mBounds = new Rect[GRID_CELL_COUNT];
+        mBounds = new Rect[mColors.length];
 
-        for (int i = 0; i < ROW_COUNT; ++i) {
+        int rowCount = (int)Math.ceil((double)mColors.length / COLUMN_COUNT);

[newt (away), 2013/08/19 23:18:19]

I don't see why you need to add/change lines 197-199 and 205-209. It should work
fine without those changes right?

If your goal is to use the value of GRID_LINE_WIDTH, you could change lines
178-182 to be:

    int left = j * (width + GRID_LINE_WIDTH) / COLUMN_COUNT + GRID_LINE_WIDTH;
    int right = (j + 1) * (width + GRID_LINE_WIDTH) / COLUMN_COUNT - 2 *
GRID_LINE_WIDTH;
    ...

But again, I'd prefer integer arithmetic over floating point division and
casting.

Also, could you move the calculations that use "i" but not "j" into the outer
loop?

+        double cellWidth = (width + GRID_LINE_WIDTH) / (double) COLUMN_COUNT;
+        double cellHeight = (height + GRID_LINE_WIDTH) / (double) rowCount;
+        for (int i = 0; i < rowCount; ++i) {
             for (int j = 0; j < COLUMN_COUNT; ++j) {
-                int left = j * (width + 1) / COLUMN_COUNT + 1;
-                int right = (j + 1) * (width + 1) / COLUMN_COUNT - 2;
+                int index = (i * COLUMN_COUNT) + j;
+                if (index >= mBounds.length)
+                    break;
+                int left = (int) (j * cellWidth + GRID_LINE_WIDTH);
+                int right = (int) ((j + 1) * cellWidth - GRID_LINE_WIDTH * 2);
 
-                int top = i * (height + 1) / ROW_COUNT + 1;
-                int bottom = (i + 1) * (height + 1) / ROW_COUNT - 2;
+                int top = (int) (i * cellHeight + GRID_LINE_WIDTH);
+                int bottom = (int) ((i + 1) * cellHeight - GRID_LINE_WIDTH * 2);
 
                 Rect rect = new Rect(left, top, right, bottom);
-                mBounds[(i * COLUMN_COUNT) + j] = rect;
+                mBounds[index] = rect;
             }
         }
     }