Print proper debug message in case of change event timeout.

Print proper debug message in case of change event timeout.

"AbortError: Timed out as the page didn't resolve the promise
from change event"

BUG=629462
Committed: https://crrev.com/f1b534bc924aa095cc581b4524c4890629b9c633
Cr-Commit-Position: refs/heads/master@{#408986}

[pals, 2016-07-29 10:08:57]

Description was changed from

==========
Print proper debug message in case of change event timeout.

BUG=629462
==========

to

==========
Print proper debug message in case of change event timeout.
 
"AbortError: Timed out as the page didn't resolve the promise
from change event"

BUG=629462
==========

[pals, 2016-07-29 10:08:57]

sanjoy.pal@samsung.com changed reviewers:
+ rouslan@chromium.org

[pals, 2016-07-29 12:15:46]

PTAL.

[please use gerrit instead, 2016-07-29 15:40:51]

Excellent work! There seems to be no need for a failure callback that takes a
ScriptValue, because it's always converted to string. So let's change
onUpdatePaymentDetailsFailure() to have a String parameter instead of a
ScriptValue parameter. Then we don't need an additional abortPaymentRequest()
function.

https://codereview.chromium.org/2193183002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/payments/PaymentRequest.cpp (right):

https://codereview.chromium.org/2193183002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:459: void
PaymentRequest::onUpdatePaymentDetailsFailure(const ScriptValue& error)
Change ScriptValue to String

https://codereview.chromium.org/2193183002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:468: {
Remove lines 461-468.

https://codereview.chromium.org/2193183002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/payments/PaymentRequest.h (right):

https://codereview.chromium.org/2193183002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/payments/PaymentRequest.h:71: void
onUpdatePaymentDetailsFailure(const ScriptValue& error) override;
Let's change this ScriptValue to String

https://codereview.chromium.org/2193183002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/payments/PaymentRequest.h:72: void
abortPaymentRequest(const String& message) override;
Let's remove this method

https://codereview.chromium.org/2193183002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/payments/PaymentRequestUpdateEvent.cpp
(right):

https://codereview.chromium.org/2193183002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/payments/PaymentRequestUpdateEvent.cpp:74:
m_updater->onUpdatePaymentDetailsFailure(value);
String message;
value.toString(message);
m_updater->onUpdatePaymentDetailsFailure(message);

https://codereview.chromium.org/2193183002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/payments/PaymentRequestUpdateEvent.cpp:132:
m_updater->abortPaymentRequest("Timed out as the page didn't resolve the promise
from change event");
Call onUpdatePaymentDetailsFailure("Timed out...")

https://codereview.chromium.org/2193183002/diff/1/third_party/WebKit/Source/m...
File
third_party/WebKit/Source/modules/payments/PaymentRequestUpdateEventTest.cpp
(right):

https://codereview.chromium.org/2193183002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/payments/PaymentRequestUpdateEventTest.cpp:31:
MOCK_METHOD1(onUpdatePaymentDetailsFailure, void(const ScriptValue& error));
Change ScriptValue to String

https://codereview.chromium.org/2193183002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/payments/PaymentRequestUpdateEventTest.cpp:32:
MOCK_METHOD1(abortPaymentRequest, void(const String& message));
No need for the extra method.

https://codereview.chromium.org/2193183002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/payments/PaymentRequestUpdateEventTest.cpp:123:
EXPECT_EQ("AbortError: Timed out as the page didn't resolve the promise from
change event", errorMessage);
Great test!

https://codereview.chromium.org/2193183002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/payments/PaymentUpdater.h (right):

https://codereview.chromium.org/2193183002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/payments/PaymentUpdater.h:9: #include
"platform/heap/GarbageCollected.h"
#include "wtf/text/WTFString.h"

https://codereview.chromium.org/2193183002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/payments/PaymentUpdater.h:18: virtual void
onUpdatePaymentDetailsFailure(const ScriptValue& error) = 0;
Change ScriptValue to String

https://codereview.chromium.org/2193183002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/payments/PaymentUpdater.h:19: virtual void
abortPaymentRequest(const String& error) = 0;
No need for extra function.

[pals, 2016-08-01 08:44:11]

The CQ bit was checked by sanjoy.pal@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-01 08:44:28]

Dry run: 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, 2016-08-01 09:58:23]

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

[commit-bot: I haz the power, 2016-08-01 09:58:24]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[pals, 2016-08-01 11:12:42]

The CQ bit was checked by sanjoy.pal@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-01 11:12:57]

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

[pals, 2016-08-01 11:18:23]

Modified the patch as suggested. PTAL.

https://codereview.chromium.org/2193183002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/payments/PaymentRequest.cpp (right):

https://codereview.chromium.org/2193183002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:459: void
PaymentRequest::onUpdatePaymentDetailsFailure(const ScriptValue& error)
On 2016/07/29 15:40:50, rouslan wrote:
> Change ScriptValue to String

Done.

https://codereview.chromium.org/2193183002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/payments/PaymentRequest.cpp:468: {
On 2016/07/29 15:40:50, rouslan wrote:
> Remove lines 461-468.

Done.

https://codereview.chromium.org/2193183002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/payments/PaymentRequest.h (right):

https://codereview.chromium.org/2193183002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/payments/PaymentRequest.h:71: void
onUpdatePaymentDetailsFailure(const ScriptValue& error) override;
On 2016/07/29 15:40:50, rouslan wrote:
> Let's change this ScriptValue to String

