Closed Bug 1041335 Opened 10 years ago Closed 10 years ago

Add mozilla::services Getter for nsIServiceWorkerManager

Categories

(Core :: DOM: Workers, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla34

People

(Reporter: nsm, Assigned: lynn_tran, Mentored)

References

Details

(Whiteboard: [mentor=nsm.nikhil@gmail.com][lang=c++])

Attachments

(1 file, 2 obsolete files)

Whiteboard: [mentor=nsm.nikhil@gmail.com][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 <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.
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] 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; }
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] 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!
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] bug-1041335-fix.patch Review of attachment 8474897 [details] [diff] [review]: ----------------------------------------------------------------- Thanks a lot, Lynn! Your patch looks great.
Attachment #8474897 - Flags: review+
Flags: needinfo?(ehsan)
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: