[TTS] Add a Feature and about:flag for Single Actions.

[TTS] Add a Feature and about:flag for Single Actions.

Adds a Field Trial Feature to enable single actions using Contextual Cards.
Exposes the feature in about:flags.
This feature doesn't do anything yet.

BUG=657063, 632451
Committed: https://crrev.com/43d1cea49ccf69b219373cfa9e8d105bb3c25f4e
Cr-Commit-Position: refs/heads/master@{#427764}

[Donn Denman, 2016-10-18 21:23:48]

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

[commit-bot: I haz the power, 2016-10-18 21:24:34]

Dry run: 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-10-18 22:20:48]

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

[commit-bot: I haz the power, 2016-10-18 22:20:49]

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

[Donn Denman, 2016-10-18 23:57:35]

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

[commit-bot: I haz the power, 2016-10-18 23:58:00]

Dry run: 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-10-19 01:35:20]

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

[commit-bot: I haz the power, 2016-10-19 01:35:21]

Dry run: This issue passed the CQ dry run.

[Donn Denman, 2016-10-21 02:08:35]

TODO(donnd): use this feature with
FeatureUtilities.isContextualSearchSingleActionsEnabled.
TODO(donnd): Consider switching to a boolean -- now shows as a tri-state with
default (like the CS-enable flag).

[Donn Denman, 2016-10-21 19:57:06]

donnd@chromium.org changed reviewers:
+ twellington@chromium.org

[Donn Denman, 2016-10-21 19:57:06]

Theresa, Please take an initial look at this at your convenience.   won't be
working on this until Monday, but want to send for first-pass and to help
coordinate effort.  Also see the 2 TODO's I posted in this review.

Thanks!

[Theresa, 2016-10-21 21:07:58]

On 2016/10/21 19:57:06, Donn Denman wrote:
> Theresa, Please take an initial look at this at your convenience.   won't be
> working on this until Monday, but want to send for first-pass and to help
> coordinate effort.  Also see the 2 TODO's I posted in this review.
> 
> Thanks!

lgtm unless you want to change it to a boolean. I don't really have a preference
how it shows up in about:// flags. I'm unfamiliar with how feature lists map to
variations data and command line flags.

[Donn Denman, 2016-10-24 18:02:27]

I think it would be better to have it be a boolean in about:flags, but doesn't
seem worth spending any time on, so will work to submit as-is.

Thanks!

[Donn Denman, 2016-10-24 18:03:32]

donnd@chromium.org changed reviewers:
+ tedchoc@chromium.org

[Donn Denman, 2016-10-24 18:03:32]

Ted, PTAL.  Thanks!

[Ted C, 2016-10-24 18:39:59]

https://codereview.chromium.org/2429033002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/ChromePreferenceManager.java
(right):

https://codereview.chromium.org/2429033002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/preferences/ChromePreferenceManager.java:44:
private static final String CONTEXTUAL_SEARCH_SINGLE_ACTIONS_ENABLED =
this doesn't seem to be used.

the changes to this file in general don't seem to be relevant anymore, but I
like the cleanup, so I'm not opposed to it being here.

https://codereview.chromium.org/2429033002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/util/FeatureUtilities.java
(right):

https://codereview.chromium.org/2429033002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/util/FeatureUtilities.java:274:
public static boolean isContextualSearchSingleActionsEnabled() {
I don't know if I'd recommend putting this here.  This file has become somewhat
of a dumping ground for "stuff".  I think having this live closer to contextual
search makes more sense.

Chrome Home and Herb are outliers because they need to be available super early
in the process (before native libraries have been loaded).  That is why we cache
a value in shared prefs and and fall back to ChromeFeatureList after native has
been spun up.  For CS, that isn't going to be the case since you can't trigger
w/o native.

So, I suggest that we just put this in contextual search

[Donn Denman, 2016-10-25 20:05:56]

Thanks Ted!

https://codereview.chromium.org/2429033002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/ChromePreferenceManager.java
(right):

https://codereview.chromium.org/2429033002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/preferences/ChromePreferenceManager.java:44:
private static final String CONTEXTUAL_SEARCH_SINGLE_ACTIONS_ENABLED =
On 2016/10/24 18:39:58, Ted C (OOO 10.25 - 30) wrote:
> this doesn't seem to be used.
> 
> the changes to this file in general don't seem to be relevant anymore, but I
> like the cleanup, so I'm not opposed to it being here.

Done.

https://codereview.chromium.org/2429033002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/util/FeatureUtilities.java
(right):

https://codereview.chromium.org/2429033002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/util/FeatureUtilities.java:274:
public static boolean isContextualSearchSingleActionsEnabled() {
On 2016/10/24 18:39:58, Ted C (OOO 10.25 - 30) wrote:
> I don't know if I'd recommend putting this here.  This file has become
somewhat
> of a dumping ground for "stuff".  I think having this live closer to
contextual
> search makes more sense.
> 
> Chrome Home and Herb are outliers because they need to be available super
early
> in the process (before native libraries have been loaded).  That is why we
cache
> a value in shared prefs and and fall back to ChromeFeatureList after native
has
> been spun up.  For CS, that isn't going to be the case since you can't trigger
> w/o native.
> 
> So, I suggest that we just put this in contextual search

Done.

[Donn Denman, 2016-10-25 20:07:13]

donnd@chromium.org changed reviewers:
+ asvitkine@google.com
- tedchoc@chromium.org

[Donn Denman, 2016-10-25 20:07:13]

asvitkine@ PTAL.  

Removing Ted since he's OOO.

[Donn Denman, 2016-10-25 20:11:34]

donnd@chromium.org changed reviewers:
+ asvitkine@chromium.org, mariakhomenko@chromium.org
- asvitkine@google.com

[Donn Denman, 2016-10-25 20:11:35]

Maria, PTAL at the Android files.

asvitkine@ for owners only.

[Maria, 2016-10-25 20:37:00]

lgtm

https://codereview.chromium.org/2429033002/diff/60001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchFieldTrial.java
(right):

https://codereview.chromium.org/2429033002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchFieldTrial.java:108:
private static Boolean sContextualSearchSingleActionsEnabled;
nit: newline below

[Alexei Svitkine (slow), 2016-10-25 20:39:25]

https://codereview.chromium.org/2429033002/diff/60001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2429033002/diff/60001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:90160: +  <int value="1455439709"
label="cs-contextual-cards-single-actions"/>
Is this needed?

I suspect you can remove this since you already have the two entries for the
feature and you're not adding a command-line flag called
--cs-contextual-cards-single-actions.

[Donn Denman, 2016-10-25 20:52:28]

Thanks Maria!

Alexei, We like using the command-line flag for development.  Does that seem
like a good
enough reason to leave it in?

https://codereview.chromium.org/2429033002/diff/60001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchFieldTrial.java
(right):

https://codereview.chromium.org/2429033002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/contextualsearch/ContextualSearchFieldTrial.java:108:
private static Boolean sContextualSearchSingleActionsEnabled;
On 2016/10/25 20:37:00, Maria wrote:
> nit: newline below

Done.

https://codereview.chromium.org/2429033002/diff/60001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2429033002/diff/60001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:90160: +  <int value="1455439709"
label="cs-contextual-cards-single-actions"/>
On 2016/10/25 20:39:25, Alexei Svitkine (slow) wrote:
> Is this needed?
> 
> I suspect you can remove this since you already have the two entries for the
> feature and you're not adding a command-line flag called
> --cs-contextual-cards-single-actions.

We like using the command-line flag for development.  Does that seem like a good
enough reason to leave it in?

[Alexei Svitkine (slow), 2016-10-25 20:54:11]

https://codereview.chromium.org/2429033002/diff/60001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2429033002/diff/60001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:90160: +  <int value="1455439709"
label="cs-contextual-cards-single-actions"/>
On 2016/10/25 20:52:28, Donn Denman wrote:
> On 2016/10/25 20:39:25, Alexei Svitkine (slow) wrote:
> > Is this needed?
> > 
> > I suspect you can remove this since you already have the two entries for the
> > feature and you're not adding a command-line flag called
> > --cs-contextual-cards-single-actions.
> 
> We like using the command-line flag for development.  Does that seem like a
good
> enough reason to leave it in?

If you also have the feature, you should be able to use
--enable-features=ContextualSearchSingleActions and
--disable-features=ContextualSearchSingleActions for development and not have
the extra flag.

[Donn Denman, 2016-10-25 21:03:30]

Alexei, PTAL, and thanks for the review!

https://codereview.chromium.org/2429033002/diff/60001/tools/metrics/histogram...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/2429033002/diff/60001/tools/metrics/histogram...
tools/metrics/histograms/histograms.xml:90160: +  <int value="1455439709"
label="cs-contextual-cards-single-actions"/>
On 2016/10/25 20:54:11, Alexei Svitkine (slow) wrote:
> On 2016/10/25 20:52:28, Donn Denman wrote:
> > On 2016/10/25 20:39:25, Alexei Svitkine (slow) wrote:
> > > Is this needed?
> > > 
> > > I suspect you can remove this since you already have the two entries for
the
> > > feature and you're not adding a command-line flag called
> > > --cs-contextual-cards-single-actions.
> > 
> > We like using the command-line flag for development.  Does that seem like a
> good
> > enough reason to leave it in?
> 
> If you also have the feature, you should be able to use
> --enable-features=ContextualSearchSingleActions and
> --disable-features=ContextualSearchSingleActions for development and not have
> the extra flag.

Done.  Thanks for explaining this.

[Alexei Svitkine (slow), 2016-10-25 21:14:05]

lgtm

[Donn Denman, 2016-10-25 21:22:12]

The CQ bit was checked by donnd@chromium.org

[Donn Denman, 2016-10-25 21:22:12]

The patchset sent to the CQ was uploaded after l-g-t-m from
twellington@chromium.org, mariakhomenko@chromium.org
Link to the patchset: https://codereview.chromium.org/2429033002/#ps100001
(title: "Removed cs-contextual-cards-single-actions from histograms.xml.")

[commit-bot: I haz the power, 2016-10-25 21:22:47]

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-10-25 21:25:35]

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

[commit-bot: I haz the power, 2016-10-25 21:25:36]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Donn Denman, 2016-10-26 17:34:22]

