If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Exited browser actors might be asked for their form

RESOLVED FIXED in Firefox 55

Status

()

Firefox
Developer Tools
P3
normal
RESOLVED FIXED
7 months ago
7 months ago

People

(Reporter: jryans, Assigned: jryans)

Tracking

Trunk
Firefox 55
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Assignee)

Description

7 months ago
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 months ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=cc1fa583488771cdc6e80005b33db0801a1cc2f9

Comment 3

7 months 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 months 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 months 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+

Comment 7

7 months ago
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 months 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 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f74c1423f855
https://hg.mozilla.org/mozilla-central/rev/29a30ad34f35
Status: ASSIGNED → RESOLVED
Last Resolved: 7 months ago
status-firefox55: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
(Assignee)

Comment 10

7 months 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.
You need to log in before you can comment on or make changes to this bug.