Last Comment Bug 726279 - Generalize use of Services.jsm in nsSearchService
: Generalize use of Services.jsm in nsSearchService
Status: RESOLVED FIXED
[mentor=Yoric][lang=js]
:
Product: Firefox
Classification: Client Software
Component: Search (show other bugs)
: unspecified
: All All
: -- enhancement (vote)
: Firefox 15
Assigned To: Raymond Lee [:raymondlee]
:
: Florian Quèze [:florian] [:flo]
Mentors:
Depends on: jsonSearchSvc
Blocks:
  Show dependency treegraph
 
Reported: 2012-02-11 01:44 PST by David Teller [:Yoric] (please use "needinfo")
Modified: 2012-04-29 15:44 PDT (History)
5 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v1 (17.96 KB, patch)
2012-04-27 07:00 PDT, Raymond Lee [:raymondlee]
no flags Details | Diff | Splinter Review
v2 (18.62 KB, patch)
2012-04-27 08:28 PDT, Raymond Lee [:raymondlee]
dteller: review+
dao+bmo: review+
Details | Diff | Splinter Review

Description David Teller [:Yoric] (please use "needinfo") 2012-02-11 01:44:30 PST
Followup to bug 699856.
Comment 1 David Teller [:Yoric] (please use "needinfo") 2012-02-15 11:37:27 PST
nsSearchService.js was written before Services.jsm . Now that we have Services.jsm, we can simplify code in a few places.
Comment 2 Raymond Lee [:raymondlee] 2012-04-27 07:00:42 PDT
Created attachment 619017 [details] [diff] [review]
v1
Comment 3 Dão Gottwald [:dao] 2012-04-27 07:22:04 PDT
Comment on attachment 619017 [details] [diff] [review]
v1

> let _dirSvc = null;
> function getDir(aKey, aIFace) {
>   if (!aKey)
>     FAIL("getDir requires a directory key!");
> 
>   if (!_dirSvc)
>-    _dirSvc = Cc["@mozilla.org/file/directory_service;1"].
>-               getService(Ci.nsIProperties);
>+    _dirSvc = Services.dirsvc;
>   return _dirSvc.get(aKey, aIFace || Ci.nsIFile);
> }

>+      var sbs = Services.strings;
> 
>       var brandBundle = sbs.createBundle(BRAND_BUNDLE);
>       var brandName = brandBundle.GetStringFromName("brandShortName");
> 
>       var searchBundle = sbs.createBundle(SEARCH_BUNDLE);

>+      var ss = Services.search;
>       if (ss.getEngineByName(aEngine.name)) {
>         if (aEngine._confirm)
>           onError("error_duplicate_engine_msg", "error_invalid_engine_title");
> 
>         LOG("_onLoad: duplicate engine found, bailing");
>         return;
>       }

_dirSvc, sbs and ss are unnecessary.
Comment 4 Raymond Lee [:raymondlee] 2012-04-27 08:28:42 PDT
Created attachment 619046 [details] [diff] [review]
v2

Removed _dirSvc, sbs and ss.
Comment 5 David Teller [:Yoric] (please use "needinfo") 2012-04-27 13:25:52 PDT
Comment on attachment 619046 [details] [diff] [review]
v2

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

Looks good to me. If this passes tests, you have my r+, but I defer to gavin for the final word.
Comment 6 Dão Gottwald [:dao] 2012-04-27 14:21:11 PDT
Comment on attachment 619046 [details] [diff] [review]
v2

stealing... four reviewing eyes should be sufficient for this
Comment 7 Ryan VanderMeulen [:RyanVM] 2012-04-28 08:22:19 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/6606ae2e48d4

Also, to make life easier for those checking in patches on your behalf, please follow the directions below for future patches you submit. Thanks!
https://developer.mozilla.org/en/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F
https://developer.mozilla.org/en/Creating_a_patch_that_can_be_checked_in
Comment 8 Ryan VanderMeulen [:RyanVM] 2012-04-29 15:44:48 PDT
http://hg.mozilla.org/mozilla-central/rev/6606ae2e48d4

Note You need to log in before you can comment on or make changes to this bug.