596808 - nsDiskCacheDevice::Init() called twice resulting in no disk cache available

Status: RESOLVED FIXED

(Core :: Networking: Cache, defect)

Description

[Bjarne (:bjarne)]

When FF checks for updates to addons/extensions on startup the disk-cache gets initialized because apparently something gets inserted. And later, when nsCacheService::OnProfileChange() is called it tries to call Init() again, which fails. The result is that the disk-cache seems to be unavailable. I typically see this if I use a profile using FF3.6, quits, and then connect to the same profile using FF4.

Comment 1

[Johnny Stenback (:jst)]

From what I've heard about this so far it means that some number of users (unclear what triggers this yet, if it's the presence of plugins, extensions, or something else) will run their first instance of Firefox with no disk cache at all. I don't think we want that to be peoples first experience with Firefox 4.

Comment 2

[Bjarne (:bjarne)]

Logs show that the responses to the versioncheck-requests are cached on disk, forcing creation of the disk-cache prior to the call to nsCacheService::OnProfileChange().

The requests are sent with "Cache-Control: no-cache" but the responses come with "Cache-Control: public".

Comment 3

[Bjarne (:bjarne)]

Attachment: As discussed...

Changing observer-topic in order to initialize the disk-cache before the version-checks happen.

Comment 4

[Jason Duell]

Comment on attachment 475994 [details] [diff] [review]
As discussed...

> static const char * observerList[] = { 
>     "profile-before-change",
>-    "profile-after-change",
>+    "profile-on-change",

>+    } else if (!strcmp("profile-on-change", topic)) {

These should be "profile-do-change".

> 
>+    NS_ASSERTION(!Initialized(), "Disk cache already initialized!");
>     NS_ENSURE_TRUE(!Initialized(), NS_ERROR_FAILURE);

We're slowly getting rid of NS_ENSURE_TRUE, so let's change to 

     if (Initialized) {
        NS_ERROR("Disk cache already initialized!");
        return NS_ERROR_UNEXPECTED;
     }

otherwise looks good.

Comment 5

[Bjarne (:bjarne)]

Attachment: Addressed comments from reviewer

Comment 6

[Jason Duell]

Comment on attachment 476014 [details] [diff] [review]
Addressed comments from reviewer

 
>-    NS_ENSURE_TRUE(!Initialized(), NS_ERROR_FAILURE);
>+    NS_ASSERTION(!Initialized(), "Disk cache already initialized!");
>+    if (Initialized()) {
>+        NS_ERROR("Disk cache already initialized!");
>+        return NS_ERROR_UNEXPECTED;
>+    }

Drop the NS_ASSERTION--that's what the NS_ERROR is for (it does the same thing, but w/o a test condition).

Comment 7

[Bjarne (:bjarne)]

Attachment: Addressed comments from reviewer

Comment 8

[Jason Duell]

http://hg.mozilla.org/mozilla-central/rev/b47978b94fc9