Hold messages in message port service when a message port is

Hold messages in message port service when a message port is
being transferred.

This CL addresses a long standing TODO for addressing message queuing.
When Message ports are in flight, race conditions that lead to dropped
messages, or ghost messages (messages arriving for already transferred
ports can occur). To prevent this, messages are queued at the
message port service during transfer.

Note that this CL also restricts transferring message ports from
Java to JS under certain circumstances. Basically, message ports
that are already used for communication from Java to JS
are not allowed to be transferred at the time. This
restriction can be relaxed later.

BUG=463269
Committed: https://crrev.com/79815049c8dcde0c59aba4e3ee1dd85048c734f0
Cr-Commit-Position: refs/heads/master@{#319943}

[sgurun-gerrit only, 2015-03-06 21:57:20]

sgurun@chromium.org changed reviewers:
+ hush@chromium.org, nasko@chromium.org

[sgurun-gerrit only, 2015-03-06 21:57:50]

On 2015/03/06 21:57:20, sgurun wrote:
> mailto:sgurun@chromium.org changed reviewers:
> + mailto:hush@chromium.org, mailto:nasko@chromium.org

hush please review android_webview/
nasko please review content/

thanks!

[hush (inactive), 2015-03-06 22:32:37]

lgtm

https://codereview.chromium.org/986553004/diff/1/android_webview/java/src/org...
File android_webview/java/src/org/chromium/android_webview/MessagePort.java
(right):

https://codereview.chromium.org/986553004/diff/1/android_webview/java/src/org...
android_webview/java/src/org/chromium/android_webview/MessagePort.java:54: * 
10.  c2.port1.postMessage("test2",[c1.port2])
 indent to the left by 1

https://codereview.chromium.org/986553004/diff/1/android_webview/native/aw_me...
File android_webview/native/aw_message_port_service_impl.cc (right):

https://codereview.chromium.org/986553004/diff/1/android_webview/native/aw_me...
android_webview/native/aw_message_port_service_impl.cc:154: int message_port_id)
{
int --> jint

[sgurun-gerrit only, 2015-03-06 22:51:34]

https://codereview.chromium.org/986553004/diff/1/android_webview/java/src/org...
File android_webview/java/src/org/chromium/android_webview/MessagePort.java
(right):

https://codereview.chromium.org/986553004/diff/1/android_webview/java/src/org...
android_webview/java/src/org/chromium/android_webview/MessagePort.java:54: * 
10.  c2.port1.postMessage("test2",[c1.port2])
On 2015/03/06 22:32:37, hush wrote:
>  indent to the left by 1

Done.

[nasko, 2015-03-09 18:39:01]

I don't quite follow why the two new methods are needed. Can you help me by
explaining the flow of method calls?

[sgurun-gerrit only, 2015-03-09 20:38:21]

On 2015/03/09 18:39:01, nasko wrote:
> I don't quite follow why the two new methods are needed. Can you help me by
> explaining the flow of method calls?


When webview embedder app creates a message channel (portA,portB), and transfers
portB to a destination, the destination can immediately start posting messages
on portB. These messages should be queued somewhere until portA is ready to
receive messages, i.e. it has the equivalent of onmessage handler to receive
these. This allows us queue the messages in message port service.

Rather than queueing the messages in message port service using holdmessages, we
could implement queueing in Android webview side. However this would:
1. Duplicate the already existing logic unnecessarily, 
2. We would have to find a way to transfer the queued messages in webview to the
right end if portA is transferred to a different destination, which would still
require exposing some other API to message port service.

These two methods allow us use the existing facility to hold messages until
webview embedder app is ready to receive.

As a second problem, you might be curious on what would happen if embedder app
transfers the message port after it sets a handler. For now, we simply not allow
it. We can chat on it offline, but I feel like it is not a significant
restriction. Further, not allowing is semantically a lot cleaner. We can remove
this restriction if there are real use cases though.

[nasko, 2015-03-10 17:53:04]

As we discussed, once this is working, we should find a better way to implement
it and have it be simpler and more performant.
For now, LGTM.

[sgurun-gerrit only, 2015-03-10 18:03:39]

The CQ bit was checked by sgurun@chromium.org

[sgurun-gerrit only, 2015-03-10 18:03:39]

The patchset sent to the CQ was uploaded after l-g-t-m from hush@chromium.org
Link to the patchset: https://codereview.chromium.org/986553004/#ps20001 (title:
"address hush nit")

[commit-bot: I haz the power, 2015-03-10 18:04:24]

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

[commit-bot: I haz the power, 2015-03-10 19:27:57]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-03-10 19:28:36]

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