Done.

https://codereview.chromium.org/2193183002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/payments/PaymentRequest.h:72: void
abortPaymentRequest(const String& message) override;
On 2016/07/29 15:40:50, rouslan wrote:
> Let's remove this method

Done.

https://codereview.chromium.org/2193183002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/payments/PaymentRequestUpdateEvent.cpp
(right):

https://codereview.chromium.org/2193183002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/payments/PaymentRequestUpdateEvent.cpp:74:
m_updater->onUpdatePaymentDetailsFailure(value);
On 2016/07/29 15:40:50, rouslan wrote:
> String message;
> value.toString(message);
> m_updater->onUpdatePaymentDetailsFailure(message);

It was crashing if we do exactly like above. Doing the same thing in a different
way. PTAL.

https://codereview.chromium.org/2193183002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/payments/PaymentRequestUpdateEvent.cpp:132:
m_updater->abortPaymentRequest("Timed out as the page didn't resolve the promise
from change event");
On 2016/07/29 15:40:50, rouslan wrote:
> Call onUpdatePaymentDetailsFailure("Timed out...")

Done.

https://codereview.chromium.org/2193183002/diff/1/third_party/WebKit/Source/m...
File
third_party/WebKit/Source/modules/payments/PaymentRequestUpdateEventTest.cpp
(right):

https://codereview.chromium.org/2193183002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/payments/PaymentRequestUpdateEventTest.cpp:31:
MOCK_METHOD1(onUpdatePaymentDetailsFailure, void(const ScriptValue& error));
On 2016/07/29 15:40:50, rouslan wrote:
> Change ScriptValue to String

Done.

https://codereview.chromium.org/2193183002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/payments/PaymentRequestUpdateEventTest.cpp:32:
MOCK_METHOD1(abortPaymentRequest, void(const String& message));
On 2016/07/29 15:40:50, rouslan wrote:
> No need for the extra method.

Done.

https://codereview.chromium.org/2193183002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/payments/PaymentRequestUpdateEventTest.cpp:123:
EXPECT_EQ("AbortError: Timed out as the page didn't resolve the promise from
change event", errorMessage);
On 2016/07/29 15:40:50, rouslan wrote:
> Great test!

Thanks.

https://codereview.chromium.org/2193183002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/payments/PaymentUpdater.h (right):

https://codereview.chromium.org/2193183002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/payments/PaymentUpdater.h:9: #include
"platform/heap/GarbageCollected.h"
On 2016/07/29 15:40:50, rouslan wrote:
> #include "wtf/text/WTFString.h"

Done.

https://codereview.chromium.org/2193183002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/payments/PaymentUpdater.h:18: virtual void
onUpdatePaymentDetailsFailure(const ScriptValue& error) = 0;
On 2016/07/29 15:40:51, rouslan wrote:
> Change ScriptValue to String

Done.

https://codereview.chromium.org/2193183002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/payments/PaymentUpdater.h:19: virtual void
abortPaymentRequest(const String& error) = 0;
On 2016/07/29 15:40:50, rouslan wrote:
> No need for extra function.

Done.

[commit-bot: I haz the power, 2016-08-01 12:16:36]

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

[commit-bot: I haz the power, 2016-08-01 12:16:37]

Dry run: This issue passed the CQ dry run.

[please use gerrit instead, 2016-08-01 16:57:44]

The CQ bit was checked by rouslan@chromium.org

[please use gerrit instead, 2016-08-01 16:57:45]

lgtm

https://codereview.chromium.org/2193183002/diff/1/third_party/WebKit/Source/m...
File third_party/WebKit/Source/modules/payments/PaymentRequestUpdateEvent.cpp
(right):

https://codereview.chromium.org/2193183002/diff/1/third_party/WebKit/Source/m...
third_party/WebKit/Source/modules/payments/PaymentRequestUpdateEvent.cpp:74:
m_updater->onUpdatePaymentDetailsFailure(value);
On 2016/08/01 11:18:23, pals wrote:
> On 2016/07/29 15:40:50, rouslan wrote:
> > String message;
> > value.toString(message);
> > m_updater->onUpdatePaymentDetailsFailure(message);
> 
> It was crashing if we do exactly like above. Doing the same thing in a
different
> way. PTAL.

Acknowledged.

[commit-bot: I haz the power, 2016-08-01 16:58:07]

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, 2016-08-01 17:03:20]

Description was changed from

==========
Print proper debug message in case of change event timeout.
 
"AbortError: Timed out as the page didn't resolve the promise
from change event"

BUG=629462
==========

to

==========
Print proper debug message in case of change event timeout.
 
"AbortError: Timed out as the page didn't resolve the promise
from change event"

BUG=629462
==========

[commit-bot: I haz the power, 2016-08-01 17:03:23]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-01 17:06:52]

Description was changed from

==========
Print proper debug message in case of change event timeout.
 
"AbortError: Timed out as the page didn't resolve the promise
from change event"

BUG=629462
==========

to

==========
Print proper debug message in case of change event timeout.

"AbortError: Timed out as the page didn't resolve the promise
from change event"

BUG=629462

Committed: https://crrev.com/f1b534bc924aa095cc581b4524c4890629b9c633
Cr-Commit-Position: refs/heads/master@{#408986}
==========

[commit-bot: I haz the power, 2016-08-01 17:06:53]

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