Fix flaky TEST PaymentAppBrowserTest.PaymentAppInvocation.

Fix flaky TEST PaymentAppBrowserTest.PaymentAppInvocation.

The test PaymentAppBrowserTest.PaymentAppInvocation was flaky. I was able to
make it fails several times. On the other side, when --browser-side-navigation
is enabled, this test fails very often.

This is due to a race condition:

payment_app.js                 ¦  payment_app_window.html
-------------------------------¦----------------------------------------
client.openWindow              ¦
        |      |_______________¦_______
        |                      ¦       |
 (promise onFullfiled called)  ¦       |
        |                      ¦       |
       ...                     ¦       |
        |                      ¦       |
        v                      ¦       |
  AddEventListener(...)        ¦ postMessage('payment_app_window_ready')

* postMessage('payment_app_window_ready') could be sended/received before
  the event listener is registered on the service worker. In this case
  the 'message' event was not handled. It causes the test to fails.

* A little bit surprising, it could also be send before the promise is
  resolved. That's why I had to use the maybeSendPaymentRequest().

BUG=703602
Review-Url: https://codereview.chromium.org/2785093003
Cr-Commit-Position: refs/heads/master@{#461074}
Committed: https://chromium.googlesource.com/chromium/src/+/007c7242b1ac8aad353e2eac5279071ac59936c0

[arthursonzogni, 2017-03-30 15:11:57]

The CQ bit was checked by arthursonzogni@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-30 15:12:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[arthursonzogni, 2017-03-30 15:15:49]

arthursonzogni@chromium.org changed reviewers:
+ jinho.bang@samsung.com

[arthursonzogni, 2017-03-30 15:15:50]

Hi jinho.

Sorry, I haven't seen the issue before uploading it:
https://crbug.com/703602

I have found the problem.
Please take a look. Thanks!

https://codereview.chromium.org/2785093003/diff/1/content/test/data/payments/...
File content/test/data/payments/payment_app.js (left):

https://codereview.chromium.org/2785093003/diff/1/content/test/data/payments/...
content/test/data/payments/payment_app.js:5: function sendResultToTest(data) {
I removed this function. I don't think it is called.

[arthursonzogni, 2017-03-30 15:25:17]

Description was changed from

==========
Fix flaky TEST PaymentAppBrowserTest.PaymentAppInvocation.

The test PaymentAppBrowserTest.PaymentAppInvocation was flaky. I was able to
make it fails several times. On the other side, when --browser-side-navigation
is enabled, this test fails very often.

This is due to a race condition:

payment_app.js                 ¦  payment_app_window.html
-------------------------------¦----------------------------------------
client.openWindow              ¦
        |      |_______________¦_______
        |                      ¦       |
 (promise onFullfiled called)  ¦       |
        |                      ¦       |
       ...                     ¦       |
        |                      ¦       |
        v                      ¦       |
  AddEventListener(...)        ¦ postMessage('payment_app_window_ready')

* postMessage('payment_app_window_ready') could be send/received before
  the event listener is registered on the service worker. In this case
  the 'message' event was not handled. It causes the test to fails and
  so not handled.

* A little bit surprising, it could also be send before the promise is
  resolved. That's why I had to use the maybeSendPaymentRequest().

BUG=703602
==========

to

==========
Fix flaky TEST PaymentAppBrowserTest.PaymentAppInvocation.

The test PaymentAppBrowserTest.PaymentAppInvocation was flaky. I was able to
make it fails several times. On the other side, when --browser-side-navigation
is enabled, this test fails very often.

This is due to a race condition:

payment_app.js                 ¦  payment_app_window.html
-------------------------------¦----------------------------------------
client.openWindow              ¦
        |      |_______________¦_______
        |                      ¦       |
 (promise onFullfiled called)  ¦       |
        |                      ¦       |
       ...                     ¦       |
        |                      ¦       |
        v                      ¦       |
  AddEventListener(...)        ¦ postMessage('payment_app_window_ready')

* postMessage('payment_app_window_ready') could be sended/received before
  the event listener is registered on the service worker. In this case
  the 'message' event was not handled. It causes the test to fails.

* A little bit surprising, it could also be send before the promise is
  resolved. That's why I had to use the maybeSendPaymentRequest().

BUG=703602
==========

[commit-bot: I haz the power, 2017-03-30 16:35:00]

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

[commit-bot: I haz the power, 2017-03-30 16:35:01]

Dry run: This issue passed the CQ dry run.

[zino, 2017-03-30 16:48:23]

On 2017/03/30 15:15:50, arthursonzogni wrote:
> Hi jinho.
> 
> Sorry, I haven't seen the issue before uploading it:
> https://crbug.com/703602
> 
> I have found the problem.
> Please take a look. Thanks!
> 
>
https://codereview.chromium.org/2785093003/diff/1/content/test/data/payments/...
> File content/test/data/payments/payment_app.js (left):
> 
>
https://codereview.chromium.org/2785093003/diff/1/content/test/data/payments/...
> content/test/data/payments/payment_app.js:5: function sendResultToTest(data) {
> I removed this function. I don't think it is called.

lgtm. Thanks.

BTW, is the issue only reproducible on Linux?
I tried it on Mac as follows but I couldn't see broken..
 content_browsertests --gtest_filter=PaymentApp*
--enable-browser-side-navigation

[arthursonzogni, 2017-03-31 07:52:54]

On 2017/03/30 16:48:23, zino wrote:
> On 2017/03/30 15:15:50, arthursonzogni wrote:
> > Hi jinho.
> > 
> > Sorry, I haven't seen the issue before uploading it:
> > https://crbug.com/703602
> > 
> > I have found the problem.
> > Please take a look. Thanks!
> > 
> >
>
https://codereview.chromium.org/2785093003/diff/1/content/test/data/payments/...
> > File content/test/data/payments/payment_app.js (left):
> > 
> >
>
https://codereview.chromium.org/2785093003/diff/1/content/test/data/payments/...
> > content/test/data/payments/payment_app.js:5: function sendResultToTest(data)
{
> > I removed this function. I don't think it is called.
> 
> lgtm. Thanks.
> 
> BTW, is the issue only reproducible on Linux?
> I tried it on Mac as follows but I couldn't see broken..
>  content_browsertests --gtest_filter=PaymentApp*
> --enable-browser-side-navigation

I don't know, I haven't tried on another OS than Linux...
BTW, on Linux, I was able to make it fails without
--enable-browser-side-navigation too, but it is very rare.

[arthursonzogni, 2017-03-31 07:53:24]

The CQ bit was checked by arthursonzogni@chromium.org

[commit-bot: I haz the power, 2017-03-31 07:53:34]

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-03-31 07:58:18]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1490946804457380, "parent_rev":
"e4218f89ae50b22efd8c98b97f57fa138a4dfe19", "commit_rev":
"007c7242b1ac8aad353e2eac5279071ac59936c0"}

[commit-bot: I haz the power, 2017-03-31 07:59:08]

Description was changed from

==========
Fix flaky TEST PaymentAppBrowserTest.PaymentAppInvocation.

The test PaymentAppBrowserTest.PaymentAppInvocation was flaky. I was able to
make it fails several times. On the other side, when --browser-side-navigation
is enabled, this test fails very often.

This is due to a race condition:

payment_app.js                 ¦  payment_app_window.html
-------------------------------¦----------------------------------------
client.openWindow              ¦
        |      |_______________¦_______
        |                      ¦       |
 (promise onFullfiled called)  ¦       |
        |                      ¦       |
       ...                     ¦       |
        |                      ¦       |
        v                      ¦       |
  AddEventListener(...)        ¦ postMessage('payment_app_window_ready')

* postMessage('payment_app_window_ready') could be sended/received before
  the event listener is registered on the service worker. In this case
  the 'message' event was not handled. It causes the test to fails.

* A little bit surprising, it could also be send before the promise is
  resolved. That's why I had to use the maybeSendPaymentRequest().

BUG=703602
==========

to

==========
Fix flaky TEST PaymentAppBrowserTest.PaymentAppInvocation.

The test PaymentAppBrowserTest.PaymentAppInvocation was flaky. I was able to
make it fails several times. On the other side, when --browser-side-navigation
is enabled, this test fails very often.

This is due to a race condition:

payment_app.js                 ¦  payment_app_window.html
-------------------------------¦----------------------------------------
client.openWindow              ¦
        |      |_______________¦_______
        |                      ¦       |
 (promise onFullfiled called)  ¦       |
        |                      ¦       |
       ...                     ¦       |
        |                      ¦       |
        v                      ¦       |
  AddEventListener(...)        ¦ postMessage('payment_app_window_ready')

* postMessage('payment_app_window_ready') could be sended/received before
  the event listener is registered on the service worker. In this case
  the 'message' event was not handled. It causes the test to fails.

* A little bit surprising, it could also be send before the promise is
  resolved. That's why I had to use the maybeSendPaymentRequest().

BUG=703602

Review-Url: https://codereview.chromium.org/2785093003
Cr-Commit-Position: refs/heads/master@{#461074}
Committed:
https://chromium.googlesource.com/chromium/src/+/007c7242b1ac8aad353e2eac5279...
==========

[commit-bot: I haz the power, 2017-03-31 07:59:09]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/007c7242b1ac8aad353e2eac5279...