Closed
Bug 1353559
Opened 7 years ago
Closed 7 years ago
Exited browser actors might be asked for their form
Categories
(DevTools :: General, enhancement, P3)
DevTools
General
Tracking
(firefox55 fixed)
RESOLVED
FIXED
Firefox 55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: jryans, Assigned: jryans)
Details
Attachments
(2 files)
Exited browser actors can still be asked for their form, presumably if someone is actively list tabs during the time they exit. We should update the `form` code path to guard against `this._browser` being null.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 2•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=cc1fa583488771cdc6e80005b33db0801a1cc2f9
Comment 3•7 years ago
|
||
mozreview-review |
Comment on attachment 8854630 [details] Bug 1353559 - Guard against null browser in exited actor. https://reviewboard.mozilla.org/r/126574/#review129336 Did you had exception in tests or in the product? It looks like we should stop using actors as soon as they are exited, so I suspect there is some more fixes to do... Especially if that happens in the product!
Attachment #8854630 -
Flags: review?(poirot.alex) → review+
Comment hidden (mozreview-request) |
Assignee | ||
Comment 5•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8854630 [details] Bug 1353559 - Guard against null browser in exited actor. https://reviewboard.mozilla.org/r/126574/#review129336 I noticed this while running some net monitor tests. I haven't seen it in product yet. The problem happens during onListTabs. Here is the sequence I saw: 1. The browser's actor is alive 2. Someone calls listTabs 3. `onListTabs` asks for the list of browser actors, which is async with various promises, etc. 4. The browser's actor exits 5. `onListTabs` gets the list back and starts iterating, and the list contains the exited actor
Comment 6•7 years ago
|
||
mozreview-review |
Comment on attachment 8854842 [details] Bug 1353559 - Filter out exited browsers from tab list. https://reviewboard.mozilla.org/r/126798/#review129416 Thanks a lot for this another cleanup, I think this is the kind of tweaks very valuable for our tests! I'm wondering again if there is still something to fix in the netmonitor test you were running to make it wait correctly for listTabs before proceeding? It sounds like this listTabs request could very likely finish after the test ends and overlap the next one?
Attachment #8854842 -
Flags: review?(poirot.alex) → review+
Pushed by jryans@gmail.com: https://hg.mozilla.org/integration/autoland/rev/f74c1423f855 Guard against null browser in exited actor. r=ochameau https://hg.mozilla.org/integration/autoland/rev/29a30ad34f35 Filter out exited browsers from tab list. r=ochameau
Assignee | ||
Comment 8•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8854842 [details] Bug 1353559 - Filter out exited browsers from tab list. https://reviewboard.mozilla.org/r/126798/#review129416 That's a good point, I'll check into this!
Comment 9•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/f74c1423f855 https://hg.mozilla.org/mozilla-central/rev/29a30ad34f35
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Assignee | ||
Comment 10•7 years ago
|
||
(In reply to Alexandre Poirot [:ochameau] from comment #6) > Comment on attachment 8854842 [details] > Bug 1353559 - Filter out exited browsers from tab list. > > https://reviewboard.mozilla.org/r/126798/#review129416 > > Thanks a lot for this another cleanup, I think this is the kind of tweaks > very valuable for our tests! > > I'm wondering again if there is still something to fix in the netmonitor > test you were running to make it wait correctly for listTabs before > proceeding? It sounds like this listTabs request could very likely finish > after the test ends and overlap the next one? Looks like the issue was a performance front getter being triggered on toolbox close. I'll address it in bug 1353897.
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•