Last Comment Bug 660768 - modules should use mozilla::Preferences
: modules should use mozilla::Preferences
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: General (show other bugs)
: Trunk
: All All
-- normal (vote)
: mozilla7
Assigned To: Masayuki Nakano [:masayuki]
:
:
Mentors:
Depends on: 656826 665388
Blocks:
  Show dependency treegraph
 
Reported: 2011-05-31 01:58 PDT by Masayuki Nakano [:masayuki]
Modified: 2011-09-01 02:49 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1.0 (9.38 KB, patch)
2011-05-31 01:58 PDT, Masayuki Nakano [:masayuki]
roc: review+
joe: review+
Details | Diff | Splinter Review
Patch v1.1 (9.32 KB, patch)
2011-06-09 18:06 PDT, Masayuki Nakano [:masayuki]
taras.mozilla: review+
Details | Diff | Splinter Review

Description User image Masayuki Nakano [:masayuki] 2011-05-31 01:58:12 PDT
Created attachment 536254 [details] [diff] [review]
Patch v1.0
Comment 1 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2011-05-31 02:30:52 PDT
Comment on attachment 536254 [details] [diff] [review]
Patch v1.0

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

So imglib doesn't unregister its observers either? Would be nice to fix :-)
Comment 2 User image Masayuki Nakano [:masayuki] 2011-06-02 23:37:47 PDT
(In reply to comment #1)
> So imglib doesn't unregister its observers either? Would be nice to fix :-)

Hmm... I have no idea to unregister it. Looks like imgRequest isn't singleton. The first instance registers an observer as strong. The observer is destroyed by nsPrefBranch at XPCOM shutdown. So, it doesn't make sense to remove the observer from its destructor.
Comment 3 User image Joe Drew (not getting mail) 2011-06-09 11:29:35 PDT
Comment on attachment 536254 [details] [diff] [review]
Patch v1.0

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

I like it!

::: modules/libpr0n/src/imgLoader.cpp
@@ +914,2 @@
>  {
> +  NS_ASSERTION(Preferences::GetRootBranch(), "Pref branch is null");

You could just drop this assertion entirely, IMO.
Comment 4 User image Masayuki Nakano [:masayuki] 2011-06-09 18:06:17 PDT
Created attachment 538403 [details] [diff] [review]
Patch v1.1
Comment 5 User image Masayuki Nakano [:masayuki] 2011-06-11 20:31:18 PDT
http://hg.mozilla.org/mozilla-central/rev/3f771ec754bb
Comment 6 User image Mihaela Velimiroviciu (:mihaelav) 2011-09-01 02:49:34 PDT
Verified that the following files are updated un mozilla-central repository:
modules/libjar/nsJARChannel.cpp
modules/libpr0n/src/DiscardTracker.cpp
modules/libpr0n/src/imgLoader.cpp
modules/libpr0n/src/imgLoader.h
modules/libpr0n/src/imgRequest.cpp

Is this enough in order to verfy the fix and mark the bug accordingly?

Thank you!

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