Add a service to handle postMessage callbacks to client

Add a service to handle postMessage callbacks to client

BUG=

Review-Url: https://codereview.chromium.org/2591513002
Committed: https://github.com/GoogleChrome/custom-tabs-client/commit/f68fb050a78be781cbaacb7a34a96ca902dda0f3

[Yusuf, 2017-01-23 22:04:56]

yusufo@chromium.org changed reviewers:
+ lizeb@chromium.org

[Benoit L, 2017-01-25 17:36:46]

https://codereview.chromium.org/2591513002/diff/80001/customtabs/src/android/...
File customtabs/src/android/support/customtabs/PostMessageService.java (right):

https://codereview.chromium.org/2591513002/diff/80001/customtabs/src/android/...
customtabs/src/android/support/customtabs/PostMessageService.java:2: * Copyright
(C) 2015 The Android Open Source Project
nit: date?

https://codereview.chromium.org/2591513002/diff/80001/customtabs/src/android/...
File customtabs/src/android/support/customtabs/PostMessageServiceConnection.java
(right):

https://codereview.chromium.org/2591513002/diff/80001/customtabs/src/android/...
customtabs/src/android/support/customtabs/PostMessageServiceConnection.java:2: *
Copyright (C) 2015 The Android Open Source Project
nit: date?

[Benoit L, 2017-01-25 21:25:03]

lgtm with nits, thanks.

https://codereview.chromium.org/2591513002/diff/80001/customtabs/src/android/...
File customtabs/src/android/support/customtabs/CustomTabsService.java (right):

