bluetooth: Android adapter can be created with and without Bluetooth permission.

bluetooth: Android adapter can be created with and without Bluetooth permission.

This enables unit tests to run with Bluetooth permission,
by adding the Bluetooth permission to native_test.

Non-test applications will not have the Bluetooth permission,
and unit tests should also verify behavior
when the permission is not given. To enable this
createWithoutPermissionForTesting is added and
results in a state equivalent to when the permission
is not available.

Unit tests will be built in parallel for both when Bluetooth
permission exists and doesn't.

BUG=471536
Committed: https://crrev.com/30e81472dc168ea1c331a8779b3f6a14f684c54b
Cr-Commit-Position: refs/heads/master@{#329212}

Committed: https://crrev.com/4261538f2e98c39395931e9e79fc5a3a4ddbf23f
Cr-Commit-Position: refs/heads/master@{#329286}

[scheib, 2015-05-06 01:39:20]

Patchset #1 (id:1) has been deleted

[scheib, 2015-05-07 19:46:01]

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

[scheib, 2015-05-07 19:46:02]

Ted, PTAL as we discussed Bluetooth permission some already.

[scheib, 2015-05-07 20:56:50]

Patchset #1 (id:20001) has been deleted

[Ted C, 2015-05-07 22:23:57]

https://codereview.chromium.org/1129683002/diff/50006/device/bluetooth/androi...
File
device/bluetooth/android/java/src/org/chromium/device/bluetooth/BluetoothAdapter.java
(right):

https://codereview.chromium.org/1129683002/diff/50006/device/bluetooth/androi...
device/bluetooth/android/java/src/org/chromium/device/bluetooth/BluetoothAdapter.java:32:
return new BluetoothAdapter();
I would look to see if you can actually do something like:

Context wrappedContext = new ContextWrapper(context) {
    @Override
    public int checkCallingOrSelfPermission(String permission) {
        return PackageManager.PERMISSION_DENIED;
    }
};

Then call the same constructor as you would otherwise.  Then you do not have to
worry about them diverging overtime.

https://codereview.chromium.org/1129683002/diff/50006/device/bluetooth/androi...
device/bluetooth/android/java/src/org/chromium/device/bluetooth/BluetoothAdapter.java:38:
context.checkCallingOrSelfPermission(android.Manifest.permission.BLUETOOTH)
you should be able to import android.Manifest to drop the leading "android."

https://codereview.chromium.org/1129683002/diff/50006/device/bluetooth/androi...
device/bluetooth/android/java/src/org/chromium/device/bluetooth/BluetoothAdapter.java:50:
Log.w(TAG, "No adapter found.");
this looks like it should fit on the previous line in which case you can drop
the braces (in java if the statement and conditional all fit on one line, then
braces aren't required).

Also, this seems like a info log since it is most likely null due to the device
lacking bluetooth support

https://codereview.chromium.org/1129683002/diff/50006/device/bluetooth/blueto...
File device/bluetooth/bluetooth_adapter_android.h (right):

https://codereview.chromium.org/1129683002/diff/50006/device/bluetooth/blueto...
device/bluetooth/bluetooth_adapter_android.h:20: class DEVICE_BLUETOOTH_EXPORT
BluetoothAdapterAndroid final
FWIW, you can call the java side BluetoothAdapterAndroid to keep it in sync with
the native side of things and then you won't have the name collision problem you
have.

With that said, it is a bit weird since it's not actually the android bluetooth
adapter but instead chrome's.  Also, the shed should be light blue.

https://codereview.chromium.org/1129683002/diff/50006/device/bluetooth/blueto...
File device/bluetooth/bluetooth_adapter_android_unittest.cc (right):

https://codereview.chromium.org/1129683002/diff/50006/device/bluetooth/blueto...
device/bluetooth/bluetooth_adapter_android_unittest.cc:29:
EXPECT_TRUE(adapter_->HasBluetoothPermission());
what happens if these tests are running on a device that doesn't actually
support bluetooth?

Isn't this dependent on the type of bots it is being run on?

[scheib, 2015-05-07 23:23:33]

Patchset #1 (id:40001) has been deleted

[scheib, 2015-05-07 23:23:43]

Patchset #1 (id:60001) has been deleted

[scheib, 2015-05-07 23:23:54]

Patchset #1 (id:80001) has been deleted

[scheib, 2015-05-07 23:59:17]

Thanks, PTAL

https://codereview.chromium.org/1129683002/diff/50006/device/bluetooth/androi...
File
device/bluetooth/android/java/src/org/chromium/device/bluetooth/BluetoothAdapter.java
(right):

https://codereview.chromium.org/1129683002/diff/50006/device/bluetooth/androi...
device/bluetooth/android/java/src/org/chromium/device/bluetooth/BluetoothAdapter.java:32:
return new BluetoothAdapter();
On 2015/05/07 22:23:57, Ted C wrote:
> I would look to see if you can actually do something like:
> 
> Context wrappedContext = new ContextWrapper(context) {
>     @Override
>     public int checkCallingOrSelfPermission(String permission) {
>         return PackageManager.PERMISSION_DENIED;
>     }
> };
> 
> Then call the same constructor as you would otherwise.  Then you do not have
to
> worry about them diverging overtime.

Done.

https://codereview.chromium.org/1129683002/diff/50006/device/bluetooth/androi...
device/bluetooth/android/java/src/org/chromium/device/bluetooth/BluetoothAdapter.java:38:
context.checkCallingOrSelfPermission(android.Manifest.permission.BLUETOOTH)
On 2015/05/07 22:23:57, Ted C wrote:
> you should be able to import android.Manifest to drop the leading "android."

Done.

https://codereview.chromium.org/1129683002/diff/50006/device/bluetooth/androi...
device/bluetooth/android/java/src/org/chromium/device/bluetooth/BluetoothAdapter.java:50:
Log.w(TAG, "No adapter found.");
On 2015/05/07 22:23:57, Ted C wrote:
> this looks like it should fit on the previous line in which case you can drop
> the braces (in java if the statement and conditional all fit on one line, then
> braces aren't required).
> 
> Also, this seems like a info log since it is most likely null due to the
device
> lacking bluetooth support

Done.

https://codereview.chromium.org/1129683002/diff/50006/device/bluetooth/blueto...
File device/bluetooth/bluetooth_adapter_android.h (right):

https://codereview.chromium.org/1129683002/diff/50006/device/bluetooth/blueto...
device/bluetooth/bluetooth_adapter_android.h:20: class DEVICE_BLUETOOTH_EXPORT
BluetoothAdapterAndroid final
On 2015/05/07 22:23:57, Ted C wrote:
> FWIW, you can call the java side BluetoothAdapterAndroid to keep it in sync
with
> the native side of things and then you won't have the name collision problem
you
> have.
> 
> With that said, it is a bit weird since it's not actually the android
bluetooth
> adapter but instead chrome's.  Also, the shed should be light blue.

Thanks. I've pondered a bit. For now I'm going to keep the java class name
shorter so the C++ calls to it are simpler, as there are more of those so far vs
the number of fully qualified android.bluetooth.BluetoothAdapter I have to use
in java classes (which also get 100 columns, so a bit more space there).

https://codereview.chromium.org/1129683002/diff/50006/device/bluetooth/blueto...
File device/bluetooth/bluetooth_adapter_android_unittest.cc (right):

https://codereview.chromium.org/1129683002/diff/50006/device/bluetooth/blueto...
device/bluetooth/bluetooth_adapter_android_unittest.cc:29:
EXPECT_TRUE(adapter_->HasBluetoothPermission());
On 2015/05/07 22:23:57, Ted C wrote:
> what happens if these tests are running on a device that doesn't actually
> support bluetooth?
> 
> Isn't this dependent on the type of bots it is being run on?

Good point. I've guarded the unit test with a detection on if the adapter is
present.

[scheib, 2015-05-08 00:38:36]

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

[scheib, 2015-05-08 00:38:36]

Arman, PTAL

[Ted C, 2015-05-08 17:33:33]

lgtm

[scheib, 2015-05-11 03:37:23]

Arman, PTAL

[armansito, 2015-05-11 05:44:13]

lgtm

[scheib, 2015-05-11 17:40:49]

The CQ bit was checked by scheib@chromium.org

[commit-bot: I haz the power, 2015-05-11 17:46:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1129683002/110001

[commit-bot: I haz the power, 2015-05-11 18:52:56]

Committed patchset #2 (id:110001)

[commit-bot: I haz the power, 2015-05-11 18:54:50]

Patchset 2 (id:??) landed as
https://crrev.com/30e81472dc168ea1c331a8779b3f6a14f684c54b
Cr-Commit-Position: refs/heads/master@{#329212}

[jbudorick, 2015-05-11 21:20:03]

jbudorick@chromium.org changed reviewers:
+ jbudorick@chromium.org

[jbudorick, 2015-05-11 21:20:04]

https://codereview.chromium.org/1129683002/diff/110001/device/bluetooth/bluet...
File device/bluetooth/bluetooth_adapter_android_unittest.cc (right):

https://codereview.chromium.org/1129683002/diff/110001/device/bluetooth/bluet...
device/bluetooth/bluetooth_adapter_android_unittest.cc:26:
TEST_F(BluetoothAdapterAndroidTest, Construct) {
This test failed on its first run through Android Tests (dbg):
http://build.chromium.org/p/chromium.linux/builders/Android%20Tests%20%28dbg%...

It appears that device_unittests does not run on the release bots on the CQ or
main waterfall (which is probably an oversight), and we (intentionally, IIRC)
don't have a debug tester bot on the CQ.

[jbudorick, 2015-05-11 21:36:16]

On 2015/05/11 21:20:04, jbudorick wrote:
>
https://codereview.chromium.org/1129683002/diff/110001/device/bluetooth/bluet...
> File device/bluetooth/bluetooth_adapter_android_unittest.cc (right):
> 
>
https://codereview.chromium.org/1129683002/diff/110001/device/bluetooth/bluet...
> device/bluetooth/bluetooth_adapter_android_unittest.cc:26:
> TEST_F(BluetoothAdapterAndroidTest, Construct) {
> This test failed on its first run through Android Tests (dbg):
>
http://build.chromium.org/p/chromium.linux/builders/Android%20Tests%20%28dbg%...
> 
> It appears that device_unittests does not run on the release bots on the CQ or
> main waterfall (which is probably an oversight), and we (intentionally, IIRC)
> don't have a debug tester bot on the CQ.

It failed in 27824 as well, so I'm reverting this CL. I'll upload a change to
add device_unittests to the rel bots on the CQ + main waterfall.

[jbudorick, 2015-05-11 21:36:48]

A revert of this CL (patchset #2 id:110001) has been created in
https://codereview.chromium.org/1137083002/ by jbudorick@chromium.org.

The reason for reverting is: Failing on the main waterfall:
http://build.chromium.org/p/chromium.linux/builders/Android%20Tests%20%28dbg%....

[scheib, 2015-05-11 23:07:45]

The CQ bit was checked by scheib@chromium.org

[scheib, 2015-05-11 23:07:45]

The patchset sent to the CQ was uploaded after l-g-t-m from
tedchoc@chromium.org, armansito@chromium.org
Link to the patchset: https://codereview.chromium.org/1129683002/#ps130001
(title: "Reland: IsPowered may be false on test infra.")

[commit-bot: I haz the power, 2015-05-11 23:11:22]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1129683002/130001

[commit-bot: I haz the power, 2015-05-12 00:09:50]

Committed patchset #3 (id:130001)

[commit-bot: I haz the power, 2015-05-12 00:11:24]

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