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

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: bjarne, Assigned: bjarne)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(blocking2.0 betaN+)

Details

Attachments

(1 attachment, 2 obsolete attachments)

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.
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.
blocking2.0: --- → betaN+
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".
Summary: nsDiskCacheDevice() called twice resulting in no disk cache available → nsDiskCacheDevice::Init() called twice resulting in no disk cache available
Assignee: nobody → bjarne
Posted patch As discussed... (obsolete) — Splinter Review
Changing observer-topic in order to initialize the disk-cache before the version-checks happen.
Attachment #475994 - Flags: review?(jduell.mcbugs)
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.
Attachment #475994 - Flags: review?(jduell.mcbugs) → review+
Attachment #475994 - Attachment is obsolete: true
Attachment #476014 - Flags: review+
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).
Attachment #476014 - Attachment is obsolete: true
Attachment #476125 - Flags: review+
http://hg.mozilla.org/mozilla-central/rev/b47978b94fc9
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Duplicate of this bug: 589810
You need to log in before you can comment on or make changes to this bug.