https://codereview.chromium.org/2591513002/diff/80001/customtabs/src/android/...
customtabs/src/android/support/customtabs/CustomTabsService.java:247: * {@link
PostMessageServiceConnection#notifyMessageChannelReady(Bundle)} is received on
the client
nit: long line?

https://codereview.chromium.org/2591513002/diff/80001/customtabs/src/android/...
File customtabs/src/android/support/customtabs/CustomTabsSession.java (right):

https://codereview.chromium.org/2591513002/diff/80001/customtabs/src/android/...
customtabs/src/android/support/customtabs/CustomTabsSession.java:182: * @return
An integer constant about the postMessage request result.
nit: Perhaps specify where are these constants? (The Result "enum")

https://codereview.chromium.org/2591513002/diff/80001/customtabs/src/android/...
File customtabs/src/android/support/customtabs/PostMessageService.java (right):

https://codereview.chromium.org/2591513002/diff/80001/customtabs/src/android/...
customtabs/src/android/support/customtabs/PostMessageService.java:21: import
android.net.Uri;
nit: not needed.

https://codereview.chromium.org/2591513002/diff/80001/customtabs/src/android/...
customtabs/src/android/support/customtabs/PostMessageService.java:23: import
android.os.IBinder;
nit: not needed.

https://codereview.chromium.org/2591513002/diff/80001/customtabs/src/android/...
customtabs/src/android/support/customtabs/PostMessageService.java:27: import
android.support.v4.util.ArrayMap;
dittp for the 4 imports above.

https://codereview.chromium.org/2591513002/diff/80001/customtabs/src/android/...
customtabs/src/android/support/customtabs/PostMessageService.java:31: import
java.util.NoSuchElementException;
none of these imports are needed.

https://codereview.chromium.org/2591513002/diff/80001/customtabs/src/android/...
customtabs/src/android/support/customtabs/PostMessageService.java:43:
callback.onMessageChannelReady(extras);
nit: This is indented too much (4 extra spaces)

https://codereview.chromium.org/2591513002/diff/80001/customtabs/src/android/...
File customtabs/src/android/support/customtabs/PostMessageServiceConnection.java
(right):

https://codereview.chromium.org/2591513002/diff/80001/customtabs/src/android/...
customtabs/src/android/support/customtabs/PostMessageServiceConnection.java:82:
* @param extras Reserved for future use.
nit: Javadoc for the return value?

https://codereview.chromium.org/2591513002/diff/80001/customtabs/src/android/...
customtabs/src/android/support/customtabs/PostMessageServiceConnection.java:98:
* {@link CustomTabsSession} has sent a postMessage. If postMessage() is called
from a single
Actually, I think that we provide an even better guarantee here, but the doc is
correct. That is, is there is *some* well-defined ordering, we preserve it.

For instance, posting from several threads, but with locks to synchronize
things. It would probably make the doc less clear, so let's keep it as is.

https://codereview.chromium.org/2591513002/diff/80001/customtabs/src/android/...
customtabs/src/android/support/customtabs/PostMessageServiceConnection.java:102:
* @param extras Reserved for future use.
nit: Javadoc for return value?

[Yusuf, 2017-01-25 22:02:01]

https://codereview.chromium.org/2591513002/diff/80001/customtabs/src/android/...
File customtabs/src/android/support/customtabs/CustomTabsService.java (right):

https://codereview.chromium.org/2591513002/diff/80001/customtabs/src/android/...
customtabs/src/android/support/customtabs/CustomTabsService.java:247: * {@link
PostMessageServiceConnection#notifyMessageChannelReady(Bundle)} is received on
the client
On 2017/01/25 21:25:03, Benoit L wrote:
> nit: long line?

Done.

https://codereview.chromium.org/2591513002/diff/80001/customtabs/src/android/...
File customtabs/src/android/support/customtabs/CustomTabsSession.java (right):

https://codereview.chromium.org/2591513002/diff/80001/customtabs/src/android/...
customtabs/src/android/support/customtabs/CustomTabsSession.java:182: * @return
An integer constant about the postMessage request result.
On 2017/01/25 21:25:03, Benoit L wrote:
> nit: Perhaps specify where are these constants? (The Result "enum")

The most viable way to refer to the enum was to refer to one of the constants.

https://codereview.chromium.org/2591513002/diff/80001/customtabs/src/android/...
File customtabs/src/android/support/customtabs/PostMessageService.java (right):

https://codereview.chromium.org/2591513002/diff/80001/customtabs/src/android/...
customtabs/src/android/support/customtabs/PostMessageService.java:2: * Copyright
(C) 2015 The Android Open Source Project
On 2017/01/25 17:36:46, Benoit L wrote:
> nit: date?

Done.

https://codereview.chromium.org/2591513002/diff/80001/customtabs/src/android/...
customtabs/src/android/support/customtabs/PostMessageService.java:21: import
android.net.Uri;
On 2017/01/25 21:25:03, Benoit L wrote:
> nit: not needed.

Done.

https://codereview.chromium.org/2591513002/diff/80001/customtabs/src/android/...
customtabs/src/android/support/customtabs/PostMessageService.java:23: import
android.os.IBinder;
On 2017/01/25 21:25:03, Benoit L wrote:
> nit: not needed.

Done.

https://codereview.chromium.org/2591513002/diff/80001/customtabs/src/android/...
customtabs/src/android/support/customtabs/PostMessageService.java:27: import
android.support.v4.util.ArrayMap;
On 2017/01/25 21:25:03, Benoit L wrote:
> dittp for the 4 imports above.

Done.

https://codereview.chromium.org/2591513002/diff/80001/customtabs/src/android/...
customtabs/src/android/support/customtabs/PostMessageService.java:31: import
java.util.NoSuchElementException;
On 2017/01/25 21:25:03, Benoit L wrote:
> none of these imports are needed.

Done.

https://codereview.chromium.org/2591513002/diff/80001/customtabs/src/android/...
customtabs/src/android/support/customtabs/PostMessageService.java:43:
callback.onMessageChannelReady(extras);
On 2017/01/25 21:25:03, Benoit L wrote:
> nit: This is indented too much (4 extra spaces)

Done.

https://codereview.chromium.org/2591513002/diff/80001/customtabs/src/android/...
File customtabs/src/android/support/customtabs/PostMessageServiceConnection.java
(right):

https://codereview.chromium.org/2591513002/diff/80001/customtabs/src/android/...
customtabs/src/android/support/customtabs/PostMessageServiceConnection.java:2: *
Copyright (C) 2015 The Android Open Source Project
On 2017/01/25 17:36:46, Benoit L wrote:
> nit: date?

Done.

https://codereview.chromium.org/2591513002/diff/80001/customtabs/src/android/...
customtabs/src/android/support/customtabs/PostMessageServiceConnection.java:82:
* @param extras Reserved for future use.
On 2017/01/25 21:25:03, Benoit L wrote:
> nit: Javadoc for the return value?

Done.

https://codereview.chromium.org/2591513002/diff/80001/customtabs/src/android/...
customtabs/src/android/support/customtabs/PostMessageServiceConnection.java:98:
* {@link CustomTabsSession} has sent a postMessage. If postMessage() is called
from a single
On 2017/01/25 21:25:03, Benoit L wrote:
> Actually, I think that we provide an even better guarantee here, but the doc
is
> correct. That is, is there is *some* well-defined ordering, we preserve it.
> 
> For instance, posting from several threads, but with locks to synchronize
> things. It would probably make the doc less clear, so let's keep it as is.

Acknowledged.

https://codereview.chromium.org/2591513002/diff/80001/customtabs/src/android/...
customtabs/src/android/support/customtabs/PostMessageServiceConnection.java:102:
* @param extras Reserved for future use.
On 2017/01/25 21:25:03, Benoit L wrote:
> nit: Javadoc for return value?

Done.

[Yusuf, 2017-01-25 22:02:17]

The CQ bit was checked by yusufo@chromium.org

[Yusuf, 2017-01-25 22:02:18]

The patchset sent to the CQ was uploaded after l-g-t-m from lizeb@chromium.org
Link to the patchset: https://codereview.chromium.org/2591513002/#ps120001
(title: "lizeb@ nits")

[commit-bot: I haz the power, 2017-01-25 22:02:19]

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-01-25 22:03:15]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1485381737722040,
"parent_rev": "2de63c2bd2c918f300bc1c2c2d53403edd6c796e", "commit_rev":
"f68fb050a78be781cbaacb7a34a96ca902dda0f3"}

[commit-bot: I haz the power, 2017-01-25 22:03:20]

Description was changed from

==========
Add a service to handle postMessage callbacks to client

BUG=
==========

to

==========
Add a service to handle postMessage callbacks to client

BUG=

Review-Url: https://codereview.chromium.org/2591513002
Committed:
https://github.com/GoogleChrome/custom-tabs-client/commit/f68fb050a78be781cba...
==========

[commit-bot: I haz the power, 2017-01-25 22:03:21]

Committed patchset #7 (id:120001) as
https://github.com/GoogleChrome/custom-tabs-client/commit/f68fb050a78be781cba...