Don't bail out when MarionetteEvents actor is registered more than once
Categories
(Remote Protocol :: Marionette, task, P3)
Tracking
(firefox84 fixed)
Tracking | Status | |
---|---|---|
firefox84 | --- | fixed |
People
(Reporter: jdescottes, Assigned: whimboo)
References
Details
(Whiteboard: [marionette-fission-mvp][simple])
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
From https://phabricator.services.mozilla.com/D96117#inline-543242
We are adding a set of helpers registerEventsActor
& unregisterEventsActor
to MarionetteEventsParent.jsm in Bug 1675320.
In theory registerEventsActor
should never be called twice in a row without calling unregisterEventsActor
. If we call it twice, the second call will throw because actors can only be registered once.
We should handle this case properly. Either silently skip the registration if the actor is already registered, or throw a meaningful error. The choice between the two options depends on whether or not we consider that it is valid to call registerEventsActor
twice.
Assignee | ||
Comment 1•4 years ago
|
||
By using a reference counter we can decide when the actor needs to be registered or unregistered.
Assignee | ||
Comment 2•4 years ago
|
||
Whereby I wonder if the ref counting logic would make sense. The actor gets swapped anyway with the next page load, so maybe we should not explicitly unregister again, but just ignore the already registered error?
Assignee | ||
Comment 3•4 years ago
|
||
Actually I think it should be ok to not care about ref counters, at least not for now. Reason is that the actor gets destroyed anyway with the next navigation, and it is small enough to not require too much memory. As such I will update one of my patches on bug 1669169 to just reference this bug. It will make sure to silently ignore a possible re-registration of the actor by filtering out a thrown NotSupportedError
error.
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 4•4 years ago
|
||
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Comment 6•4 years ago
|
||
bugherder |
Updated•2 years ago
|
Description
•