The CQ bit was checked by donnd@chromium.org

[Donn Denman, 2016-10-26 17:34:23]

The patchset sent to the CQ was uploaded after l-g-t-m from
twellington@chromium.org, asvitkine@chromium.org, mariakhomenko@chromium.org
Link to the patchset: https://codereview.chromium.org/2429033002/#ps120001
(title: "Just a rebase.")

[commit-bot: I haz the power, 2016-10-26 17:34:50]

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-10-26 18:52:16]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-10-26 19:22:10]

Description was changed from

==========
[TTS] Add a Feature and about:flag for Single Actions.

Adds a Field Trial Feature to enable single actions using Contextual Cards.
Exposes the feature in about:flags.
This feature doesn't do anything yet.

BUG=657063,632451
==========

to

==========
[TTS] Add a Feature and about:flag for Single Actions.

Adds a Field Trial Feature to enable single actions using Contextual Cards.
Exposes the feature in about:flags.
This feature doesn't do anything yet.

BUG=657063,632451

Committed: https://crrev.com/43d1cea49ccf69b219373cfa9e8d105bb3c25f4e
Cr-Commit-Position: refs/heads/master@{#427764}
==========

[commit-bot: I haz the power, 2016-10-26 19:22:11]

Patchset 7 (id:??) landed as
https://crrev.com/43d1cea49ccf69b219373cfa9e8d105bb3c25f4e
Cr-Commit-Position: refs/heads/master@{#427764}