Verify that Message Channel Message Ports can be transferred correctly

Verify that Message Channel Message Ports can be transferred correctly

BUG=393291
Write a test to make sure Message Channel MesssagePorts can be
transferred correctly from Webview to JS by testing communication
between a Worker and the main frame. Also fix a bug discovered
by this test.

Committed: https://crrev.com/77dabb2bd7d5ce0e99c0ec2c44698ed66268bd47
Cr-Commit-Position: refs/heads/master@{#306449}

[sgurun-gerrit only, 2014-11-27 01:34:25]

sgurun@chromium.org changed reviewers:
+ jochen@chromium.org, mnaganov@chromium.org

[sgurun-gerrit only, 2014-11-27 01:35:03]

On 2014/11/27 01:34:25, sgurun wrote:
> mailto:sgurun@chromium.org changed reviewers:
> + mailto:jochen@chromium.org, mailto:mnaganov@chromium.org

mnaganov: Please review aw/
jochen: please review content/

thanks!

[mnaganov (inactive), 2014-11-27 08:41:31]

Looks good, but I have some comments about the test code arrangement.

https://codereview.chromium.org/766623002/diff/1/android_webview/javatests/sr...
File
android_webview/javatests/src/org/chromium/android_webview/test/PostMessageTest.java
(right):

https://codereview.chromium.org/766623002/diff/1/android_webview/javatests/sr...
android_webview/javatests/src/org/chromium/android_webview/test/PostMessageTest.java:38:
private static final String TEST_PAGE_FOR_PORT_TRANSFER =
Perhaps, it's better to put these scripts (and WORKER_MESSAGE const) inside or
near testTransferPortsToWorker function? They do not seem to be used by other
tests, anyway. Otherwise, one have to scroll the source back and forth.

https://codereview.chromium.org/766623002/diff/1/android_webview/javatests/sr...
android_webview/javatests/src/org/chromium/android_webview/test/PostMessageTest.java:42:
+ "        var messageChannelPort = null;"
You can declare this var inside "onmessage" handler body.

https://codereview.chromium.org/766623002/diff/1/android_webview/javatests/sr...
android_webview/javatests/src/org/chromium/android_webview/test/PostMessageTest.java:48:
+ "        onmessage = function (e) {"
Maybe swap "function receiveWorkerMessage" block with this one to match the
execution order? Alternatively, you can inline the definition of
"receiveWorkerMessage" at the point where you are assigning it to
"messageChannelPort.onmessage", as it's a almost a one-liner.

https://codereview.chromium.org/766623002/diff/1/android_webview/javatests/sr...
android_webview/javatests/src/org/chromium/android_webview/test/PostMessageTest.java:49:
+ "            if (e.data == \"from_webview\") {"
Use WEBVIEW_MESSAGE here

[jochen (gone - plz use gerrit), 2014-11-27 15:04:19]

content lgtm once mnaganov@ is happy with the change

[sgurun-gerrit only, 2014-12-01 19:11:29]

https://codereview.chromium.org/766623002/diff/1/android_webview/javatests/sr...
File
android_webview/javatests/src/org/chromium/android_webview/test/PostMessageTest.java
(right):

https://codereview.chromium.org/766623002/diff/1/android_webview/javatests/sr...
android_webview/javatests/src/org/chromium/android_webview/test/PostMessageTest.java:38:
private static final String TEST_PAGE_FOR_PORT_TRANSFER =
On 2014/11/27 08:41:30, mnaganov (cr) wrote:
> Perhaps, it's better to put these scripts (and WORKER_MESSAGE const) inside or
> near testTransferPortsToWorker function? They do not seem to be used by other
> tests, anyway. Otherwise, one have to scroll the source back and forth.

Done. In this case, for consistency, it is better to keep the other Javascript
code closer to their dependent test code, so moved them too.

https://codereview.chromium.org/766623002/diff/1/android_webview/javatests/sr...
android_webview/javatests/src/org/chromium/android_webview/test/PostMessageTest.java:42:
+ "        var messageChannelPort = null;"
On 2014/11/27 08:41:30, mnaganov (cr) wrote:
> You can declare this var inside "onmessage" handler body.

Done.

https://codereview.chromium.org/766623002/diff/1/android_webview/javatests/sr...
android_webview/javatests/src/org/chromium/android_webview/test/PostMessageTest.java:48:
+ "        onmessage = function (e) {"
On 2014/11/27 08:41:30, mnaganov (cr) wrote:
> Maybe swap "function receiveWorkerMessage" block with this one to match the
> execution order? Alternatively, you can inline the definition of
> "receiveWorkerMessage" at the point where you are assigning it to
> "messageChannelPort.onmessage", as it's a almost a one-liner.

ok swapped. Isn't it more readable though when the definitions comes earlier
then referencing a name? and I find it very hard to read when methods are nested
inside methods inside double quotes of java variables.

https://codereview.chromium.org/766623002/diff/1/android_webview/javatests/sr...
android_webview/javatests/src/org/chromium/android_webview/test/PostMessageTest.java:49:
+ "            if (e.data == \"from_webview\") {"
On 2014/11/27 08:41:30, mnaganov (cr) wrote:
> Use WEBVIEW_MESSAGE here

Done.

[mnaganov (inactive), 2014-12-02 09:49:38]

lgtm

https://codereview.chromium.org/766623002/diff/1/android_webview/javatests/sr...
File
android_webview/javatests/src/org/chromium/android_webview/test/PostMessageTest.java
(right):

https://codereview.chromium.org/766623002/diff/1/android_webview/javatests/sr...
android_webview/javatests/src/org/chromium/android_webview/test/PostMessageTest.java:48:
+ "        onmessage = function (e) {"
On 2014/12/01 19:11:29, sgurun wrote:
> On 2014/11/27 08:41:30, mnaganov (cr) wrote:
> > Maybe swap "function receiveWorkerMessage" block with this one to match the
> > execution order? Alternatively, you can inline the definition of
> > "receiveWorkerMessage" at the point where you are assigning it to
> > "messageChannelPort.onmessage", as it's a almost a one-liner.
> 
> ok swapped. Isn't it more readable though when the definitions comes earlier
> then referencing a name? and I find it very hard to read when methods are
nested
> inside methods inside double quotes of java variables.

NP. It was just a suggestion to try out.

[sgurun-gerrit only, 2014-12-02 14:52:26]

The CQ bit was checked by sgurun@chromium.org

[commit-bot: I haz the power, 2014-12-02 14:52:48]

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

[commit-bot: I haz the power, 2014-12-02 15:35:12]

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

[commit-bot: I haz the power, 2014-12-02 15:35:13]

Try jobs failed on following builders:
  android_clang_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  win_chromium_x64_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[sgurun-gerrit only, 2014-12-02 18:09:19]

The CQ bit was checked by sgurun@chromium.org

[commit-bot: I haz the power, 2014-12-02 18:10:35]

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

[commit-bot: I haz the power, 2014-12-02 19:49:07]

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

[commit-bot: I haz the power, 2014-12-02 19:49:08]

Try jobs failed on following builders:
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)

[sgurun-gerrit only, 2014-12-02 19:58:57]

The CQ bit was checked by sgurun@chromium.org

[commit-bot: I haz the power, 2014-12-02 19:59:25]

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

[commit-bot: I haz the power, 2014-12-02 20:50:10]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2014-12-02 20:50:53]

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