📰 Add a context menu to remove status items

[NTP Client] Add a context menu to remove status items

Also modifies the ContextMenuHelper and related interfaces
to make it easier to select which items to show.

BUG=638580, 663747
Committed: https://crrev.com/be9cf09ebbbb0990e0a5c82b5a68c4523259b35b
Cr-Commit-Position: refs/heads/master@{#432613}

[dgn, 2016-10-28 16:17:21]

dgn@chromium.org changed reviewers:
+ bauerb@chromium.org

[dgn, 2016-10-28 16:17:22]

PTAL

https://codereview.chromium.org/2459033002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/InnerNode.java
(right):

https://codereview.chromium.org/2459033002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/InnerNode.java:80:
public Object getItemAt(int position) {
Made this non specific to SnippetArticle so that I can trigger the dismissal of
something else than an article. But that makes the whole thing a bit ugly since
we don't have a single type of object to return here. I could make it a generic
method maybe, and return null if the type does not match?

https://codereview.chromium.org/2459033002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapter.java
(right):

https://codereview.chromium.org/2459033002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapter.java:527:
announceItemRemoved("Status card removed"); // TODO
Strings TBD

[Bernhard Bauer, 2016-10-28 16:48:17]

https://codereview.chromium.org/2459033002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/InnerNode.java
(right):

https://codereview.chromium.org/2459033002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/InnerNode.java:80:
public Object getItemAt(int position) {
On 2016/10/28 16:17:21, dgn wrote:
> Made this non specific to SnippetArticle so that I can trigger the dismissal
of
> something else than an article. But that makes the whole thing a bit ugly
since
> we don't have a single type of object to return here. I could make it a
generic
> method maybe, and return null if the type does not match?

Ugh. Yeah, I would maybe suggest something like

  <T> T getItemAt(Class<? extends T> clazz, int position)

but see my other comment below.

https://codereview.chromium.org/2459033002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageRecyclerView.java
(right):

https://codereview.chromium.org/2459033002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageRecyclerView.java:479:
final int position = getNewTabPageAdapter().getItemPosition(item);
So, all of these shenanigans are just so that we can get the position in case it
changed between the time the context menu was created and the time the dismiss
item was selected, right? It seems like there should be an easier way to do
this. Couldn't we just get the adapter position from the ViewHolder, and maybe
cancel the context menu (or neuter the dismiss action) if the ViewHolder is
re-bound?

[dgn, 2016-10-28 17:15:48]

https://codereview.chromium.org/2459033002/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageRecyclerView.java
(right):

https://codereview.chromium.org/2459033002/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageRecyclerView.java:479:
final int position = getNewTabPageAdapter().getItemPosition(item);
On 2016/10/28 16:48:17, Bernhard (OOO until Nov 11) wrote:
> So, all of these shenanigans are just so that we can get the position in case
it
> changed between the time the context menu was created and the time the dismiss
> item was selected, right? It seems like there should be an easier way to do
> this. Couldn't we just get the adapter position from the ViewHolder, and maybe
> cancel the context menu (or neuter the dismiss action) if the ViewHolder is
> re-bound?

We'd still have to get the viewHolder, but if we manually track it from
onBindViewHolder it should be better, yes. Will try that on Monday :D

[dgn, 2016-11-10 19:44:02]

Still very WIP, but PTAL to sanity check the design

https://codereview.chromium.org/2459033002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapter.java
(right):

https://codereview.chromium.org/2459033002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapter.java:638:
mNewTabPageManager.getContextMenuLifetimeManager().onViewModified(holder.itemView);
problem2: making actions a noop when tapped is kind of okay, but I tried to just
close the menu when it becomes irrelevant. Here, when it is removed from the
recycler view. For most likely I pass the above the fold view. as associated
view, so that when that one goes off the menu is closed.

TBD: check if it is necessary to do something in onBindViewHolder

https://codereview.chromium.org/2459033002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageRecyclerView.java
(left):

https://codereview.chromium.org/2459033002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/NewTabPageRecyclerView.java:492:
final int position = getNewTabPageAdapter().getSuggestionPosition(suggestion);
problem1: this does not support things that are not SnippetArticles: status
cards, button, etc.

[Bernhard Bauer, 2016-11-11 13:30:08]

https://codereview.chromium.org/2459033002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/ContextMenuHandler.java
(right):

https://codereview.chromium.org/2459033002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/ContextMenuHandler.java:113:
mManager.getContextMenuLifetimeManager().onContextMenuCreated(menu,
associatedView);
Wait, so the manager owns the LifetimeManager, which is defined by this class
and has a private method that only we are able to call? That seems like very
tight coupling. Could we move this into its own class? And maybe we could move
the contextMenuCloseCallback methods to that class as well, making it a
ContextMenuManager or some'n?

https://codereview.chromium.org/2459033002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/ContextMenuHandler.java:174:
if (mAssociatedView == null || mAssociatedView != v) return;
|mAssociatedView| would be null if either that is passed at construction time,
or the menu is closed, right? In the former case, that will make the whole class
a no-op, so the client could just skip creating it, and in the latter case the
next line will catch it anyway.

https://codereview.chromium.org/2459033002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/StatusCardViewHolder.java
(right):

https://codereview.chromium.org/2459033002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/StatusCardViewHolder.java:94:

Remove the empty line?

[dgn, 2016-11-14 18:33:51]

Description was changed from

==========
[NTP Client] Add a context menu to remove status items

BUG=638580
==========

to

==========
[NTP Client] Add a context menu to remove status items

Also modifies the ContextMenuHelper and related interfaces
to make it easier to select which items to show.

BUG=638580,663747
==========

[dgn, 2016-11-14 18:34:08]

PTAL

https://codereview.chromium.org/2459033002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/ContextMenuHandler.java
(right):

https://codereview.chromium.org/2459033002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/ContextMenuHandler.java:113:
mManager.getContextMenuLifetimeManager().onContextMenuCreated(menu,
associatedView);
On 2016/11/11 13:30:08, Bernhard Bauer wrote:
> Wait, so the manager owns the LifetimeManager, which is defined by this class
> and has a private method that only we are able to call? That seems like very
> tight coupling. Could we move this into its own class? And maybe we could move
> the contextMenuCloseCallback methods to that class as well, making it a
> ContextMenuManager or some'n?

Done.

https://codereview.chromium.org/2459033002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/ContextMenuHandler.java:174:
if (mAssociatedView == null || mAssociatedView != v) return;
On 2016/11/11 13:30:08, Bernhard Bauer wrote:
> |mAssociatedView| would be null if either that is passed at construction time,
> or the menu is closed, right? In the former case, that will make the whole
class
> a no-op, so the client could just skip creating it, and in the latter case the
> next line will catch it anyway.

Simplified this and removed the redundant checks.

https://codereview.chromium.org/2459033002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/StatusCardViewHolder.java
(right):

https://codereview.chromium.org/2459033002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/StatusCardViewHolder.java:94:

On 2016/11/11 13:30:08, Bernhard Bauer wrote:
> Remove the empty line?

Done.

[Bernhard Bauer, 2016-11-15 16:35:26]

https://codereview.chromium.org/2459033002/diff/60001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/ContextMenuManager.java
(right):

https://codereview.chromium.org/2459033002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/ContextMenuManager.java:53:
/** Removed the current item. */
Nit: "Remove"

https://codereview.chromium.org/2459033002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/ContextMenuManager.java:59:
/** @return ids of the menu items that are supported. Note: Must be a sorted
array. */
Nit: "IDs"

https://codereview.chromium.org/2459033002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/ContextMenuManager.java:60:
int[] getSupportedMenuItems();
Could you just return a Set<Integer> instead?

https://codereview.chromium.org/2459033002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/ContextMenuManager.java:64:
public interface TouchDisableableView {
I'm still not a huge fan of this interface. Could we just split it up and pass
the view separately? Then optionally we could replace the interface with a
Callback<Boolean>.

https://codereview.chromium.org/2459033002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/ContextMenuManager.java:76:
public void createContextMenu(ContextMenu menu, View associatedView, Delegate
delegate) {
Javadoc for public methods.

https://codereview.chromium.org/2459033002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/ContextMenuManager.java:102:
// Disable touch events on the RecyclerView while the context menu is open. This
is to
Nit: If you are abstracting the actual type of the outer view away, try not to
refer to RecyclerView directly.

https://codereview.chromium.org/2459033002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/ContextMenuManager.java:134:
private boolean showItem(@ContextMenuItemId int itemId, Delegate delegate) {
shouldShowItem maybe?

https://codereview.chromium.org/2459033002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/ContextMenuManager.java:150:
return false;
assert false here?

https://codereview.chromium.org/2459033002/diff/60001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/MostVisitedItem.java
(right):

https://codereview.chromium.org/2459033002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/MostVisitedItem.java:158:
menu, mNewTabPageLayout, mContextMenuDelegate);
You could just create a new anonymous subclass. Or, if you want to keep the
member, make it final?

https://codereview.chromium.org/2459033002/diff/60001/chrome/android/java/src...
File chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java
(right):

https://codereview.chromium.org/2459033002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java:294:
ContextMenuManager getContextMenuManager();
Javadoc

[dgn, 2016-11-16 12:43:34]

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

[commit-bot: I haz the power, 2016-11-16 12:43:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dgn, 2016-11-16 12:44:37]

PTAL

https://codereview.chromium.org/2459033002/diff/60001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/ContextMenuManager.java
(right):

https://codereview.chromium.org/2459033002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/ContextMenuManager.java:53:
/** Removed the current item. */
On 2016/11/15 16:35:26, Bernhard Bauer wrote:
> Nit: "Remove"

Done.

https://codereview.chromium.org/2459033002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/ContextMenuManager.java:59:
/** @return ids of the menu items that are supported. Note: Must be a sorted
array. */
On 2016/11/15 16:35:26, Bernhard Bauer wrote:
> Nit: "IDs"

Done.

https://codereview.chromium.org/2459033002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/ContextMenuManager.java:60:
int[] getSupportedMenuItems();
On 2016/11/15 16:35:26, Bernhard Bauer wrote:
> Could you just return a Set<Integer> instead?

I'm using an int[] so that it can be defined as a static without issue. I
remember a discussion discouraging instantiating static objects to avoid
affecting startup time. I suppose I can use an instance member instead.
Thoughts?

https://codereview.chromium.org/2459033002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/ContextMenuManager.java:64:
public interface TouchDisableableView {
On 2016/11/15 16:35:26, Bernhard Bauer wrote:
> I'm still not a huge fan of this interface. Could we just split it up and pass
> the view separately? Then optionally we could replace the interface with a
> Callback<Boolean>.

I realised that I don't need the reference to the View itself. I left the
interface since even with a Callback, I would have to implement enabling and
disabling touch on the view. I think having it as an interface makes it easier
to see why it's needed.

https://codereview.chromium.org/2459033002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/ContextMenuManager.java:76:
public void createContextMenu(ContextMenu menu, View associatedView, Delegate
delegate) {
On 2016/11/15 16:35:26, Bernhard Bauer wrote:
> Javadoc for public methods.

Done.

https://codereview.chromium.org/2459033002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/ContextMenuManager.java:102:
// Disable touch events on the RecyclerView while the context menu is open. This
is to
On 2016/11/15 16:35:26, Bernhard Bauer wrote:
> Nit: If you are abstracting the actual type of the outer view away, try not to
> refer to RecyclerView directly.

Done.

https://codereview.chromium.org/2459033002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/ContextMenuManager.java:134:
private boolean showItem(@ContextMenuItemId int itemId, Delegate delegate) {
On 2016/11/15 16:35:26, Bernhard Bauer wrote:
> shouldShowItem maybe?

Done.

https://codereview.chromium.org/2459033002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/ContextMenuManager.java:150:
return false;
On 2016/11/15 16:35:26, Bernhard Bauer wrote:
> assert false here?

Done.

https://codereview.chromium.org/2459033002/diff/60001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/MostVisitedItem.java
(right):

https://codereview.chromium.org/2459033002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/MostVisitedItem.java:158:
menu, mNewTabPageLayout, mContextMenuDelegate);
On 2016/11/15 16:35:26, Bernhard Bauer wrote:
> You could just create a new anonymous subclass. Or, if you want to keep the
> member, make it final?

Done.

https://codereview.chromium.org/2459033002/diff/60001/chrome/android/java/src...
File chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java
(right):

https://codereview.chromium.org/2459033002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/NewTabPageView.java:294:
ContextMenuManager getContextMenuManager();
On 2016/11/15 16:35:26, Bernhard Bauer wrote:
> Javadoc

Done.

[commit-bot: I haz the power, 2016-11-16 14:22:43]

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

[commit-bot: I haz the power, 2016-11-16 14:22:43]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)

[Michael van Ouwerkerk, 2016-11-16 14:56:30]

https://codereview.chromium.org/2459033002/diff/80001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/ntp/snippets/ArticleSnippetsTest.java
(right):

https://codereview.chromium.org/2459033002/diff/80001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/ntp/snippets/ArticleSnippetsTest.java:205:
// TODO(dgn): provide a RecyclerView if we need to test the context menu.
It should be relatively easy to add a test for this functionality in
NewTabPageRecyclerViewTest: https://codereview.chromium.org/2504113003/

[Bernhard Bauer, 2016-11-16 15:07:40]

https://codereview.chromium.org/2459033002/diff/60001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/ContextMenuManager.java
(right):

https://codereview.chromium.org/2459033002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/ContextMenuManager.java:60:
int[] getSupportedMenuItems();
On 2016/11/16 12:44:36, dgn wrote:
> On 2016/11/15 16:35:26, Bernhard Bauer wrote:
> > Could you just return a Set<Integer> instead?
> 
> I'm using an int[] so that it can be defined as a static without issue. I
> remember a discussion discouraging instantiating static objects to avoid
> affecting startup time. I suppose I can use an instance member instead.
> Thoughts?

In Java? Statics should be created when the class is used for the first time,
which should not affect startup time too much. And if necessary you could always
do lazy instantiation, either manually, or via the trick with a static inner
class (https://en.wikipedia.org/wiki/Initialization-on-demand_holder_idiom).

[dgn, 2016-11-16 15:46:22]

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

[dgn, 2016-11-16 15:46:29]

PTAL

https://codereview.chromium.org/2459033002/diff/60001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/ntp/ContextMenuManager.java
(right):

https://codereview.chromium.org/2459033002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/ntp/ContextMenuManager.java:60:
int[] getSupportedMenuItems();
On 2016/11/16 15:07:39, Bernhard Bauer wrote:
> On 2016/11/16 12:44:36, dgn wrote:
> > On 2016/11/15 16:35:26, Bernhard Bauer wrote:
> > > Could you just return a Set<Integer> instead?
> > 
> > I'm using an int[] so that it can be defined as a static without issue. I
> > remember a discussion discouraging instantiating static objects to avoid
> > affecting startup time. I suppose I can use an instance member instead.
> > Thoughts?
> 
> In Java? Statics should be created when the class is used for the first time,
> which should not affect startup time too much. And if necessary you could
always
> do lazy instantiation, either manually, or via the trick with a static inner
> class (https://en.wikipedia.org/wiki/Initialization-on-demand_holder_idiom).

Oh neat! Anyway, I realised that context menu is an edge case anyway, no reason
not to only instantiate the set when needed.

[commit-bot: I haz the power, 2016-11-16 15:46:44]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Bernhard Bauer, 2016-11-16 16:11:51]

LGTM

On 2016/11/16 15:46:29, dgn wrote:
> PTAL
> 
>
https://codereview.chromium.org/2459033002/diff/60001/chrome/android/java/src...
> File
>
chrome/android/java/src/org/chromium/chrome/browser/ntp/ContextMenuManager.java
> (right):
> 
>
https://codereview.chromium.org/2459033002/diff/60001/chrome/android/java/src...
>
chrome/android/java/src/org/chromium/chrome/browser/ntp/ContextMenuManager.java:60:
> int[] getSupportedMenuItems();
> On 2016/11/16 15:07:39, Bernhard Bauer wrote:
> > On 2016/11/16 12:44:36, dgn wrote:
> > > On 2016/11/15 16:35:26, Bernhard Bauer wrote:
> > > > Could you just return a Set<Integer> instead?
> > > 
> > > I'm using an int[] so that it can be defined as a static without issue. I
> > > remember a discussion discouraging instantiating static objects to avoid
> > > affecting startup time. I suppose I can use an instance member instead.
> > > Thoughts?
> > 
> > In Java? Statics should be created when the class is used for the first
time,
> > which should not affect startup time too much. And if necessary you could
> always
> > do lazy instantiation, either manually, or via the trick with a static inner
> > class (https://en.wikipedia.org/wiki/Initialization-on-demand_holder_idiom).
> 
> Oh neat! Anyway, I realised that context menu is an edge case anyway, no
reason
> not to only instantiate the set when needed.

Eh. I wouldn't mind making the full set a constant in ContextMenuManager, but up
to you.

[dgn, 2016-11-16 18:43:00]

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

[commit-bot: I haz the power, 2016-11-16 18:44:02]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dgn, 2016-11-16 19:16:06]

The CQ bit was unchecked by dgn@chromium.org

[dgn, 2016-11-16 19:16:06]

The CQ bit was checked by dgn@chromium.org

[dgn, 2016-11-16 19:16:07]

The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org
Link to the patchset: https://codereview.chromium.org/2459033002/#ps220001
(title: "fix test")

[commit-bot: I haz the power, 2016-11-16 19:18:16]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-16 20:36:46]

Description was changed from

==========
[NTP Client] Add a context menu to remove status items

Also modifies the ContextMenuHelper and related interfaces
to make it easier to select which items to show.

BUG=638580,663747
==========

to

==========
[NTP Client] Add a context menu to remove status items

Also modifies the ContextMenuHelper and related interfaces
to make it easier to select which items to show.

BUG=638580,663747
==========

[commit-bot: I haz the power, 2016-11-16 20:36:47]

Committed patchset #12 (id:220001)

[commit-bot: I haz the power, 2016-11-16 20:55:23]

Description was changed from

==========
[NTP Client] Add a context menu to remove status items

Also modifies the ContextMenuHelper and related interfaces
to make it easier to select which items to show.

BUG=638580,663747
==========

to

==========
[NTP Client] Add a context menu to remove status items

Also modifies the ContextMenuHelper and related interfaces
to make it easier to select which items to show.

BUG=638580,663747

Committed: https://crrev.com/be9cf09ebbbb0990e0a5c82b5a68c4523259b35b
Cr-Commit-Position: refs/heads/master@{#432613}
==========

[commit-bot: I haz the power, 2016-11-16 20:55:24]

Patchset 12 (id:??) landed as
https://crrev.com/be9cf09ebbbb0990e0a5c82b5a68c4523259b35b
Cr-Commit-Position: refs/heads/master@{#432613}