Limit the number of custom menu items to 5

Limit the number of custom menu items to 5

Right now we don't have a strict limit on number of custom
menu items. Declaring this to be 5 initially.

BUG=527478
TEST=CustomTabActivityTest#testCustomTabMaxMenuItems

Committed: https://crrev.com/9621f7d0e4b94fc4918a609c59ae63c234cd2473
Cr-Commit-Position: refs/heads/master@{#347081}

[Yusuf, 2015-09-02 20:24:56]

yusufo@chromium.org changed reviewers:
+ newt@chromium.org

[newt (away), 2015-09-02 20:28:31]

https://codereview.chromium.org/1329733003/diff/1/chrome/android/javatests/sr...
File
chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabActivityTest.java
(right):

https://codereview.chromium.org/1329733003/diff/1/chrome/android/javatests/sr...
chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabActivityTest.java:229:
final int expectedMenuSize = 8;
Why 8? Maybe add a comment explaining what the additional menu items are. Or
only count the menu items with the desired title or id (which would be less
fragile).

[Yusuf, 2015-09-02 21:24:45]

https://codereview.chromium.org/1329733003/diff/1/chrome/android/javatests/sr...
File
chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabActivityTest.java
(right):

https://codereview.chromium.org/1329733003/diff/1/chrome/android/javatests/sr...
chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabActivityTest.java:229:
final int expectedMenuSize = 8;
On 2015/09/02 20:28:31, newt wrote:
> Why 8? Maybe add a comment explaining what the additional menu items are. Or
> only count the menu items with the desired title or id (which would be less
> fragile).

Done.

[newt (away), 2015-09-02 21:48:19]

https://codereview.chromium.org/1329733003/diff/20001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabActivityTest.java
(right):

https://codereview.chromium.org/1329733003/diff/20001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabActivityTest.java:234:
final int expectedMenuSize =
Actually, I think it was better before, when it was more explicit (instead of
calling Math.min()). How about this:

    expectedMenuSize = 5 + NUM_CHROME_MENU_ITEMS;

or

    expectedMenuSize = MAX_MENU_CUSTOM_ITEMS + NUM_CHROME_MENU_ITEMS;

In general, it's preferable for tests to minimize logic and be as
straightforward as possible.

[Yusuf, 2015-09-02 23:36:19]

https://codereview.chromium.org/1329733003/diff/20001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabActivityTest.java
(right):

https://codereview.chromium.org/1329733003/diff/20001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/customtabs/CustomTabActivityTest.java:234:
final int expectedMenuSize =
On 2015/09/02 21:48:19, newt wrote:
> Actually, I think it was better before, when it was more explicit (instead of
> calling Math.min()). How about this:
> 
>     expectedMenuSize = 5 + NUM_CHROME_MENU_ITEMS;
> 
> or
> 
>     expectedMenuSize = MAX_MENU_CUSTOM_ITEMS + NUM_CHROME_MENU_ITEMS;
> 
> In general, it's preferable for tests to minimize logic and be as
> straightforward as possible.

Done.

[Yusuf, 2015-09-02 23:36:24]

The CQ bit was checked by yusufo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-09-02 23:36:41]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1329733003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1329733003/40001

[newt (away), 2015-09-02 23:52:56]

Yay. Lgtm.

[commit-bot: I haz the power, 2015-09-03 00:33:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-09-03 00:33:58]

Dry run: This issue passed the CQ dry run.

[Yusuf, 2015-09-03 00:43:26]

The CQ bit was checked by yusufo@chromium.org

[commit-bot: I haz the power, 2015-09-03 00:43:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1329733003/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1329733003/40001

[commit-bot: I haz the power, 2015-09-03 00:51:41]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-09-03 00:52:30]

Patchset 3 (id:??) landed as
https://crrev.com/9621f7d0e4b94fc4918a609c59ae63c234cd2473
Cr-Commit-Position: refs/heads/master@{#347081}