Closed Bug 1299798 Opened 6 years ago Closed 6 years ago

checkForPendingCrashReports() raises "TypeError: win is null"

Categories

(Firefox :: General, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 51
Tracking Status
firefox49 --- wontfix
firefox50 --- fixed
firefox51 --- fixed

People

(Reporter: whimboo, Assigned: blassey)

References

Details

Attachments

(1 file)

With bug 1269998 the method checkForPendingCrashReports() got added. As we have seen in our firefox-ui-update tests it can fail if no browser window is open. It means in case of applying updates it can happen that the browser window is closed first, followed by the about window. I believe that this is the situation here.

The following assertion is thrown:

05:54:25     INFO -  *************************
05:54:25     INFO -  A coding exception was thrown in a Promise resolution callback.
05:54:25     INFO -  See https://developer.mozilla.org/Mozilla/JavaScript_code_modules/Promise.jsm/Promise
05:54:25     INFO -  Full message: TypeError: win is null
05:54:25     INFO -  Full stack: onSuccess@resource://app/components/nsBrowserGlue.js:758:17
05:54:25     INFO -  Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:937:23
05:54:25     INFO -  this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:816:7
05:54:25     INFO -  Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:747:11
05:54:25     INFO -  this.PromiseWalker.schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:779:7
05:54:25     INFO -  this.PromiseWalker.completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:714:7
05:54:25     INFO -  get _worker/worker.onmessage@resource://gre/modules/PromiseWorker.jsm:231:9
05:54:25     INFO -  Spinner.prototype.observe@resource://gre/modules/AsyncShutdown.jsm:551:9
05:54:26     INFO -  *************************

As it can be seen by the following code we blindly assume that there is at least one browser window open:

https://dxr.mozilla.org/mozilla-central/rev/b7f7ae14590aced450bb0b0469dfb38edd2c0ace/browser/components/nsBrowserGlue.js#758

I would say we should early return if we cannot get a reference to the global notification box, and should try again in showing the notification when Firefox got updated. Brad, can you please check that? Thanks.
Flags: needinfo?(blassey.bugs)
Priority: P1 → --
Wait. This exception actually occurs when Firefox has been restarted, and the old software update dialog is shown because the downloaded partial update cannot be applied due to a forced failure by our tests.
Attached patch test_win.patchSplinter Review
Assignee: nobody → blassey.bugs
Flags: needinfo?(blassey.bugs)
Attachment #8787488 - Flags: review?(mconley)
Comment on attachment 8787488 [details] [diff] [review]
test_win.patch

Review of attachment 8787488 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!
Attachment #8787488 - Flags: review?(mconley) → review+
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/fx-team/rev/bcab45259b4f
Add an early return if we cannot get a reference to the global notification box. r=mconley
Keywords: checkin-needed
Depends on: 1300377
https://hg.mozilla.org/mozilla-central/rev/bcab45259b4f
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51
No longer depends on: 1300377
Brad, would you mind requesting uplift to at least aurora? I believe it's too late for beta given our merge today.
Flags: needinfo?(blassey.bugs)
Comment on attachment 8787488 [details] [diff] [review]
test_win.patch

(In reply to Henrik Skupin (:whimboo) from comment #6)
> Brad, would you mind requesting uplift to at least aurora? I believe it's
> too late for beta given our merge today.

fwiw, you can also request uplift

Approval Request Comment
[Feature/regressing bug #]:
[User impact if declined]: none known, exceptions seen in automation
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: low risk, simple null check and early return. We'll take care of the lingering report on the next browser start.
[String/UUID change made/needed]:
Flags: needinfo?(blassey.bugs)
Attachment #8787488 - Flags: approval-mozilla-aurora?
Comment on attachment 8787488 [details] [diff] [review]
test_win.patch

Has been in Nightly for a few days, Aurora50+.
Attachment #8787488 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.