If location services are turned off, have the BT chooser prompt the user to turn them on.

If location services are turned off, have the BT chooser prompt the user to turn them on.

Based on jyasskin's patch at http://crrev.com/2032273002

This patch just fixes conflicts and uses the new method to control Location Services
state

BUG=545867
Committed: https://crrev.com/0fbab99112bf947322e9b1dc0f3612e0b23f29fe
Cr-Commit-Position: refs/heads/master@{#403597}

[ortuno, 2016-06-27 21:50:57]

Patchset #1 (id:1) has been deleted

[ortuno, 2016-06-27 21:59:55]

Description was changed from

==========
If location services are turned off, have the BT chooser prompt the user to turn
them on.

BUG=545867
==========

to

==========
If location services are turned off, have the BT chooser prompt the user to turn
them on.

Based on jyasskin's patch at http://crrev.com/2032273002

This patch just fixes conflicts and uses the new method to control Location
Services
state

BUG=545867
==========

[ortuno, 2016-06-27 22:04:10]

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

[commit-bot: I haz the power, 2016-06-27 22:04:50]

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-06-28 01:11:23]

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

[commit-bot: I haz the power, 2016-06-28 01:11:24]

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

[ortuno, 2016-06-29 16:43:28]

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

[commit-bot: I haz the power, 2016-06-29 16:46:05]

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

[ortuno, 2016-06-29 17:07:09]

ortuno@chromium.org changed reviewers:
+ finnur@chromium.org

[ortuno, 2016-06-29 17:07:09]

finnur: PTAL at chrome/android/

[commit-bot: I haz the power, 2016-06-29 19:41:01]

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

[commit-bot: I haz the power, 2016-06-29 19:41:02]

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

[ortuno, 2016-06-30 19:28:52]

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

[ortuno, 2016-06-30 19:28:53]

tedchoc: Could you take a look at chrome/android? Finnur is in another timezone
so I don't think he'll have to time to review and we really want to land this
before branch point. If not, then it's OK!

[Ted C, 2016-07-01 20:18:35]

lgtm

https://codereview.chromium.org/2102813002/diff/60001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/BluetoothChooserDialog.java
(right):

https://codereview.chromium.org/2102813002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/BluetoothChooserDialog.java:140:
new IntentFilter(LocationManager.MODE_CHANGED_ACTION));
does this give a lint warning?  MODE_CHANGED_ACTION is Api 19 and I don't see
any TargetApi annotations about it

