Stubs for triggering the LSD in Clank

Stubs for triggering the LSD in Clank

BUG=672301
Review-Url: https://codereview.chromium.org/2709883005
Cr-Commit-Position: refs/heads/master@{#454435}
Committed: https://chromium.googlesource.com/chromium/src/+/6b1ebf77022a1754cedb39c47fb94a0a1c9f8777

[qfiard, 2017-02-22 13:55:13]

Description was changed from

==========
Stubs for triggering the LSD in Clank

BUG=672301
==========

to

==========
Stubs for triggering the LSD in Clank

BUG=672301
==========

[qfiard, 2017-02-24 15:21:33]

The CQ bit was checked by qfiard@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-24 15:21:56]

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

[qfiard, 2017-02-24 15:22:31]

qfiard@google.com changed reviewers:
+ benwells@chromium.org

[commit-bot: I haz the power, 2017-02-24 17:01:16]

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

[commit-bot: I haz the power, 2017-02-24 17:01:17]

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...)

[qfiard, 2017-02-24 23:15:37]

The CQ bit was checked by qfiard@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-24 23:16:17]

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, 2017-02-25 00:47:46]

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

[commit-bot: I haz the power, 2017-02-25 00:47:47]

Dry run: This issue passed the CQ dry run.

[qfiard, 2017-02-27 14:56:37]

The CQ bit was checked by qfiard@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-27 14:56:46]

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, 2017-02-27 16:17:13]

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

[commit-bot: I haz the power, 2017-02-27 16:17:14]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[qfiard, 2017-02-27 22:38:30]

The CQ bit was checked by qfiard@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-27 22:39:41]

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, 2017-02-28 01:18:55]

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

[commit-bot: I haz the power, 2017-02-28 01:18:56]

Dry run: This issue passed the CQ dry run.

[benwells, 2017-02-28 06:26:37]

https://codereview.chromium.org/2709883005/diff/100001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/LocationSettings.java
(right):

https://codereview.chromium.org/2709883005/diff/100001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/preferences/LocationSettings.java:121:
private static native void nativeRunBoolCallback(long nativeBoolCallback,
boolean result);
You should call these something more meaningful / specific, like
nativeOnSomethingSomethign, e.g. nativeOnCanPromptDetermined.

See https://cs.chromium.org/search/?q=nativeOn+file:java&type=cs for examples.

Also I don't think you need to create the two nested classes for this, you
should be able to have OnCanPromptDetermined (or whatever you call it) on
LocationSettingsImpl.

https://codereview.chromium.org/2709883005/diff/100001/chrome/browser/android...
File chrome/browser/android/location_settings.h (right):

https://codereview.chromium.org/2709883005/diff/100001/chrome/browser/android...
chrome/browser/android/location_settings.h:41: //
https://cs.chromium.org/chromium/src/components/location/android/java/src/org...
Nit: you can refer to source files as //components/location/...etc. without the
bit https://cs.chromium.org... etc prefix.

https://codereview.chromium.org/2709883005/diff/100001/chrome/browser/android...
chrome/browser/android/location_settings.h:45: GESTURE = 1,
Nit: I think we should call this NON_SEARCH. We might one day relax or change
the gesture requirement.

[benwells, 2017-02-28 06:51:38]

https://codereview.chromium.org/2709883005/diff/100001/chrome/browser/android...
File chrome/browser/android/location_settings.h (right):

https://codereview.chromium.org/2709883005/diff/100001/chrome/browser/android...
chrome/browser/android/location_settings.h:28: // Asynchronously checks if a
prompt can be triggered to ask the user to turn
Is it necessary for this to be async? It makes the permission side a little
nastier if it is.

[benwells, 2017-02-28 07:03:26]

Sorry for the scattershot comments.

https://codereview.chromium.org/2709883005/diff/100001/chrome/browser/android...
File chrome/browser/android/location_settings.h (right):

https://codereview.chromium.org/2709883005/diff/100001/chrome/browser/android...
chrome/browser/android/location_settings.h:42: enum PromptContext {
This (and PromptOutcome) are a bit vague - there might be other prompts at some
stage. Can we make the names more specific, e.g.
LocationSettingsDialogPromptContext?

[qfiard, 2017-02-28 14:19:09]

On 2017/02/28 06:26:37, benwells wrote:
>
https://codereview.chromium.org/2709883005/diff/100001/chrome/android/java/sr...
> File
>
chrome/android/java/src/org/chromium/chrome/browser/preferences/LocationSettings.java
> (right):
> 
>
https://codereview.chromium.org/2709883005/diff/100001/chrome/android/java/sr...
>
chrome/android/java/src/org/chromium/chrome/browser/preferences/LocationSettings.java:121:
> private static native void nativeRunBoolCallback(long nativeBoolCallback,
> boolean result);
> You should call these something more meaningful / specific, like
> nativeOnSomethingSomethign, e.g. nativeOnCanPromptDetermined.
> 
> See https://cs.chromium.org/search/?q=nativeOn+file:java&type=cs for examples.
Done.

> 
> Also I don't think you need to create the two nested classes for this, you
> should be able to have OnCanPromptDetermined (or whatever you call it) on
> LocationSettingsImpl.
Done, thanks for the tip.

> 
>
https://codereview.chromium.org/2709883005/diff/100001/chrome/browser/android...
> File chrome/browser/android/location_settings.h (right):
> 
>
https://codereview.chromium.org/2709883005/diff/100001/chrome/browser/android...
> chrome/browser/android/location_settings.h:41: //
>
https://cs.chromium.org/chromium/src/components/location/android/java/src/org...
> Nit: you can refer to source files as //components/location/...etc. without
the
> bit https://cs.chromium.org... etc prefix.
Done.

> 
>
https://codereview.chromium.org/2709883005/diff/100001/chrome/browser/android...
> chrome/browser/android/location_settings.h:45: GESTURE = 1,
> Nit: I think we should call this NON_SEARCH. We might one day relax or change
> the gesture requirement.
I do not like NON_SEARCH/SEARCH too much because it will be confusing to extend
it to more contexts in the future (e.g. differentiate the prompt after an
infobar click, a gesture or onload) as NON_SEARCH/SEARCH feels like it covers
all possible contexts. I have updated the enum names to DEFAULT and SEARCH to
make it clearer that this might be updated in the future. Please let me know if
that makes sense.

[qfiard, 2017-02-28 14:22:14]

On 2017/02/28 06:51:38, benwells wrote:
>
https://codereview.chromium.org/2709883005/diff/100001/chrome/browser/android...
> File chrome/browser/android/location_settings.h (right):
> 
>
https://codereview.chromium.org/2709883005/diff/100001/chrome/browser/android...
> chrome/browser/android/location_settings.h:28: // Asynchronously checks if a
> prompt can be triggered to ask the user to turn
> Is it necessary for this to be async? It makes the permission side a little
> nastier if it is.

I checked and in AGSA checking that GmsCore is available and the system location
setting is off is enough to guarantee that a LSD prompt can be triggered 99.85%
of the time. These two checks can be done synchronously, so if a 0.15% error
rate is acceptable to you we can indeed switch this method to simply return a
boolean.

I have changed the interface to be synchronous, please let me know if you change
your mind and I can revert to the asynchronous API.

[qfiard, 2017-02-28 14:22:33]

On 2017/02/28 07:03:26, benwells wrote:
> Sorry for the scattershot comments.
> 
>
https://codereview.chromium.org/2709883005/diff/100001/chrome/browser/android...
> File chrome/browser/android/location_settings.h (right):
> 
>
https://codereview.chromium.org/2709883005/diff/100001/chrome/browser/android...
> chrome/browser/android/location_settings.h:42: enum PromptContext {
> This (and PromptOutcome) are a bit vague - there might be other prompts at
some
> stage. Can we make the names more specific, e.g.
> LocationSettingsDialogPromptContext?

Done.

[qfiard, 2017-02-28 14:40:36]

The CQ bit was checked by qfiard@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-02-28 14:40:56]

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, 2017-02-28 15:53:05]

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

