1168916 - Redundant pref callback in nsXULPrototypeCache

Status: RESOLVED FIXED

(Core :: XUL, defect)

Description

[(inactive) Jim Chen [:jchen] [:darchons]]

We have two pref callbacks in nsXULPrototypeCache for the same pref. We can merge the two.

Comment 1

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Get rid of redundant pref callback in nsXULPrototypeCache (v1)

This patch removes the redundant pref callback, and merges it into the other existing one.

Comment 2

[(inactive) Jim Chen [:jchen] [:darchons]]

Hi Jan, can you review these or should I ask someone else? Thanks!

Comment 3

[Jan Varga [:janv]]

Comment on attachment 8611308 [details] [diff] [review]
Get rid of redundant pref callback in nsXULPrototypeCache (v1)

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

Looks good, but please respond/address my comment below.

::: dom/xul/nsXULPrototypeCache.cpp
@@ +61,5 @@
>      // Flush the cache, regardless
>      nsXULPrototypeCache* cache = nsXULPrototypeCache::GetInstance();
> +    if (cache) {
> +        // AbortCaching() calls Flush() for us.
> +        cache->AbortCaching();

Hm, don't you want to check |wasEnabled| here like in the callback you're removing ?
If the cache was enabled then it makes sense to abort caching.
Not sure why we would need to call Flush() if the cache was just enabled.

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cb9fe5f0fd6c

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/cb9fe5f0fd6c