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] (Mozilla Japan)
:
Mentors:
Depends on: 656826 665388
Blocks:
  Show dependency treegraph
 
Reported: 2011-05-31 01:58 PDT by Masayuki Nakano [:masayuki] (Mozilla Japan)
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] (Mozilla Japan)
roc: review+
joe: review+
Details | Diff | Splinter Review
Patch v1.1 (9.32 KB, patch)
2011-06-09 18:06 PDT, Masayuki Nakano [:masayuki] (Mozilla Japan)
taras.mozilla: review+
Details | Diff | Splinter Review

Description Masayuki Nakano [:masayuki] (Mozilla Japan) 2011-05-31 01:58:12 PDT
Created attachment 536254 [details] [diff] [review]
Patch v1.0
Comment 1 Robert O'Callahan (:roc) (Exited; 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 Masayuki Nakano [:masayuki] (Mozilla Japan) 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 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 Masayuki Nakano [:masayuki] (Mozilla Japan) 2011-06-09 18:06:17 PDT
Created attachment 538403 [details] [diff] [review]
Patch v1.1
Comment 5 Masayuki Nakano [:masayuki] (Mozilla Japan) 2011-06-11 20:31:18 PDT
http://hg.mozilla.org/mozilla-central/rev/3f771ec754bb
Comment 6 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.