Closed Bug 1394767 Opened 2 years ago Closed 2 years ago

restoreTab shouldn't use NS_ASSERT

Categories

(Firefox :: Session Restore, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 57
Tracking Status
firefox57 --- fixed

People

(Reporter: dao, Assigned: dao)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

from bug 1388628 comment 10:

> Okay, I can reproduce with a file:// homepage in nightly.
> 
> This seems relevant:
> 
> A coding exception was thrown in a Promise resolution callback.
> See
> https://developer.mozilla.org/Mozilla/JavaScript_code_modules/Promise.jsm/
> Promise
> 
> Full message: TypeError: access to strict mode caller function is censored
> Full stack: NS_ASSERT@resource://gre/modules/debug.js:50:7
> restoreTab@resource:///modules/sessionstore/SessionStore.jsm:3552:5
> restoreTabs@resource:///modules/sessionstore/SessionStore.jsm:3539:7
> ssi_restoreWindow@resource:///modules/sessionstore/SessionStore.jsm:3404:7
> ssi_restoreWindows@resource:///modules/sessionstore/SessionStore.jsm:3484:5
> initializeWindow@resource:///modules/sessionstore/SessionStore.jsm:1158:11
> onBeforeBrowserWindowShown/<@resource:///modules/sessionstore/SessionStore.
> jsm:1307:9
> process@resource://gre/modules/Promise-backend.js:922:23
> walkerLoop@resource://gre/modules/Promise-backend.js:806:7
> scheduleWalkerLoop/<@resource://gre/modules/Promise-backend.js:742:11

restoreTab should not break the whole session restore process when this happens. It should instead log an error and return, or something like that.
Assignee: nobody → dao+bmo
Status: NEW → ASSIGNED
Comment on attachment 8905460 [details]
Bug 1394767 - Log an error and return in unexpected situations rather than calling NS_ASSERT and letting the subsequent code fail.

https://reviewboard.mozilla.org/r/177274/#review182286

Thanks! Can you update the strings passed into `Cu.reportError()` to be proper sentences, starting with capitals and ending with a dot?
Attachment #8905460 - Flags: review?(mdeboer) → review+
(In reply to Dão Gottwald [::dao] from comment #1)
> Returning in onBrowserCrashed if the browser isn't remote breaks various
> tests, so I simply removed this seemingly bogus check.

Try run with the check and return:

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

Without that check:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9d9f1cfea23b0f9aac024fff68dfa8ee6b3f4dd9
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b4b9eaba235e
Log an error and return in unexpected situations rather than calling NS_ASSERT and letting the subsequent code fail. r=mikedeboer
https://hg.mozilla.org/mozilla-central/rev/b4b9eaba235e
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
You need to log in before you can comment on or make changes to this bug.