Closed Bug 564950 Opened 10 years ago Closed 10 years ago

Make more use of mozilla::services

Categories

(Core :: XPCOM, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: Mitch, Assigned: Mitch)

References

Details

Attachments

(15 files, 2 obsolete files)

2.14 KB, patch
surkov
: review+
Details | Diff | Splinter Review
1.05 KB, patch
jst
: review+
Details | Diff | Splinter Review
7.50 KB, patch
jst
: review+
Details | Diff | Splinter Review
6.82 KB, patch
jst
: review+
Details | Diff | Splinter Review
dom
745 bytes, patch
jst
: review+
Details | Diff | Splinter Review
1.41 KB, patch
neil
: review+
Details | Diff | Splinter Review
4.76 KB, patch
smontagu
: review+
Details | Diff | Splinter Review
5.99 KB, patch
roc
: review+
Details | Diff | Splinter Review
1.95 KB, patch
jaas
: review+
Details | Diff | Splinter Review
1.25 KB, patch
jst
: review+
Details | Diff | Splinter Review
6.47 KB, patch
Gavin
: review+
Details | Diff | Splinter Review
6.11 KB, patch
roc
: review+
Details | Diff | Splinter Review
1.51 KB, patch
shaver
: review+
Details | Diff | Splinter Review
765 bytes, patch
neil
: review+
Details | Diff | Splinter Review
48.65 KB, patch
Details | Diff | Splinter Review
We should replace more do_GetService() calls with mozilla::services::Get*Service(), as was done in bug 560095 (which itself stems from bug 516085).
Attached patch Patch v1 (obsolete) — Splinter Review
This patch touches several modules and passes TryServer. Appropriate peers/owners should state which part(s) they're reviewing.
Attachment #444539 - Flags: review?(surkov.alexander
Attachment #444539 - Flags: review?(smontagu)
Attachment #444539 - Flags: review?(shaver)
Attachment #444539 - Flags: review?(roc)
Attachment #444539 - Flags: review?(neil)
Attachment #444539 - Flags: review?(jst)
Attachment #444539 - Flags: review?(joshmoz)
Attachment #444539 - Flags: review?(gavin.sharp)
Attachment #444539 - Flags: review?(bzbarsky)
Comment on attachment 444539 [details] [diff] [review]
Patch v1

+    nsCOMPtr<nsIStringBundleService> strings =
+      mozilla::services::GetStringBundleService();
+    if (!strings)
+      NS_ERROR_FAILURE;

Missing "return".
Attachment #444539 - Flags: review?(joshmoz) → review-
Attached patch Patch v1.1 (obsolete) — Splinter Review
Fixed.
Attachment #444539 - Attachment is obsolete: true
Attachment #444555 - Flags: review?(surkov.alexander)
Attachment #444555 - Flags: review?(smontagu)
Attachment #444555 - Flags: review?(shaver)
Attachment #444555 - Flags: review?(roc)
Attachment #444555 - Flags: review?(jst)
Attachment #444555 - Flags: review?(joshmoz)
Attachment #444555 - Flags: review?(gavin.sharp)
Attachment #444555 - Flags: review?(bzbarsky)
Attachment #444539 - Flags: review?(surkov.alexander)
Attachment #444539 - Flags: review?(smontagu)
Attachment #444539 - Flags: review?(shaver)
Attachment #444539 - Flags: review?(roc)
Attachment #444539 - Flags: review?(neil)
Attachment #444539 - Flags: review?(jst)
Attachment #444539 - Flags: review?(gavin.sharp)
Attachment #444539 - Flags: review?(bzbarsky)
Could you break it up so that people know which part they're supposed to review?
Attached patch accessibleSplinter Review
Attachment #444555 - Attachment is obsolete: true
Attachment #444561 - Flags: review?(surkov.alexander)
Attachment #444555 - Flags: review?(surkov.alexander)
Attachment #444555 - Flags: review?(smontagu)
Attachment #444555 - Flags: review?(shaver)
Attachment #444555 - Flags: review?(roc)
Attachment #444555 - Flags: review?(jst)
Attachment #444555 - Flags: review?(joshmoz)
Attachment #444555 - Flags: review?(gavin.sharp)
Attachment #444555 - Flags: review?(bzbarsky)
Attached patch capsSplinter Review
Attachment #444562 - Flags: review?(bzbarsky)
Attached patch contentSplinter Review
Attachment #444563 - Flags: review?(bzbarsky)
Attached patch docshellSplinter Review
Attachment #444564 - Flags: review?(jst)
Attached patch domSplinter Review
Attachment #444565 - Flags: review?(jst)
Attached patch editorSplinter Review
Attachment #444566 - Flags: review?(neil)
Attached patch intlSplinter Review
Attachment #444567 - Flags: review?(smontagu)
Attached patch layoutSplinter Review
Attachment #444568 - Flags: review?(roc)
Attached patch modulesSplinter Review
Attachment #444569 - Flags: review?(joshmoz)
Attached patch parserSplinter Review
Attachment #444570 - Flags: review?(jst)
Attached patch toolkitSplinter Review
Attachment #444571 - Flags: review?(gavin.sharp)
Attached patch widgetSplinter Review
Attachment #444572 - Flags: review?(roc)
Attached patch xpcomSplinter Review
Attachment #444573 - Flags: review?(shaver)
Attachment #444573 - Flags: review?(tglek)
Attached patch xpfeSplinter Review
Attachment #444575 - Flags: review?(neil)
Attachment #444569 - Flags: review?(joshmoz) → review+
Attachment #444566 - Flags: review?(neil) → review+
Attachment #444575 - Flags: review?(neil) → review+
Comment on attachment 444573 [details] [diff] [review]
xpcom

TBH, I think that we should just crash if we're in the middle of running the app and can't get a key service, but that's a battle for clarity and minimalism that I'll fight in another context!
Attachment #444573 - Flags: review?(tglek)
Attachment #444573 - Flags: review?(shaver)
Attachment #444573 - Flags: review+
Attachment #444571 - Flags: review?(gavin.sharp) → review+
Attachment #444564 - Flags: review?(jst) → review+
Attachment #444562 - Flags: review?(bzbarsky) → review+
Attachment #444563 - Flags: review?(bzbarsky) → review+
Attachment #444565 - Flags: review?(jst) → review+
Attachment #444570 - Flags: review?(jst) → review+
Attachment #444561 - Flags: review?(surkov.alexander) → review+
Attachment #444567 - Flags: review?(smontagu) → review+
Thanks for the reviews. This amalgamation is ready for check-in. Patch includes Hg changeset summary.
landed on 1.9.3 - http://hg.mozilla.org/mozilla-central/rev/840fcbf52747
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.