Closed Bug 1321133 Opened 4 years ago Closed 4 years ago

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

Categories

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

defect
Not set
major

Tracking

(firefox50+ fixed, firefox51+ fixed, firefox52+ fixed, firefox53+ fixed)

RESOLVED FIXED
mozilla53
Tracking Status
firefox50 + fixed
firefox51 + fixed
firefox52 + fixed
firefox53 + fixed

People

(Reporter: mkaply, Assigned: mkaply)

References

Details

(Keywords: regression)

Attachments

(1 file)

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.
FYI
Flags: needinfo?(rkothari)
Flags: needinfo?(lhenry)
Mike has requested we include the fix for this in 50.1.0. Please nominate patches for uplift when ready.
Flags: needinfo?(rkothari)
Amazon has verified this fixes their problem.
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.
Attachment #8815763 - Flags: review+
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
Attachment #8815763 - Flags: approval-mozilla-release?
https://bugzilla.mozilla.org/show_bug.cgi?id=1321133 - patch landed 11 hours ago
Assignee: nobody → mozilla
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
(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 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
Attachment #8815763 - Flags: approval-mozilla-release? → approval-mozilla-release+
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.
Flags: needinfo?(mozilla)
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.
Flags: needinfo?(mozilla)
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.
Flags: needinfo?(mozilla)
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.
Attachment #8815763 - Flags: approval-mozilla-beta?
Attachment #8815763 - Flags: approval-mozilla-aurora?
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
Attachment #8815763 - Flags: approval-mozilla-beta?
Attachment #8815763 - Flags: approval-mozilla-beta+
Attachment #8815763 - Flags: approval-mozilla-aurora?
Attachment #8815763 - Flags: approval-mozilla-aurora+
> 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...
Flags: needinfo?(mozilla)
You need to log in before you can comment on or make changes to this bug.