Closed Bug 1041335 Opened 10 years ago Closed 9 years ago

Add mozilla::services Getter for nsIServiceWorkerManager


(Core :: DOM: Workers, enhancement)

Not set





(Reporter: nsm, Assigned: lynn_tran, Mentored)



(Whiteboard: [][lang=c++])


(1 file, 2 obsolete files)

Whiteboard: [][lang=c++]
Mentor: nsm.nikhil → ehsan
Hi, I'm interested in this bug, can I have some more information about this bug to tackle it?
Thanks for your interest, Lynn!

In order to fix this bug, you need to add nsIServiceWorkerManager to <>.  This file defines the list of XPCOM services available from the mozilla::services namespace.  Once you have done that, you want to convert the existing consumers of this service which use do_GetService() to get access to it to use the newly added mozilla::services::ServiceWorkerManager() function instead.  Please see <> which is an example of how this has been done for another XPCOM service.
Assignee: nobody → lynn_tran
Attached patch bug-1041335-fix.patch (obsolete) — Splinter Review
Attachment #8470353 - Flags: feedback?(ehsan)
Flags: needinfo?(ehsan)
Comment on attachment 8470353 [details] [diff] [review]

Review of attachment 8470353 [details] [diff] [review]:

This is a good start!  Please see my comment below.  Note that you should probably change all of the occurrences in this list: <>

::: dom/workers/ServiceWorkerContainer.cpp
@@ -43,5 @@
>    nsCOMPtr<nsISupports> promise;
> -  nsresult rv;
> -  nsCOMPtr<nsIServiceWorkerManager> swm = do_GetService(SERVICEWORKERMANAGER_CONTRACTID, &rv);
> -  if (NS_WARN_IF(NS_FAILED(rv))) {

You should not omit the error checking paths here and in the other place below.  You should probably do something like:

  if (!swm) {
    return nullptr;
Attachment #8470353 - Flags: feedback?(ehsan) → feedback+
Flags: needinfo?(ehsan)
Attached patch bug-1041335-fix.patch (obsolete) — Splinter Review
Attachment #8470353 - Attachment is obsolete: true
Attachment #8470384 - Flags: feedback?(ehsan)
Flags: needinfo?(ehsan)
Some of the lines that the search result yields don't exist
Comment on attachment 8470384 [details] [diff] [review]

Review of attachment 8470384 [details] [diff] [review]:

This looks good, but I think your tree is out of date, and that is why you're not seeing the rest of the occurrences of SERVICEWORKERMANAGER_CONTRACTID.  Here's what I get on my local tree if I grep SERVICEWORKERMANAGER_CONTRACTID:

content/base/src/nsDocument.cpp:4472:    nsCOMPtr<nsIServiceWorkerManager> swm = do_GetService(SERVICEWORKERMANAGER_CONTRACTID);
content/base/src/nsDocument.cpp:8496:  nsCOMPtr<nsIServiceWorkerManager> swm = do_GetService(SERVICEWORKERMANAGER_CONTRACTID);
dom/interfaces/base/nsIServiceWorkerManager.idl:50:#define SERVICEWORKERMANAGER_CONTRACTID ";1"
dom/workers/ServiceWorkerContainer.cpp:64:  nsCOMPtr<nsIServiceWorkerManager> swm = do_GetService(SERVICEWORKERMANAGER_CONTRACTID, &rv);
dom/workers/ServiceWorkerContainer.cpp:87:  nsCOMPtr<nsIServiceWorkerManager> swm = do_GetService(SERVICEWORKERMANAGER_CONTRACTID, &rv);
dom/workers/ServiceWorkerContainer.cpp:141:    nsCOMPtr<nsIServiceWorkerManager> swm = do_GetService(SERVICEWORKERMANAGER_CONTRACTID, &rv);
dom/workers/ServiceWorkerContainer.cpp:180:  nsCOMPtr<nsIServiceWorkerManager> swm = do_GetService(SERVICEWORKERMANAGER_CONTRACTID);
dom/workers/ServiceWorkerContainer.cpp:189:  nsCOMPtr<nsIServiceWorkerManager> swm = do_GetService(SERVICEWORKERMANAGER_CONTRACTID);
dom/workers/ServiceWorkerContainer.cpp:215:  nsCOMPtr<nsIServiceWorkerManager> swm = do_GetService(SERVICEWORKERMANAGER_CONTRACTID, &rv);
dom/workers/ServiceWorkerContainer.cpp:258:  nsCOMPtr<nsIServiceWorkerManager> swm = do_GetService(SERVICEWORKERMANAGER_CONTRACTID, &rv);
dom/workers/ServiceWorkerManager.cpp:612:    do_GetService(SERVICEWORKERMANAGER_CONTRACTID);

Can you please try updating your tree and convert the rest of these?  Thanks!
Attachment #8470384 - Flags: feedback?(ehsan) → feedback+
Flags: needinfo?(ehsan)
Attachment #8470384 - Attachment is obsolete: true
Flags: needinfo?(ehsan)
Comment on attachment 8474897 [details] [diff] [review]

Review of attachment 8474897 [details] [diff] [review]:

Thanks a lot, Lynn!  Your patch looks great.
Attachment #8474897 - Flags: review+
I landed the patch for you:
Flags: needinfo?(ehsan)
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in before you can comment on or make changes to this bug.