The default bug view has changed. See this FAQ.

[FlyWeb] Published server is not shut down when tab is closed

RESOLVED FIXED

Status

()

Core
Networking
RESOLVED FIXED
11 months ago
11 months ago

People

(Reporter: djvj, Unassigned)

Tracking

(Blocks: 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

11 months ago
When a tab publishes a server via 'publishServer', and later the tab is closed without shutting down the server, the server is not shut down.
(Reporter)

Comment 1

11 months ago
Created attachment 8751815 [details] [diff] [review]
close-published-server-on-tab-close.patch

I think this is happening because when we observe the 'inner-window-destroyed' event, we only get it after the inner window has been destroyed.  This means that the nsPIDOMWindowInner* has already been deleted, and nulled out in the "PublishedServer" instance (I'm assuming DOMEventTargetHelper has some mechanism where it owner is nulled out when the window is destroyed).

Debugging showed what when we get the "inner-window-destroyed" event, the "GetOwner" call on the PublishedServer instance was returning null.

This patch changes things to just have the PublishedServer remember the WindowID directly, and checks against that window id instead.
Attachment #8751815 - Flags: review?(jonas)
Comment on attachment 8751815 [details] [diff] [review]
close-published-server-on-tab-close.patch

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

Thanks for finding this!

::: dom/flyweb/FlyWebPublishedServer.h
@@ +37,5 @@
>    virtual JSObject* WrapObject(JSContext *cx, JS::Handle<JSObject*> aGivenProto) override;
>  
>    NS_DECL_ISUPPORTS_INHERITED
>  
> +  uint64_t GetOwnerWindowID() const {

Drop the 'Get' part of the name. Gecko naming convention is that only getters which can fail should use the "Get" prefix IIRC.

(Though i'm not why the webidl compiler require them for string-getters as well)

::: dom/flyweb/FlyWebService.cpp
@@ +982,5 @@
>    NS_ENSURE_SUCCESS(rv, rv);
>  
>    for (FlyWebPublishedServer* server : mServers) {
> +    uint64_t serverWindowID = server->GetOwnerWindowID();
> +    if (serverWindowID == innerID) {

Get rid of the temporary.
Attachment #8751815 - Flags: review?(jonas) → review+
(Reporter)

Comment 3

11 months ago
https://hg.mozilla.org/projects/larch/rev/68dc67558a08
(Reporter)

Updated

11 months ago
Status: NEW → RESOLVED
Last Resolved: 11 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.