bluetooth: android: register for adapter on/off events.

bluetooth: android: register for adapter on/off events.

Register for BluetoothAdapter.ACTION_STATE_CHANGED and reflect the
state changes in the device chooser dialog.

BUG=543060
Committed: https://crrev.com/3a38a463d379e27cb72a3c0334f76ecf4f8de8c4
Cr-Commit-Position: refs/heads/master@{#377982}

[perja, 2016-02-19 20:08:50]

perja@opera.com changed reviewers:
+ scheib@chromium.org, tedchoc@chromium.org

[perja, 2016-02-19 20:08:50]

This change makes the code aware of adapter on/off broadcast events and updates
the state of the device chooser dialog accordingly. Please take a look.

[scheib, 2016-02-26 00:05:59]

Thanks, LGTM. Please follow up with a patch to address following nits:

https://codereview.chromium.org/1711393002/diff/1/device/bluetooth/android/ja...
File
device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothAdapter.java
(right):

https://codereview.chromium.org/1711393002/diff/1/device/bluetooth/android/ja...
device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothAdapter.java:304:
Log.w(TAG, "BluetoothStateReceiver#onReceive action: %d", state);
This string doesn't mean much to me, would it be more direct as
"onReceive: BluetoothAdapter.ACTION_STATE_CHANGED: %d"
Would be nice to output string values for the 4 state values too, but that's
optional.

https://codereview.chromium.org/1711393002/diff/1/device/bluetooth/bluetooth_...
File device/bluetooth/bluetooth_adapter.h (right):

https://codereview.chromium.org/1711393002/diff/1/device/bluetooth/bluetooth_...
device/bluetooth/bluetooth_adapter.h:402: void NotifyAdapterStateChanged(bool
powered);
Merge with block of all Notify* methods below, and fix that comment to not be
GATT specific.

[scheib, 2016-02-26 00:06:11]

The CQ bit was checked by scheib@chromium.org

[commit-bot: I haz the power, 2016-02-26 00:13:50]

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

[scheib, 2016-02-26 00:59:18]

Ted C PTAL as OWNER for these files:

chrome/android/java/src/org/chromium/chrome/browser/BluetoothChooserDialog.java

chrome/browser/ui/android/bluetooth_chooser_android.cc

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

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

[commit-bot: I haz the power, 2016-02-26 01:10:04]

Try jobs failed on following builders:
  linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[perja, 2016-02-26 10:51:04]

https://codereview.chromium.org/1711393002/diff/1/device/bluetooth/android/ja...
File
device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothAdapter.java
(right):

https://codereview.chromium.org/1711393002/diff/1/device/bluetooth/android/ja...
device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothAdapter.java:304:
Log.w(TAG, "BluetoothStateReceiver#onReceive action: %d", state);
On 2016/02/26 00:05:59, scheib wrote:
> This string doesn't mean much to me, would it be more direct as
> "onReceive: BluetoothAdapter.ACTION_STATE_CHANGED: %d"
> Would be nice to output string values for the 4 state values too, but that's
> optional.

Done.

https://codereview.chromium.org/1711393002/diff/1/device/bluetooth/bluetooth_...
File device/bluetooth/bluetooth_adapter.h (right):

https://codereview.chromium.org/1711393002/diff/1/device/bluetooth/bluetooth_...
device/bluetooth/bluetooth_adapter.h:402: void NotifyAdapterStateChanged(bool
powered);
On 2016/02/26 00:05:59, scheib wrote:
> Merge with block of all Notify* methods below, and fix that comment to not be
> GATT specific.

Done.

[Ted C, 2016-02-26 17:50:35]

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

https://codereview.chromium.org/1711393002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/BluetoothChooserDialog.java:325:
mItemChooserDialog.clear();
unless this line can somehow call closeDialog(), the native pointer check on the
next line can never be non-zero as this method is explicitly marked as called by
native.

https://codereview.chromium.org/1711393002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/BluetoothChooserDialog.java:327:
nativeRestartSearch(mNativeBluetoothChooserDialogPtr);
Why not have the native side just call this method right after calling
notifyAdapterTurnedOn?

https://codereview.chromium.org/1711393002/diff/40001/device/bluetooth/androi...
File
device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothAdapter.java
(right):

https://codereview.chromium.org/1711393002/diff/40001/device/bluetooth/androi...
device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothAdapter.java:37:
private Wrappers.BluetoothAdapterWrapper mAdapter;
can you mark this final?

https://codereview.chromium.org/1711393002/diff/40001/device/bluetooth/androi...
device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothAdapter.java:205:
mAdapter.getContext().unregisterReceiver(this);
my comment about marking the adapter final is to ensure the null-ness of it can
not change.  We want to make sure we are unregistering the receiver so we can't
have something setting it to null and having this be incorrectly skipped.

[scheib, 2016-02-26 20:23:47]

In order to land for M50 branch point I've addressed tedchoc's comments on a new
code review:
https://codereview.chromium.org/1745463002

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

https://codereview.chromium.org/1711393002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/BluetoothChooserDialog.java:325:
mItemChooserDialog.clear();
On 2016/02/26 17:50:35, Ted C wrote:
> unless this line can somehow call closeDialog(), the native pointer check on
the
> next line can never be non-zero as this method is explicitly marked as called
by
> native.

Done.

https://codereview.chromium.org/1711393002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/BluetoothChooserDialog.java:327:
nativeRestartSearch(mNativeBluetoothChooserDialogPtr);
On 2016/02/26 17:50:35, Ted C wrote:
> Why not have the native side just call this method right after calling
> notifyAdapterTurnedOn?

Done.

https://codereview.chromium.org/1711393002/diff/40001/device/bluetooth/androi...
File
device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothAdapter.java
(right):

https://codereview.chromium.org/1711393002/diff/40001/device/bluetooth/androi...
device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothAdapter.java:37:
private Wrappers.BluetoothAdapterWrapper mAdapter;
On 2016/02/26 17:50:35, Ted C wrote:
> can you mark this final?

Done.

https://codereview.chromium.org/1711393002/diff/40001/device/bluetooth/androi...
device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothAdapter.java:205:
mAdapter.getContext().unregisterReceiver(this);
On 2016/02/26 17:50:35, Ted C wrote:
> my comment about marking the adapter final is to ensure the null-ness of it
can
> not change.  We want to make sure we are unregistering the receiver so we
can't
> have something setting it to null and having this be incorrectly skipped.

Acknowledged.

[Ted C, 2016-02-26 20:51:32]

On 2016/02/26 20:23:47, scheib wrote:
> In order to land for M50 branch point I've addressed tedchoc's comments on a
new
> code review:
> https://codereview.chromium.org/1745463002
> 
>
https://codereview.chromium.org/1711393002/diff/40001/chrome/android/java/src...
> File
>
chrome/android/java/src/org/chromium/chrome/browser/BluetoothChooserDialog.java
> (right):
> 
>
https://codereview.chromium.org/1711393002/diff/40001/chrome/android/java/src...
>
chrome/android/java/src/org/chromium/chrome/browser/BluetoothChooserDialog.java:325:
> mItemChooserDialog.clear();
> On 2016/02/26 17:50:35, Ted C wrote:
> > unless this line can somehow call closeDialog(), the native pointer check on
> the
> > next line can never be non-zero as this method is explicitly marked as
called
> by
> > native.
> 
> Done.
> 
>
https://codereview.chromium.org/1711393002/diff/40001/chrome/android/java/src...
>
chrome/android/java/src/org/chromium/chrome/browser/BluetoothChooserDialog.java:327:
> nativeRestartSearch(mNativeBluetoothChooserDialogPtr);
> On 2016/02/26 17:50:35, Ted C wrote:
> > Why not have the native side just call this method right after calling
> > notifyAdapterTurnedOn?
> 
> Done.
> 
>
https://codereview.chromium.org/1711393002/diff/40001/device/bluetooth/androi...
> File
>
device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothAdapter.java
> (right):
> 
>
https://codereview.chromium.org/1711393002/diff/40001/device/bluetooth/androi...
>
device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothAdapter.java:37:
> private Wrappers.BluetoothAdapterWrapper mAdapter;
> On 2016/02/26 17:50:35, Ted C wrote:
> > can you mark this final?
> 
> Done.
> 
>
https://codereview.chromium.org/1711393002/diff/40001/device/bluetooth/androi...
>
device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothAdapter.java:205:
> mAdapter.getContext().unregisterReceiver(this);
> On 2016/02/26 17:50:35, Ted C wrote:
> > my comment about marking the adapter final is to ensure the null-ness of it
> can
> > not change.  We want to make sure we are unregistering the receiver so we
> can't
> > have something setting it to null and having this be incorrectly skipped.
> 
> Acknowledged.

Do you want to land your change, or should I lgtm this and then you land the
delta in your followup?

[scheib, 2016-02-26 20:55:44]

On 2016/02/26 20:51:32, Ted C wrote:
> On 2016/02/26 20:23:47, scheib wrote:
> > In order to land for M50 branch point I've addressed tedchoc's comments on a
> new
> > code review:
> > https://codereview.chromium.org/1745463002
> > 
> >
>
https://codereview.chromium.org/1711393002/diff/40001/chrome/android/java/src...
> > File
> >
>
chrome/android/java/src/org/chromium/chrome/browser/BluetoothChooserDialog.java
> > (right):
> > 
> >
>
https://codereview.chromium.org/1711393002/diff/40001/chrome/android/java/src...
> >
>
chrome/android/java/src/org/chromium/chrome/browser/BluetoothChooserDialog.java:325:
> > mItemChooserDialog.clear();
> > On 2016/02/26 17:50:35, Ted C wrote:
> > > unless this line can somehow call closeDialog(), the native pointer check
on
> > the
> > > next line can never be non-zero as this method is explicitly marked as
> called
> > by
> > > native.
> > 
> > Done.
> > 
> >
>
https://codereview.chromium.org/1711393002/diff/40001/chrome/android/java/src...
> >
>
chrome/android/java/src/org/chromium/chrome/browser/BluetoothChooserDialog.java:327:
> > nativeRestartSearch(mNativeBluetoothChooserDialogPtr);
> > On 2016/02/26 17:50:35, Ted C wrote:
> > > Why not have the native side just call this method right after calling
> > > notifyAdapterTurnedOn?
> > 
> > Done.
> > 
> >
>
https://codereview.chromium.org/1711393002/diff/40001/device/bluetooth/androi...
> > File
> >
>
device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothAdapter.java
> > (right):
> > 
> >
>
https://codereview.chromium.org/1711393002/diff/40001/device/bluetooth/androi...
> >
>
device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothAdapter.java:37:
> > private Wrappers.BluetoothAdapterWrapper mAdapter;
> > On 2016/02/26 17:50:35, Ted C wrote:
> > > can you mark this final?
> > 
> > Done.
> > 
> >
>
https://codereview.chromium.org/1711393002/diff/40001/device/bluetooth/androi...
> >
>
device/bluetooth/android/java/src/org/chromium/device/bluetooth/ChromeBluetoothAdapter.java:205:
> > mAdapter.getContext().unregisterReceiver(this);
> > On 2016/02/26 17:50:35, Ted C wrote:
> > > my comment about marking the adapter final is to ensure the null-ness of
it
> > can
> > > not change.  We want to make sure we are unregistering the receiver so we
> > can't
> > > have something setting it to null and having this be incorrectly skipped.
> > 
> > Acknowledged.
> 
> Do you want to land your change, or should I lgtm this and then you land the
> delta in your followup?

Let's land Per's change and I'll then land my follow up.

[scheib, 2016-02-26 20:56:07]

Description was changed from

==========
bluetooth: android: register for adapter on/off events.

Register for BluetoothAdapter.ACTION_STATE_CHANGED and reflect the
state changes in the device chooser dialog.

BUG=543060
==========

to

==========
bluetooth: android: register for adapter on/off events.

Register for BluetoothAdapter.ACTION_STATE_CHANGED and reflect the
state changes in the device chooser dialog.

BUG=543060
==========

[Ted C, 2016-02-26 20:57:57]

lgtm

[scheib, 2016-02-26 21:07:53]

The CQ bit was checked by scheib@chromium.org

[scheib, 2016-02-26 21:07:53]

The patchset sent to the CQ was uploaded after l-g-t-m from scheib@chromium.org
Link to the patchset: https://codereview.chromium.org/1711393002/#ps40001
(title: "Rebased on master")

[commit-bot: I haz the power, 2016-02-26 21:12:56]

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

[commit-bot: I haz the power, 2016-02-26 21:37:58]

Description was changed from

==========
bluetooth: android: register for adapter on/off events.

Register for BluetoothAdapter.ACTION_STATE_CHANGED and reflect the
state changes in the device chooser dialog.

BUG=543060
==========

to

==========
bluetooth: android: register for adapter on/off events.

Register for BluetoothAdapter.ACTION_STATE_CHANGED and reflect the
state changes in the device chooser dialog.

BUG=543060
==========

[commit-bot: I haz the power, 2016-02-26 21:38:00]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-02-26 21:39:14]

Description was changed from

==========
bluetooth: android: register for adapter on/off events.

Register for BluetoothAdapter.ACTION_STATE_CHANGED and reflect the
state changes in the device chooser dialog.

BUG=543060
==========

to

==========
bluetooth: android: register for adapter on/off events.

Register for BluetoothAdapter.ACTION_STATE_CHANGED and reflect the
state changes in the device chooser dialog.

BUG=543060

Committed: https://crrev.com/3a38a463d379e27cb72a3c0334f76ecf4f8de8c4
Cr-Commit-Position: refs/heads/master@{#377982}
==========

[commit-bot: I haz the power, 2016-02-26 21:39:17]

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