Implement Client.postMessage message source

RESOLVED DUPLICATE of bug 1143717

Status

()

RESOLVED DUPLICATE of bug 1143717
4 years ago
3 years ago

People

(Reporter: catalinb, Assigned: catalinb)

Tracking

(Blocks: 1 bug)

Trunk
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox40 affected)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
Messages dispatched by the service worker to a service worker client should have a source object.
(Assignee)

Comment 1

4 years ago
Created attachment 8592856 [details] [diff] [review]
Add source to messages dispatched by ServiceWorkerClient.postMessage.
Attachment #8592856 - Flags: review?(nsm.nikhil)
Comment on attachment 8592856 [details] [diff] [review]
Add source to messages dispatched by ServiceWorkerClient.postMessage.

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

You cannot pass around the worker private to the main thread without ensuring it stays alive. It seems like you only need the ID though, so why not just pass that?
Where was the ID introduced?

::: dom/workers/ServiceWorkerClient.cpp
@@ +142,5 @@
>        xpc::Throw(aCx, NS_ERROR_DOM_DATA_CLONE_ERR);
>        return NS_ERROR_FAILURE;
>      }
>  
> +    nsRefPtr<nsIDOMMessageEvent> event = new MessageEvent(aTargetContainer,

Use nsRefPtr<MessageEvent>...

@@ +156,5 @@
>        xpc::Throw(aCx, rv);
>        return NS_ERROR_FAILURE;
>      }
>  
> +    static_cast<MessageEvent*>(event.get())->SetSource(aSource);

...and you don't need this cast.
Attachment #8592856 - Flags: review?(nsm.nikhil) → review-
(Assignee)

Comment 3

4 years ago
(In reply to Nikhil Marathe [:nsm] (needinfo? please) from comment #2)
> Comment on attachment 8592856 [details] [diff] [review]
> Add source to messages dispatched by ServiceWorkerClient.postMessage.
> 
> Review of attachment 8592856 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> You cannot pass around the worker private to the main thread without
> ensuring it stays alive. It seems like you only need the ID though, so why
> not just pass that?
An oversight, I'll fix it.

> Where was the ID introduced?
Please see bug 1130684. I expect you'll have some comments about the id thing.

> ::: dom/workers/ServiceWorkerClient.cpp
> @@ +142,5 @@
> >        xpc::Throw(aCx, NS_ERROR_DOM_DATA_CLONE_ERR);
> >        return NS_ERROR_FAILURE;
> >      }
> >  
> > +    nsRefPtr<nsIDOMMessageEvent> event = new MessageEvent(aTargetContainer,
> 
> Use nsRefPtr<MessageEvent>...
> 
> @@ +156,5 @@
> >        xpc::Throw(aCx, rv);
> >        return NS_ERROR_FAILURE;
> >      }
> >  
> > +    static_cast<MessageEvent*>(event.get())->SetSource(aSource);
> 
> ...and you don't need this cast.

MessageEvent's downcast to nsIDOMMessageEvent is ambiguous (which is the argument type of the dispatch event method). Either way, I'll need a cast.
Depends on: 1130684

Updated

3 years ago
Blocks: 1173500
No longer blocks: 1059784
Keywords: dev-doc-needed

Updated

3 years ago
Status: NEW → ASSIGNED
Blocks: 1226983

Comment 4

3 years ago
Catalin, are you still planning to work on this?

Thanks!
(Assignee)

Comment 5

3 years ago
This was fixed by dimi in bug 1143717.
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1143717
Keywords: dev-doc-needed
You need to log in before you can comment on or make changes to this bug.