1041335 - Add mozilla::services Getter for nsIServiceWorkerManager

Status: RESOLVED FIXED

(Core :: DOM: Workers, enhancement)

Comment 1

[Lynn Tran]

Hi, I'm interested in this bug, can I have some more information about this bug to tackle it?

Comment 2

[(no longer active)]

Thanks for your interest, Lynn!

In order to fix this bug, you need to add nsIServiceWorkerManager to <http://mxr.mozilla.org/mozilla-central/source/xpcom/build/ServiceList.h>.  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 <http://hg.mozilla.org/mozilla-central/rev/dfe5ab77a706> which is an example of how this has been done for another XPCOM service.

Comment 3

[Lynn Tran]

Attachment: bug-1041335-fix.patch

Comment 4

[(no longer active)]

Comment on attachment 8470353 [details] [diff] [review]
bug-1041335-fix.patch

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: <http://mxr.mozilla.org/mozilla-central/search?string=do_GetService%28SERVICEWORKERMANAGER_CONTRACTID>

::: 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) {
    aRv.Throw(NS_ERROR_FAILURE);
    return nullptr;
  }

Comment 5

[Lynn Tran]

Attachment: bug-1041335-fix.patch

Comment 6

[Lynn Tran]

Some of the lines that the search result yields don't exist

Comment 7

[(no longer active)]

Comment on attachment 8470384 [details] [diff] [review]
bug-1041335-fix.patch

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 "@mozilla.org/serviceworkers/manager;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);
layout/build/nsLayoutModule.cpp:1169:  { SERVICEWORKERMANAGER_CONTRACTID, &kSERVICEWORKERMANAGER_CID },

Can you please try updating your tree and convert the rest of these?  Thanks!

Comment 8

[Lynn Tran]

Attachment: bug-1041335-fix.patch

Comment 9

[(no longer active)]

Comment on attachment 8474897 [details] [diff] [review]
bug-1041335-fix.patch

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

Thanks a lot, Lynn!  Your patch looks great.

Comment 10

[(no longer active)]

I landed the patch for you: https://hg.mozilla.org/integration/mozilla-inbound/rev/33fcf51e6d13

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/33fcf51e6d13