1321133 - Amazon Assistant page workers aren't loading properly (addon sdk).

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect)

Description

[Mike Kaply [:mkaply]]

Amazon Assistant started having major issues in Firefox 50 related to the loading of pageworkers.

They used mozregression and tracked it down to bug 1285373.

There were two test failures at the same time this bug went in covering the same subject (pageworkers)

https://bugzilla.mozilla.org/show_bug.cgi?id=1288619
https://bugzilla.mozilla.org/show_bug.cgi?id=1288708

but they were just turned off.

I've sent test builds to Amazon and I will know soon if this was 100% caused by bug 1285373.

Comment 1

[Mike Kaply [:mkaply]]

FYI

Comment 2

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Mike has requested we include the fix for this in 50.1.0. Please nominate patches for uplift when ready.

Comment 3

[Mike Kaply [:mkaply]]

Attachment: Bug 1321133. Backout 1285373 because it broke addon-sdk page workers.

Review commit: https://reviewboard.mozilla.org/r/96570/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/96570/

Comment 4

[Mike Kaply [:mkaply]]

Amazon has verified this fixes their problem.

Comment 5

[Kris Maglione [:kmag]]

Comment on attachment 8815763 [details]
Bug 1321133. Backout 1285373 because it broke addon-sdk page workers.

https://reviewboard.mozilla.org/r/96570/#review96942

Please also backout the tests that were added with this commit.

Comment 6

[Mike Kaply [:mkaply]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=adeac8d95105

Comment 7

[Mike Kaply [:mkaply]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2d8e3c16f55b492e77cd0d9147d235141d3f4287
Bug 1321133 - Backout 1285373 for regressions in pageworks. r=kmaglione

Comment 8

[Mike Kaply [:mkaply]]

Comment on attachment 8815763 [details]
Bug 1321133. Backout 1285373 because it broke addon-sdk page workers.

Approval Request Comment
[Feature/Bug causing the regression]: 1285373
[User impact if declined]: A number of SDK add-ons don't work
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: no (hasn't merged yet)
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: Backout of existing patch
[String changes made/needed]: None

Comment 9

[Guilherme Lima]

https://bugzilla.mozilla.org/show_bug.cgi?id=1321133 - patch landed 11 hours ago

Comment 10

[Julien Cristau [:jcristau]]

(In reply to Guilherme Lima from comment #9)
> https://bugzilla.mozilla.org/show_bug.cgi?id=1321133 - patch landed 11 hours
> ago

This is probably the URL you were looking for:
https://hg.mozilla.org/mozilla-central/rev/2d8e3c16f55b492e77cd0d9147d235141d3f4287

Comment 11

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8815763 [details]
Bug 1321133. Backout 1285373 because it broke addon-sdk page workers.

Recent regression that is critical, let's include the fix in 50.1.0

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-release/rev/815978dd9901

Looks like this still needs Aurora/Beta approvals.

Comment 13

[Ben Kelly [:bkelly, not reviewing]]

Mike, can you add a testcase in a new bug that covers this usage so we don't regress it again in the future?  Its still unclear to me what behavior we require here and none of our automated tests caught this problem.

Comment 14

[Mike Kaply [:mkaply]]

Actually two tests caught the failure. One was modified to avoid the problem code and the other is still open. See the description. 

We should change the first test back.

Comment 15

[Ben Kelly [:bkelly, not reviewing]]

Not consistently.  They were intermittent as I understand it.  If we have a case where we want to ensure something isn't GC'd we could write a test that performs a GC explicitly.

Comment 16

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 8815763 [details]
Bug 1321133. Backout 1285373 because it broke addon-sdk page workers.

Approval Request Comment:

See the release request in comment 8 for details.

Comment 17

[Julien Cristau [:jcristau]]

Comment on attachment 8815763 [details]
Bug 1321133. Backout 1285373 because it broke addon-sdk page workers.

This was backed out of nightly and release, let's do the same on beta51 and aurora52

Comment 18

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/85e1c3905c26

Comment 19

[Carsten Book [:Tomcat]]

 https://hg.mozilla.org/releases/mozilla-beta/rev/857a38ebade2a61264d5a7e3c0200e17d7085c8c

Comment 20

[Mike Kaply [:mkaply]]

> Not consistently.  They were intermittent as I understand it.  If we have a case where we want to ensure something isn't GC'd we could write a test that performs a GC explicitly.

I'm not sure who would do that, but I think the consensus at this point is that SDK add-ons are going away soon, so it's probably not worth investing the effort in anything related to them...