https://codereview.chromium.org/2102813002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/BluetoothChooserDialog.java:175:
final BroadcastReceiver mLocationModeBroadcastReceiver = new BroadcastReceiver()
{
move this up to be with the other params (but at the bottom of them)

https://codereview.chromium.org/2102813002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/BluetoothChooserDialog.java:178:
if (LocationManager.MODE_CHANGED_ACTION.equals(intent.getAction())) {
make this an early return to avoid indenting

[ortuno, 2016-07-01 22:08:50]

Thanks!

https://codereview.chromium.org/2102813002/diff/60001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/BluetoothChooserDialog.java
(right):

https://codereview.chromium.org/2102813002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/BluetoothChooserDialog.java:140:
new IntentFilter(LocationManager.MODE_CHANGED_ACTION));
On 2016/07/01 at 20:18:34, Ted C wrote:
> does this give a lint warning?  MODE_CHANGED_ACTION is Api 19 and I don't see
any TargetApi annotations about it

Hmm no lint errors. Is there anything I should do about it?

https://codereview.chromium.org/2102813002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/BluetoothChooserDialog.java:175:
final BroadcastReceiver mLocationModeBroadcastReceiver = new BroadcastReceiver()
{
On 2016/07/01 at 20:18:34, Ted C wrote:
> move this up to be with the other params (but at the bottom of them)

Done.

https://codereview.chromium.org/2102813002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/BluetoothChooserDialog.java:178:
if (LocationManager.MODE_CHANGED_ACTION.equals(intent.getAction())) {
On 2016/07/01 at 20:18:34, Ted C wrote:
> make this an early return to avoid indenting

Done.

[ortuno, 2016-07-01 22:10:49]

ortuno@chromium.org changed reviewers:
+ scheib@chromium.org

[ortuno, 2016-07-01 22:10:50]

scheib: PTAL at device/

[scheib, 2016-07-01 22:18:44]

LGTM

[Ted C, 2016-07-01 22:22:56]

https://codereview.chromium.org/2102813002/diff/60001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/BluetoothChooserDialog.java
(right):

https://codereview.chromium.org/2102813002/diff/60001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/BluetoothChooserDialog.java:140:
new IntentFilter(LocationManager.MODE_CHANGED_ACTION));
On 2016/07/01 22:08:50, ortuno wrote:
> On 2016/07/01 at 20:18:34, Ted C wrote:
> > does this give a lint warning?  MODE_CHANGED_ACTION is Api 19 and I don't
see
> any TargetApi annotations about it
> 
> Hmm no lint errors. Is there anything I should do about it?

While we lack the bluetooth permission prior to M, is there anything that
prevents this code from running prior to API 19?

Since this is in the Chrome package, we don't need to be overly cautious but if
this were below content/ then we'd probably want to ensure developers of a
consistent behavior (where prior to 19 this wouldn't actually ever be called).

For now, if there are no lint errors then I guess you're fine?

[ortuno, 2016-07-01 22:37:00]

On 2016/07/01 at 22:22:56, tedchoc wrote:
>
https://codereview.chromium.org/2102813002/diff/60001/chrome/android/java/src...
> File
chrome/android/java/src/org/chromium/chrome/browser/BluetoothChooserDialog.java
(right):
> 
>
https://codereview.chromium.org/2102813002/diff/60001/chrome/android/java/src...
>
chrome/android/java/src/org/chromium/chrome/browser/BluetoothChooserDialog.java:140:
new IntentFilter(LocationManager.MODE_CHANGED_ACTION));
> On 2016/07/01 22:08:50, ortuno wrote:
> > On 2016/07/01 at 20:18:34, Ted C wrote:
> > > does this give a lint warning?  MODE_CHANGED_ACTION is Api 19 and I don't
see
> > any TargetApi annotations about it
> > 
> > Hmm no lint errors. Is there anything I should do about it?
> 
> While we lack the bluetooth permission prior to M, is there anything that
prevents this code from running prior to API 19?
> 

Web Bluetooth will never call this code in <21 because the BLE APIs we depend on
are defined in Lollipop.

> Since this is in the Chrome package, we don't need to be overly cautious but
if this were below content/ then we'd probably want to ensure developers of a
consistent behavior (where prior to 19 this wouldn't actually ever be called).
>

I see.
 
> For now, if there are no lint errors then I guess you're fine?

Got it. Thanks for the explanation!

[ortuno, 2016-07-01 22:37:06]

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

[commit-bot: I haz the power, 2016-07-01 22:37:25]

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-07-02 00:02:29]

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

[commit-bot: I haz the power, 2016-07-02 00:02:31]

Dry run: This issue passed the CQ dry run.

[ortuno, 2016-07-02 03:03:40]

The CQ bit was checked by ortuno@chromium.org

[ortuno, 2016-07-02 03:03:41]

The patchset sent to the CQ was uploaded after l-g-t-m from tedchoc@chromium.org
Link to the patchset: https://codereview.chromium.org/2102813002/#ps80001
(title: "Address tedchoc's comments")

[commit-bot: I haz the power, 2016-07-02 03:03:57]

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-07-02 03:08:55]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2016-07-02 03:08:59]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-02 03:10:07]

Description was changed from

==========
If location services are turned off, have the BT chooser prompt the user to turn
them on.

Based on jyasskin's patch at http://crrev.com/2032273002

This patch just fixes conflicts and uses the new method to control Location
Services
state

BUG=545867
==========

to

==========
If location services are turned off, have the BT chooser prompt the user to turn
them on.

Based on jyasskin's patch at http://crrev.com/2032273002

This patch just fixes conflicts and uses the new method to control Location
Services
state

BUG=545867

Committed: https://crrev.com/0fbab99112bf947322e9b1dc0f3612e0b23f29fe
Cr-Commit-Position: refs/heads/master@{#403597}
==========

[commit-bot: I haz the power, 2016-07-02 03:10:09]

Patchset 4 (id:??) landed as
https://crrev.com/0fbab99112bf947322e9b1dc0f3612e0b23f29fe
Cr-Commit-Position: refs/heads/master@{#403597}