Push HTTP cache index build when asked for disk cache size

RESOLVED FIXED in Firefox 49

Status

()

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: mayhemer, Assigned: mayhemer)

Tracking

unspecified
mozilla49
Points:
---

Firefox Tracking Flags

(firefox49 fixed)

Details

(Whiteboard: [necko-active])

Attachments

(1 attachment, 4 obsolete attachments)

Comment hidden (empty)
(Assignee)

Updated

3 years ago
Blocks: 428916
(Assignee)

Comment 1

3 years ago
Created attachment 8754842 [details] [diff] [review]
v1
Attachment #8754842 - Flags: review?(michal.novotny)
Comment on attachment 8754842 [details] [diff] [review]
v1

This seems to be a wrong patch.
Flags: needinfo?(honzab.moz)
Attachment #8754842 - Flags: review?(michal.novotny)
(Assignee)

Comment 3

3 years ago
Created attachment 8755110 [details] [diff] [review]
v1

This is the one.
Attachment #8754842 - Attachment is obsolete: true
Flags: needinfo?(honzab.moz)
Attachment #8755110 - Flags: review?(michal.novotny)
Comment on attachment 8755110 [details] [diff] [review]
v1

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

::: netwerk/cache2/CacheIndex.cpp
@@ +1372,5 @@
> +  // Move forward with index re/building if it is pending
> +  RefPtr<CacheIOThread> ioThread = CacheFileIOManager::IOThread();
> +  if (ioThread) {
> +    ioThread->Dispatch(NS_NewRunnableFunction([]() -> void {
> +      RefPtr<CacheIndex> index = gInstance;

gInstance must be accessed under sLock
Attachment #8755110 - Flags: review?(michal.novotny) → feedback+
Whiteboard: [necko-active]
(Assignee)

Comment 6

3 years ago
Created attachment 8756364 [details] [diff] [review]
v1.1
Attachment #8755110 - Attachment is obsolete: true
Attachment #8756364 - Flags: review?(michal.novotny)
Comment on attachment 8756364 [details] [diff] [review]
v1.1

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

::: netwerk/cache2/CacheIndex.cpp
@@ +1378,5 @@
> +      RefPtr<CacheIndex> index = gInstance;
> +      if (index && index->mUpdateTimer) {
> +        index->mUpdateTimer->Cancel();
> +        index->mUpdateTimer = nullptr;
> +        index->DelayedUpdate(nullptr, nullptr);

DelayedUpdate() grabs the lock too, so you have to call it outside the lock.
Attachment #8756364 - Flags: review?(michal.novotny) → feedback+
(Assignee)

Comment 8

3 years ago
Created attachment 8757370 [details] [diff] [review]
v1.2
Attachment #8756364 - Attachment is obsolete: true
Attachment #8757370 - Flags: review?(michal.novotny)
Comment on attachment 8757370 [details] [diff] [review]
v1.2

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

::: netwerk/cache2/CacheIndex.cpp
@@ +2517,5 @@
> +}
> +
> +// static
> +void
> +CacheIndex::DelayedUpdateUnlocked()

If think DelayedUpdateLocked makes IMO more sense.

@@ +2523,1 @@
>    LOG(("CacheIndex::DelayedUpdate()"));

Please update logged names too.

@@ +2529,5 @@
>    RefPtr<CacheIndex> index = gInstance;
>  
>    if (!index) {
>      return;
>    }

Could you make DelayedUpdateLocked non-static method and check gInstance in DelayedUpdate?
Attachment #8757370 - Flags: review?(michal.novotny) → feedback+
(Assignee)

Comment 10

3 years ago
Created attachment 8757890 [details] [diff] [review]
v1.3
Attachment #8757370 - Attachment is obsolete: true
Attachment #8757890 - Flags: review?(michal.novotny)
Comment on attachment 8757890 [details] [diff] [review]
v1.3

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

thanks!
Attachment #8757890 - Flags: review?(michal.novotny) → review+

Comment 14

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/6750817f09e1
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox49: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in before you can comment on or make changes to this bug.