1495301 - Perma Multiple wpt failures on /payment-request when Gecko 64 merges to Beta on 2018-10-15

Status: VERIFIED FIXED

(Core :: DOM: Web Payments, defect, P1)

Description

[Narcis Beleuzu [:NarcisB]]

Central as Beta:
https://treeherder.mozilla.org/#/jobs?repo=try&resultStatus=testfailed,busted,exception,retry,runnable&revision=75e5bf1a70f520ee8ef65fb059ca93a4209b85f3&selectedJob=202468379

Failure log e.g.:
https://treeherder.mozilla.org/logviewer.html#?job_id=202468379&repo=try&lineNumber=3550
https://treeherder.mozilla.org/logviewer.html#?job_id=202468332&repo=try&lineNumber=21475

Eden, this is a regression from https://bugzilla.mozilla.org/show_bug.cgi?id=1481295

Comment 1

[Eden Chuang[:edenchuang]]

It seems PaymentRequest API is not built in beta environment.

Try to figure it out what wrong with it.

Comment 2

[Andreea Pavel [:apavel]]

Thanks for looking into this Eden.

Comment 3

[Intermittent Failures Robot]

1 failures in 2773 pushes (0.0 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:
* try: 1

Platform breakdown:
* linux32: 1

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1495301&startday=2018-09-24&endday=2018-09-30&tree=all

Comment 4

[Eden Chuang[:edenchuang]]

The root cause is here

https://searchfox.org/mozilla-central/rev/819cd31a93fd50b7167979607371878c4d6f18e8/dom/payments/PaymentRequest.cpp#60-68

Matt, Marcos, Should PaymentRequest API be used in beta or just keep it in nightly?

Comment 5

[Marcos Caceres [:marcosc]]

Pretty sure it should be Nightly only at the moment? Right Matt?

Comment 6

[Matthew N. [:MattN]]

Yeah, Nightly-only because people were trying to enable it beta/Nightly and were getting crashes or reporting issues that we didn't care about. Can we have the WPT tests also check NIGHTLY_BUILD?

Comment 7

[Intermittent Failures Robot]

46 failures in 561 pushes (0.082 failures/push) were associated with this bug yesterday.

Repository breakdown:
* try: 46

Platform breakdown:
* macosx64-nightly: 2
* windows7-32: 4
* linux64-nightly: 2
* windows10-64-nightly: 2
* linux32: 4
* osx-10-10: 4
* linux32-nightly: 2
* linux64: 6
* windows7-32-nightly: 2
* windows10-64-devedition: 2
* linux64-qr: 4
* macosx64-devedition: 2
* windows10-64: 4
* windows7-32-devedition: 2
* linux64-devedition: 2
* linux32-devedition: 2

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1495301&startday=2018-10-02&endday=2018-10-02&tree=all

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: Bug 1495301 - Don't run payment-request web-platform-tests on non-Nightly builds. r?edenchuang

PaymentRequest is perma-disabled for non-Nightly builds and ignores the preference dom.payments.request.enabled there, see https://searchfox.org/mozilla-central/rev/819cd31a93fd50b7167979607371878c4d6f18e8/dom/payments/PaymentRequest.cpp#60-68

Comment 9

[Intermittent Failures Robot]

18 failures in 465 pushes (0.039 failures/push) were associated with this bug yesterday.

Repository breakdown:
* try: 18

Platform breakdown:
* linux32-devedition: 1
* linux64-devedition: 1
* linux64-nightly: 1
* osx-10-10: 2
* linux64-qr: 2
* linux32: 2
* macosx64-nightly: 1
* windows7-32-nightly: 1
* windows10-64-devedition: 1
* linux64: 3
* linux32-nightly: 1
* windows10-64: 2

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1495301&startday=2018-10-03&endday=2018-10-03&tree=all

Comment 10

[Eden Chuang[:edenchuang]]

Sebastian, Thanks for the patch. It really helps.

I am going to submit a patch base on the patch and add "disabled" flag for the tests under /feature-policy.

Comment 11

[Eden Chuang[:edenchuang]]

Attachment: Disable PaymentRequest web-platform tests on non-nightly build.

We only expose the Payment-Request on nightly, so disable the web-platform tests on non-nightly build.

Comment 12

[Andrea Marchesini [:baku]]

I did the opposite in bug 1496114. Any particular reason why should we enable Payment just in nightly? If the pref is on, Payment API should be enabled everywhere as any other under-development API.

Comment 13

[Eden Chuang[:edenchuang]]

(In reply to Andrea Marchesini [:baku] from comment #12)
> I did the opposite in bug 1496114. Any particular reason why should we
> enable Payment just in nightly? If the pref is on, Payment API should be
> enabled everywhere as any other under-development API.

Did not notice the fix on bug 1496114.
I wrote the patch base on the comment 5 and comment 6.
I am totally fine with removing the nightly build only restriction.
Maybe front-end team would like to do more user testing in nightly for stability and reliability.

Matt, Marcos, could you describe the concerns for turn on PaymentRequest API on Beta by perf dom.payments.request.enabled?
If you think the pref guard is enough for Beta. I will set this bug as dup of bug 1496114.

Comment 14

[Andrea Marchesini [:baku]]

> Matt, Marcos, could you describe the concerns for turn on PaymentRequest API
> on Beta by perf dom.payments.request.enabled?

Note that we are not enabling payment API in beta. We just enable it for testing when running WPTs.

Comment 15

[Matthew N. [:MattN]]

A) Personally I think we shouldn't even *build* the feature on beta since it's a waste of memory if it's not ready for use. That's what we do with the front-end.
B) As I said in comment 6, users were reporting crashes that we didn't care about so that's why we have the feature check the channel. Since we don't package the front-end outside Nightly the crashes will continue to be a problem unless we change that but changing that is just a distraction from more important things IMO. Triaging the beta/release bug reports will also be wasting more of our time.

