Closed Bug 1094312 Opened 5 years ago Closed 5 years ago

Intermittent browser_bug553455.js | Should have seen the progress notification - Got addon-install-failed-notification, expected addon-progress-notification

Categories

(Toolkit :: Add-ons Manager, defect)

defect
Not set
Points:
2

Tracking

()

RESOLVED FIXED
mozilla37
Iteration:
37.3 - 12 Jan
Tracking Status
firefox35 --- unaffected
firefox36 --- fixed
firefox37 --- fixed
firefox-esr31 --- unaffected

People

(Reporter: mossop, Assigned: mossop)

References

Details

(Keywords: intermittent-failure)

Attachments

(3 files, 1 obsolete file)

And disabling this test made browser_bug585558.js permafail:

Disabled in https://hg.mozilla.org/mozilla-central/rev/2114ef80f6ae
Status: NEW → ASSIGNED
Iteration: --- → 36.2
Points: --- → 2
Flags: qe-verify-
Flags: firefox-backlog+
Keywords: leave-open
Iteration: 36.2 → 36.3
Iteration: 36.3 → 37.1
Iteration: 37.1 → 37.2
Duplicate of this bug: 1094743
Iteration: 37.2 → 37.3
Depends on: 1116629
Attached file MozReview Request: bz://1094312/Mossop (obsolete) —
Attachment #8542828 - Flags: review?(mconley)
Attachment #8542828 - Flags: review?(gijskruitbosch+bugs)
/r/1837 - Bug 1094312: Properly destroy browsers when switching between remote and non-remove pages and override the default destroy method in remote-browser.xml.
/r/1839 - Bug 1094312: Fix browser_bug553455.js to handle the cases where the progress notification is hidden before it has fully appeared.
/r/1841 - Bug 1094312: Fix browser_bug553455.js:test_cancel_restart by pausing the download for long enough for the progress notification to show reliably.

Pull down these commits:

hg pull review -r b52013eacda71455eb733b68a99b70af5e634d62
Note that the patches here (in particular the last commit) depend on bug 1116629. Mconley to review the first commit and Gijs the rest.
https://reviewboard.mozilla.org/r/1839/#review1271

::: browser/base/content/test/general/browser.ini
(Diff revision 1)
> -#skip-if = buildapp == 'mulet' || e10s # Bug 1066070 - I don't think either popup notifications nor addon install stuff works on mulet? ; for e10s, indefinite waiting halfway through the test, tracked in bug 1093586

Please make sure to update bug 1093586 appropriately.

::: browser/base/content/test/general/browser_bug553455.js
(Diff revision 1)
> -             "Should have seen the install blocked - 2nd time");

It's kind of a shame that now the test failure messages won't be unique, and we'll have to dig through logs to figure out what's going on if it fails, but I don't see a good way to fix that.
https://reviewboard.mozilla.org/r/1841/#review1273

with the below addressed:

::: toolkit/mozapps/extensions/test/xpinstall/slowinstall.sjs
(Diff revision 1)
> +function parseQueryString(aQueryString) {

Can't you use URLSearchParams instead? (not sure if it is available for sjs files...)

::: toolkit/mozapps/extensions/test/xpinstall/slowinstall.sjs
(Diff revision 1)
> +        // nsIOutputStream so here we are.

Did you file a bug? :-)

This is test code so I don't care about perf so much, but it'd be nice if there were a good way to do this in prod code, too.
Oh, with the ship it conditional on the traditional green try push. ;-)
Attachment #8542828 - Flags: review?(gijskruitbosch+bugs) → review+
Blocks: 1093586
https://reviewboard.mozilla.org/r/1837/#review1381

::: toolkit/content/widgets/remote-browser.xml
(Diff revision 1)
> +      <!-- This is necessary because the destructor doesn't always get called when
> +           we are removed from a tabbrowser. This will be explicitly called by tabbrowser -->

Huh. That's a surprising result. Also seems to be an old one looking at browser.xml. :/
Attachment #8542828 - Flags: review?(mconley) → review+
(In reply to :Gijs Kruitbosch from comment #9)
> https://reviewboard.mozilla.org/r/1841/#review1273
> 
> with the below addressed:
> 
> ::: toolkit/mozapps/extensions/test/xpinstall/slowinstall.sjs
> (Diff revision 1)
> > +function parseQueryString(aQueryString) {
> 
> Can't you use URLSearchParams instead? (not sure if it is available for sjs
> files...)

Not available in sjs unfortunately.

> ::: toolkit/mozapps/extensions/test/xpinstall/slowinstall.sjs
> (Diff revision 1)
> > +        // nsIOutputStream so here we are.
> 
> Did you file a bug? :-)
> 
> This is test code so I don't care about perf so much, but it'd be nice if
> there were a good way to do this in prod code, too.

Filed bug 1119464
Did you want to land this on Aurora36 too?
Flags: needinfo?(dtownsend)
Might as well make that a Beta approval request at this point since it's highly unlikely to get nominated, approved, and landed on Aurora in time for the uplift today.
Comment on attachment 8542828 [details]
MozReview Request: bz://1094312/Mossop

Approval Request Comment
[Feature/regressing bug #]: Bug 1082764 and bug 1093586
[User impact if declined]: Lost test coverage around add-on installation
[Describe test coverage new/current, TBPL]: Test running on m-c for 
[Risks and why]: Very low risk, The bulk of this is test code changes. The product code that is changed is all e10s only code so should never be used on beta.
[String/UUID change made/needed]: None
Flags: needinfo?(dtownsend)
Attachment #8542828 - Flags: approval-mozilla-beta?
Attachment #8542828 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #8542828 - Attachment is obsolete: true
Attachment #8618554 - Flags: review+
Attachment #8618555 - Flags: review+
Attachment #8618556 - Flags: review+
You need to log in before you can comment on or make changes to this bug.