[commit-bot: I haz the power, 2017-02-28 15:53:06]

Dry run: This issue passed the CQ dry run.

[benwells, 2017-03-01 03:51:27]

looking really good :)

https://codereview.chromium.org/2709883005/diff/120001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/LocationSettings.java
(right):

https://codereview.chromium.org/2709883005/diff/120001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/preferences/LocationSettings.java:115:
private static native void nativeOnLocationSettingsDialogPromptOutcomeReceived(
Nit: can we make this just nativeOnLocationSettingsDialogOutcome?

https://codereview.chromium.org/2709883005/diff/120001/chrome/browser/android...
File chrome/browser/android/location_settings.h (right):

https://codereview.chromium.org/2709883005/diff/120001/chrome/browser/android...
chrome/browser/android/location_settings.h:40: enum
LocationSettingsDialogPromptContext {
nit: Sorry, I know I suggested this name but the 'Prompt' is redundant. Could
you make it LocationSettingsDialogContext?

https://codereview.chromium.org/2709883005/diff/120001/chrome/browser/android...
chrome/browser/android/location_settings.h:52: enum
LocationSettingsDialogPromptOutcome {
nit: Ditto here, the 'Prompt' is redundant.

https://codereview.chromium.org/2709883005/diff/120001/chrome/browser/android...
File chrome/browser/android/location_settings_impl.cc (right):

https://codereview.chromium.org/2709883005/diff/120001/chrome/browser/android...
chrome/browser/android/location_settings_impl.cc:54:
std::move(*callback).Run(static_cast<LocationSettingsDialogPromptOutcome>(
I don't really understand the call to std::move here. 

Can you try doing something like 
https://cs.chromium.org/chromium/src/chrome/browser/android/shortcut_helper.c...

and

https://cs.chromium.org/chromium/src/chrome/browser/android/shortcut_helper.c...

https://codereview.chromium.org/2709883005/diff/120001/chrome/browser/android...
File chrome/browser/android/mock_location_settings.cc (right):

https://codereview.chromium.org/2709883005/diff/120001/chrome/browser/android...
chrome/browser/android/mock_location_settings.cc:44:
std::move(callback).Run(LocationSettingsDialogPromptOutcome::NO_PROMPT);
Again, I don't really understand the use of move here.

[benwells, 2017-03-01 03:51:27]

looking really good :)

[benwells, 2017-03-01 03:52:44]

On 2017/03/01 03:51:27, benwells wrote:
> looking really good :)

[benwells, 2017-03-01 03:53:27]

On 2017/03/01 03:52:44, benwells wrote:
> On 2017/03/01 03:51:27, benwells wrote:
> > looking really good :)

Oh, BTW, please respond to comments in the comment itself (vs. replying). It
makes it easier to track what's done / what isn't.

[qfiard, 2017-03-01 08:48:18]

https://codereview.chromium.org/2709883005/diff/120001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/LocationSettings.java
(right):

https://codereview.chromium.org/2709883005/diff/120001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/preferences/LocationSettings.java:115:
private static native void nativeOnLocationSettingsDialogPromptOutcomeReceived(
On 2017/03/01 03:51:26, benwells wrote:
> Nit: can we make this just nativeOnLocationSettingsDialogOutcome?

Done.

https://codereview.chromium.org/2709883005/diff/120001/chrome/browser/android...
File chrome/browser/android/location_settings.h (right):

https://codereview.chromium.org/2709883005/diff/120001/chrome/browser/android...
chrome/browser/android/location_settings.h:40: enum
LocationSettingsDialogPromptContext {
On 2017/03/01 03:51:26, benwells wrote:
> nit: Sorry, I know I suggested this name but the 'Prompt' is redundant. Could
> you make it LocationSettingsDialogContext?

Done.

https://codereview.chromium.org/2709883005/diff/120001/chrome/browser/android...
chrome/browser/android/location_settings.h:52: enum
LocationSettingsDialogPromptOutcome {
On 2017/03/01 03:51:26, benwells wrote:
> nit: Ditto here, the 'Prompt' is redundant.

Done.

https://codereview.chromium.org/2709883005/diff/120001/chrome/browser/android...
File chrome/browser/android/location_settings_impl.cc (right):

https://codereview.chromium.org/2709883005/diff/120001/chrome/browser/android...
chrome/browser/android/location_settings_impl.cc:54:
std::move(*callback).Run(static_cast<LocationSettingsDialogPromptOutcome>(
On 2017/03/01 03:51:27, benwells wrote:
> I don't really understand the call to std::move here. 
> 
> Can you try doing something like 
>
https://cs.chromium.org/chromium/src/chrome/browser/android/shortcut_helper.c...
> 
> and
> 
>
https://cs.chromium.org/chromium/src/chrome/browser/android/shortcut_helper.c...

I followed the instructions in
https://cs.chromium.org/chromium/src/docs/callback.md, IIUC OnceCallbacks are
now the recommended way of passing callbacks that are meant to be called only
once. OnceCallbacks must be called on a rvalue to be able to consume the
callback.

But I am happy to revert to regular callbacks if you are not familiar with these
and would rather keep using base::Callbacks.

[qfiard, 2017-03-01 08:55:46]

The CQ bit was checked by qfiard@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-01 08:56:03]

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, 2017-03-01 08:58:31]

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

[commit-bot: I haz the power, 2017-03-01 08:58:32]

Dry run: Try jobs failed on following builders:
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[qfiard, 2017-03-01 09:22:17]

The CQ bit was checked by qfiard@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-01 09:22:42]

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

[benwells, 2017-03-01 10:47:45]

I think this is good to send to owners :)

lgtm

https://codereview.chromium.org/2709883005/diff/120001/chrome/browser/android...
File chrome/browser/android/location_settings_impl.cc (right):

https://codereview.chromium.org/2709883005/diff/120001/chrome/browser/android...
chrome/browser/android/location_settings_impl.cc:54:
std::move(*callback).Run(static_cast<LocationSettingsDialogPromptOutcome>(
On 2017/03/01 08:48:18, qfiard wrote:
> On 2017/03/01 03:51:27, benwells wrote:
> > I don't really understand the call to std::move here. 
> > 
> > Can you try doing something like 
> >
>
https://cs.chromium.org/chromium/src/chrome/browser/android/shortcut_helper.c...
> > 
> > and
> > 
> >
>
https://cs.chromium.org/chromium/src/chrome/browser/android/shortcut_helper.c...
> 
> I followed the instructions in
> https://cs.chromium.org/chromium/src/docs/callback.md, IIUC OnceCallbacks are
> now the recommended way of passing callbacks that are meant to be called only
> once. OnceCallbacks must be called on a rvalue to be able to consume the
> callback.
> 
> But I am happy to revert to regular callbacks if you are not familiar with
these
> and would rather keep using base::Callbacks.

Ah ... well there you go, thanks for educating me!

[qfiard, 2017-03-01 10:52:03]

qfiard@google.com changed reviewers:
+ tedchoc@chromium.org

[commit-bot: I haz the power, 2017-03-01 10:57:41]

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

[commit-bot: I haz the power, 2017-03-01 10:57:42]

Dry run: This issue passed the CQ dry run.

[Ted C, 2017-03-02 00:21:56]

https://codereview.chromium.org/2709883005/diff/180001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/LocationSettings.java
(right):

https://codereview.chromium.org/2709883005/diff/180001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/preferences/LocationSettings.java:63:
public static boolean canPromptToEnableSystemLocationSetting() {
do these need to be public?

https://codereview.chromium.org/2709883005/diff/180001/chrome/browser/android...
File chrome/browser/android/location_settings.h (right):

https://codereview.chromium.org/2709883005/diff/180001/chrome/browser/android...
chrome/browser/android/location_settings.h:38: // Keep in sync with the
LocationSettingsDialogContext IntDef in
We have a system to generate java int constants based on a C++ enum.

If you look for GENERATED_JAVA_ENUM_PACKAGE, you'll see other examples.

The problem is there are no C++ class in components/location/android, but I
think we should consider them here as the layering is very weird.

If that is infeasible right now, please add a TODO and address it after branch.

https://codereview.chromium.org/2709883005/diff/180001/chrome/browser/android...
chrome/browser/android/location_settings.h:78: virtual void
PromptToEnableSystemLocationSetting(
I would add comments about the lifetime of this.  This could potentially trigger
a new Android widget whose lifecycle is outside of our control.  If the owner of
the callback has any chance of being destroyed during this process, they should
be careful to use the proper weak ptr semantics to avoid crashing here.

https://codereview.chromium.org/2709883005/diff/180001/chrome/browser/android...
File chrome/browser/android/location_settings_impl.cc (right):

https://codereview.chromium.org/2709883005/diff/180001/chrome/browser/android...
chrome/browser/android/location_settings_impl.cc:39: auto* callback_ptr = new
LocationSettingsDialogOutcomeCallback(
maybe a comment around ownership and that Outcome is guaranteed to be called and
destroy it.

https://codereview.chromium.org/2709883005/diff/180001/components/location/an...
File
components/location/android/java/src/org/chromium/components/location/LocationUtils.java
(right):

https://codereview.chromium.org/2709883005/diff/180001/components/location/an...
components/location/android/java/src/org/chromium/components/location/LocationUtils.java:119:
int GESTURE = 1;
should be indented +4 more for all the contents of this...same for below.

[qfiard, 2017-03-02 13:28:13]

https://codereview.chromium.org/2709883005/diff/180001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/preferences/LocationSettings.java
(right):

https://codereview.chromium.org/2709883005/diff/180001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/preferences/LocationSettings.java:63:
public static boolean canPromptToEnableSystemLocationSetting() {
On 2017/03/02 00:21:56, Ted C wrote:
> do these need to be public?

No, thanks for the tip.

https://codereview.chromium.org/2709883005/diff/180001/chrome/browser/android...
File chrome/browser/android/location_settings.h (right):

https://codereview.chromium.org/2709883005/diff/180001/chrome/browser/android...
chrome/browser/android/location_settings.h:38: // Keep in sync with the
LocationSettingsDialogContext IntDef in
On 2017/03/02 00:21:56, Ted C wrote:
> We have a system to generate java int constants based on a C++ enum.
> 
> If you look for GENERATED_JAVA_ENUM_PACKAGE, you'll see other examples.
> 
> The problem is there are no C++ class in components/location/android, but I
> think we should consider them here as the layering is very weird.
> 
> If that is infeasible right now, please add a TODO and address it after
branch.

Done, nice!

https://codereview.chromium.org/2709883005/diff/180001/chrome/browser/android...
chrome/browser/android/location_settings.h:78: virtual void
PromptToEnableSystemLocationSetting(
On 2017/03/02 00:21:56, Ted C wrote:
> I would add comments about the lifetime of this.  This could potentially
trigger
> a new Android widget whose lifecycle is outside of our control.  If the owner
of
> the callback has any chance of being destroyed during this process, they
should
> be careful to use the proper weak ptr semantics to avoid crashing here.

Done.

https://codereview.chromium.org/2709883005/diff/180001/chrome/browser/android...
File chrome/browser/android/location_settings_impl.cc (right):

https://codereview.chromium.org/2709883005/diff/180001/chrome/browser/android...
chrome/browser/android/location_settings_impl.cc:39: auto* callback_ptr = new
LocationSettingsDialogOutcomeCallback(
On 2017/03/02 00:21:56, Ted C wrote:
> maybe a comment around ownership and that Outcome is guaranteed to be called
and
> destroy it.

Done.

https://codereview.chromium.org/2709883005/diff/180001/components/location/an...
File
components/location/android/java/src/org/chromium/components/location/LocationUtils.java
(right):

https://codereview.chromium.org/2709883005/diff/180001/components/location/an...
components/location/android/java/src/org/chromium/components/location/LocationUtils.java:119:
int GESTURE = 1;
On 2017/03/02 00:21:56, Ted C wrote:
> should be indented +4 more for all the contents of this...same for below.

Obsolete now that the Java enum is autogenerated.

[qfiard, 2017-03-02 13:38:32]

The CQ bit was checked by qfiard@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-03-02 13:38:47]

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, 2017-03-02 14:58:18]

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

[commit-bot: I haz the power, 2017-03-02 14:58:19]

Dry run: This issue passed the CQ dry run.

[Ted C, 2017-03-02 16:55:48]

lgtm w/ last nit.  thanks!

https://codereview.chromium.org/2709883005/diff/200001/components/location/an...
File components/location/android/BUILD.gn (right):

https://codereview.chromium.org/2709883005/diff/200001/components/location/an...
components/location/android/BUILD.gn:7:
java_cpp_enum("location_settings_dialog_context_java") {
I "think" you should be able to just have one java_cpp_enum
(location_settings_dialog_enums_java) that has both sources listed than having
two java_cpp_enum targets

[qfiard, 2017-03-02 20:40:28]

https://codereview.chromium.org/2709883005/diff/200001/components/location/an...
File components/location/android/BUILD.gn (right):

https://codereview.chromium.org/2709883005/diff/200001/components/location/an...
components/location/android/BUILD.gn:7:
java_cpp_enum("location_settings_dialog_context_java") {
On 2017/03/02 16:55:48, Ted C wrote:
> I "think" you should be able to just have one java_cpp_enum
> (location_settings_dialog_enums_java) that has both sources listed than having
> two java_cpp_enum targets

Done.

[qfiard, 2017-03-02 21:48:32]

The CQ bit was checked by qfiard@google.com

[qfiard, 2017-03-02 21:48:33]

The patchset sent to the CQ was uploaded after l-g-t-m from
benwells@chromium.org, tedchoc@chromium.org
Link to the patchset: https://codereview.chromium.org/2709883005/#ps220001
(title: "Group location settings dialog enums into a single java_cpp_enum rule")

[commit-bot: I haz the power, 2017-03-02 21:49:05]

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, 2017-03-02 23:41:23]

CQ is committing da patch.

Bot data: {"patchset_id": 220001, "attempt_start_ts": 1488491312727750,
"parent_rev": "73a34473d6cc86352f226be7186e1e292fe35734", "commit_rev":
"6b1ebf77022a1754cedb39c47fb94a0a1c9f8777"}

[commit-bot: I haz the power, 2017-03-02 23:42:03]

Description was changed from

==========
Stubs for triggering the LSD in Clank

BUG=672301
==========

to

==========
Stubs for triggering the LSD in Clank

BUG=672301

Review-Url: https://codereview.chromium.org/2709883005
Cr-Commit-Position: refs/heads/master@{#454435}
Committed:
https://chromium.googlesource.com/chromium/src/+/6b1ebf77022a1754cedb39c47fb9...
==========

[commit-bot: I haz the power, 2017-03-02 23:42:06]

Committed patchset #12 (id:220001) as
https://chromium.googlesource.com/chromium/src/+/6b1ebf77022a1754cedb39c47fb9...