Comment 16

[Andrea Marchesini [:baku]]

> A) Personally I think we shouldn't even *build* the feature on beta since
> it's a waste of memory if it's not ready for use. That's what we do with the
> front-end.

The current code is fully compiled and part of firefox. Just the PaymentRequest CTOR is not exposed to content if not nightly. Same result of checking the pref, same memory usage.

In platform we build all the features also if disabled by pref. There are several examples of APIs under-developmented disabled by prefs in beta right now. Those APIs are usually written in C++/rust and there is not waste of memory because they are not exposed to content. If the user doesn't touch about:config, of course...

> B) As I said in comment 6, users were reporting crashes that we didn't care
> about so that's why we have the feature check the channel. Since we don't
> package the front-end outside Nightly the crashes will continue to be a
> problem unless we change that but changing that is just a distraction from
> more important things IMO. Triaging the beta/release bug reports will also
> be wasting more of our time.

Yes. I see the problem. touching about:config can trigger several issues.
I'm fine of the  Eden's patch or what I pushed to m-i this morning.

But, is it a real problem the number of crash reports for users in beta who enabled the pref?

Comment 17

[Matthew N. [:MattN]]

(In reply to Andrea Marchesini [:baku] from comment #16)
> > A) Personally I think we shouldn't even *build* the feature on beta since
> > it's a waste of memory if it's not ready for use. That's what we do with the
> > front-end.
> 
> The current code is fully compiled and part of firefox. Just the
> PaymentRequest CTOR is not exposed to content if not nightly. Same result of
> checking the pref, same memory usage.

I know that and it seems wrong, especially for mobile.

> In platform we build all the features also if disabled by pref. There are
> several examples of APIs under-developmented disabled by prefs in beta right
> now. Those APIs are usually written in C++/rust and there is not waste of
> memory because they are not exposed to content. If the user doesn't touch
> about:config, of course...

Doesn't it still take memory from the binary size?

> > B) As I said in comment 6, users were reporting crashes that we didn't care
> > about so that's why we have the feature check the channel. Since we don't
> > package the front-end outside Nightly the crashes will continue to be a
> > problem unless we change that but changing that is just a distraction from
> > more important things IMO. Triaging the beta/release bug reports will also
> > be wasting more of our time.
> 
> Yes. I see the problem. touching about:config can trigger several issues.
> I'm fine of the  Eden's patch or what I pushed to m-i this morning.
> 
> But, is it a real problem the number of crash reports for users in beta who
> enabled the pref?

Yeah, see the bug that added the check. There were at least 3 bugs filed and we got nagged by relman to fix the crashes.

Comment 18

[Matthew N. [:MattN]]

We also got crash complaints on IRC from random people btw.

Comment 19

[Marcos Caceres [:marcosc]]

I don't have a strong opinion, but feel we should follow normal practice: At least on the DOM side, we generally ship APIs in the way Baku mentioned. I'm sympathetic to what MattN is saying about getting bogus reports for things we know are still very much "alpha" quality.

Comment 20

[Intermittent Failures Robot]

64 failures in 2708 pushes (0.024 failures/push) were associated with this bug in the last 7 days.

This is the #28 most frequent failure this week.

** This failure happened more than 30 times this week! Resolving this bug is a high priority. **

** Try to resolve this bug as soon as possible. If unresolved for 2 weeks, the affected test(s) may be disabled. ** 

Repository breakdown:
* try: 64

Platform breakdown:
* linux64-qr: 6
* windows10-64: 6
* linux32: 6
* linux64-nightly: 3
* windows7-32-nightly: 3
* macosx64-nightly: 3
* osx-10-10: 6
* macosx64-devedition: 2
* windows10-64-nightly: 2
* linux64-devedition: 3
* windows10-64-devedition: 3
* windows7-32: 4
* windows7-32-devedition: 2
* linux32-nightly: 3
* linux32-devedition: 3
* linux64: 9

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1495301&startday=2018-10-01&endday=2018-10-07&tree=all

Comment 21

[Eden Chuang[:edenchuang]]

IMO, For web-platform tests, we should not treat PaymentRequest API different in Beta and Nightly. But we might not want it can be tested for user testing in Beta. 

So I would like to dup this bug as bug 1496114, But adding more enabling control in Beta for reducing the bug report we don't care now.

Comment 23

[Eden Chuang[:edenchuang]]

Attachment: Disable PaymentRequest web-platform tests on non-nightly build.

Comment 24

[Eden Chuang[:edenchuang]]

try result on nightly 
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a0b12082c378b45025ea1c78ca3494e544a318c6

try result on Beta
https://treeherder.mozilla.org/#/jobs?repo=try&revision=92e348aab59e5ed2a0ffc11e87cd7d5ebe2a67e9

Comment 25

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8ce5fe12ae8d
Disbale payment-request web-platform tests on non-nightly build. r=a=test-only,revert

Comment 26

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/8ce5fe12ae8d

Comment 27

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Verified fixed with yesterday's beta simulation: https://treeherder.mozilla.org/#/jobs?repo=try&resultStatus=testfailed,busted,exception,retry,usercancel,runnable&revision=3157d00d6962b95cdc5d13199556e35dd9e6e641