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

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Kannan Vijayan [:djvj]]

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.

Comment 1

[Kannan Vijayan [:djvj]]

Attachment: 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.

Comment 2

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

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.

Comment 3

[Kannan Vijayan [:djvj]]

https://hg.mozilla.org/projects/larch/rev/68dc67558a08