1290318 - Media cache is not cleared when the main cache is cleared

Status: VERIFIED FIXED

(Core :: Audio/Video: Playback, defect, P1)

Description

[tenbob0]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:47.0) Gecko/20100101 Firefox/47.0
Build ID: 20160623154057

Steps to reproduce:

I have just discovered that Firefox has a media cache which is completely separate from the main cache. The problem I am seeing is the media cache does not get cleared as expected. This results in stale content being served from the cache when you try to refresh the page. 
 
Steps to reproduce.
(1) View a webpage containing an HTML5 <video> element. 
(2) Update the webserver by uploading a different video file with the same filename.
(3) Try to reload the page in Firefox. Whatever you do you can't get the new video to download and display. Firefox keeps playing the old version from the cache.




Expected results:

Any action which forces the page to reload should clear the media cache and download the new file. Specifically the following should clear the media cache. 
- User selects 'Clear recent history' and deletes everything.
- User turns off caching in Developer tools.
- User forces a complete page reload with Ctrl+F5.

As far as I can tell the only way to clear the media cache is to close Firefox and open it again.

Note Bug 848344 claims this has been fixed when the user selects 'Clear recent history'. It's not working for me.

Comment 1

[Tooru Fujisawa [:arai]]

This is partially regression.

tested with the following steps:

  1. open nightly
  2. open the HTML file with video element
  3. play
  4. replace the video file on server side
  5. force reload
  6. play
  7. Clear recent history - Everything
  8. force reload
  9. play

"force reload" (Ctrl+F5) in step 5 doesn't work at all.
"Clear recent history" in step 7 was working after bug 848344 fix, but it's regressed in the following range:
  https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=e2e1b19fcffc&tochange=8d591a3f6fea

Comment 2

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

I'm assuming that this is a pain for people developing against Firefox and isn't super hard to fix. I'm therefore marking it as a P1 because it is also a regression.

Comment 3

[James Willcox (:snorp) ([e-mail]) (he/him)]

Anthony do you want to assign someone to this?

Comment 4

[Anthony Jones (:ajones, :kentuckyfriedtakahe, :k17e)]

(In reply to James Willcox (:snorp) ([e-mail]) from comment #3)
> Anthony do you want to assign someone to this?

No. I don't assign bugs until I know who is going to work on them. There are 4 possible people who may work on this bug.

Comment 5

[JW Wang [:jwwang] [:jw_wang]]

(In reply to Tooru Fujisawa [:arai] from comment #1)
> This is partially regression.
> 
> tested with the following steps:
> 
>   1. open nightly
>   2. open the HTML file with video element
>   3. play
>   4. replace the video file on server side
>   5. force reload
>   6. play
>   7. Clear recent history - Everything
>   8. force reload
>   9. play
> 
> "force reload" (Ctrl+F5) in step 5 doesn't work at all.
> "Clear recent history" in step 7 was working after bug 848344 fix, but it's
> regressed in the following range:
>  
> https://hg.mozilla.org/mozilla-central/
> pushloghtml?fromchange=e2e1b19fcffc&tochange=8d591a3f6fea

I can't repro the issue by uploading small files to http://people.mozilla.org. Can you upload the files to bugzilla so I can have a test with them?

Comment 6

[Tooru Fujisawa [:arai]]

Attachment: testcase that have video element

here's HTML file.
I used video files that is not mine, so I cannot upload them here.
I'll search something else, give me some time.

Comment 7

[Tooru Fujisawa [:arai]]

Attachment: full testcase with replacing video

okay, prepared full testcase.

Steps to reproduce:

1. Put all files in this archive on web server
2. Run before_test.sh (or copy test1.webm to test.webm)
3. Run nightly
4. Open test.html from web server (http://.../test.html)
5. Play the video
6. Run replace_video.sh (or copy test2.webm to test.webm)
7. Force reload
8. Play the video
9. Clear recent history - Everything
10. Force reload
11. Play the video


Actual result:

test1.webm (video with "A") is played in steps 5, 8, and 11.


Expeceted result:

test1.webm (video with "A") is played in step 5.
test2.webm (video with "B") is played in steps 8 and 11.

Comment 8

[JW Wang [:jwwang] [:jw_wang]]

Thanks! I will take a look.

Comment 9

[JW Wang [:jwwang] [:jw_wang]]

https://hg.mozilla.org/mozilla-central/annotate/331c4166a3a2df2d3a037107addef5d85cdc31b5/netwerk/cache/nsCacheService.cpp#l1524

Somehow |Clear recent history - Everything| doesn't trigger nsCacheService::EvictEntries().

Hi Honza,
Do you know what will trigger nsCacheService::EvictEntries()?

Comment 10

[Honza Bambas (:mayhemer)]

(In reply to JW Wang [:jwwang] from comment #9)
> https://hg.mozilla.org/mozilla-central/annotate/
> 331c4166a3a2df2d3a037107addef5d85cdc31b5/netwerk/cache/nsCacheService.
> cpp#l1524
> 
> Somehow |Clear recent history - Everything| doesn't trigger
> nsCacheService::EvictEntries().
> 
> Hi Honza,
> Do you know what will trigger nsCacheService::EvictEntries()?

Nothing and don't do it.  It's the old HTTP cache v1 code, now obsolete and not used.  We now have nsICacheStorageService.clear() which is normally called from Clear Recent/All History dialog (when cache is checked to be cleared).

Media cache is disconnected from HTTP cache.

If you still believe there is HTTP cache involved here, please describe why should that be or lead me to a comment in this bug that does.

Comment 11

[JW Wang [:jwwang] [:jw_wang]]

https://hg.mozilla.org/mozilla-central/annotate/331c4166a3a2df2d3a037107addef5d85cdc31b5/dom/media/MediaCache.cpp#l360

MediaCache used to listen to "network-clear-cache-stored-anywhere" to clear the cache data and we don't have the notification anymore since cache v1 is obsolete. What should we do to have the behavior in the new HTTP cache v2?

Comment 12

[Honza Bambas (:mayhemer)]

(In reply to JW Wang [:jwwang] from comment #11)
> https://hg.mozilla.org/mozilla-central/annotate/
> 331c4166a3a2df2d3a037107addef5d85cdc31b5/dom/media/MediaCache.cpp#l360
> 
> MediaCache used to listen to "network-clear-cache-stored-anywhere" to clear
> the cache data and we don't have the notification anymore since cache v1 is
> obsolete. What should we do to have the behavior in the new HTTP cache v2?

You have "cacheservice:empty-cache" notification these days.  Works the same way.

Comment 13

[JW Wang [:jwwang] [:jw_wang]]

(In reply to Honza Bambas (:mayhemer) from comment #12)
> You have "cacheservice:empty-cache" notification these days.  Works the same
> way.

Somehow, the notification isn't received in an e10s tab. Is there something I missed?

Comment 14

[Honza Bambas (:mayhemer)]

(In reply to JW Wang [:jwwang] from comment #13)
> (In reply to Honza Bambas (:mayhemer) from comment #12)
> > You have "cacheservice:empty-cache" notification these days.  Works the same
> > way.
> 
> Somehow, the notification isn't received in an e10s tab. Is there something
> I missed?

It's only a chrome notification.  There is no code to send it to a child process(es).  We either need to do it in necko (prefered - needs a new bug) or you may do it on your side.

Comment 15

[JW Wang [:jwwang] [:jw_wang]]

Attachment: Bug 1290318 - listen to "cacheservice:empty-cache" to clear media cache.

Review commit: https://reviewboard.mozilla.org/r/70562/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/70562/

Comment 16

[Honza Bambas (:mayhemer)]

Comment on attachment 8779614 [details]
Bug 1290318 - listen to "cacheservice:empty-cache" to clear media cache.

https://reviewboard.mozilla.org/r/70562/#review68002

::: dom/ipc/PContent.ipdl:665
(Diff revision 1)
>      async NotifyGMPsChanged();
>  
>      /**
> +     * Notify the child that cache is emptied.
> +     */
> +    async NotifyEmptyCache();

nit: maybe worth naming it NotifyEmptyHTTPCache()

Comment 17

[JW Wang [:jwwang] [:jw_wang]]

Thanks! Will change the name.

Comment 18

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8779614 [details]
Bug 1290318 - listen to "cacheservice:empty-cache" to clear media cache.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/70562/diff/1-2/

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/517d1167728f
listen to "cacheservice:empty-cache" to clear media cache. r=mayhemer

Comment 21

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/517d1167728f

Comment 22

[Gerry Chang [:gchang]]

This needs QE's help to verify.

Comment 23

[Gerry Chang [:gchang]]

Hi :jwwang,
Since this bug is a regression and also affects 49/50, are you also considering to uplift this patch to 49/50?

Comment 24

[JW Wang [:jwwang] [:jw_wang]]

Attachment: 1290318_fix_aurora_beta.patch

Approval Request Comment
[Feature/regressing bug #]:some bug about HTTP cache v2
[User impact if declined]: |Clear recent history -> Everything| will not clear media cache.
[Describe test coverage new/current, TreeHerder]:TreeHerder
[Risks and why]: Low. The change is simple.
[String/UUID change made/needed]:none

Comment 25

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hello tenbob0, could you please verify this issue is fixed as expected on a latest Nightly build? Thanks!

Comment 26

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8781836 [details] [diff] [review]
1290318_fix_aurora_beta.patch

While this is not a new regression, it seems like a pretty basic scenario which is why I think it makes sense to uplift the fix to Aurora channel. The fix has stability on Nightly for a few days. Aurora50+

Comment 27

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ae48669d6b07

Comment 28

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8781836 [details] [diff] [review]
1290318_fix_aurora_beta.patch

Agreed, this must be a commonly encountered situation. Let's uplift for beta 6.

Comment 29

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/42ce30d2775a

Comment 30

[Cornel Ionce [:noni] [Hubs QA]]

I can confirm that the expected results are met on Windows 7x64 using latest 51.0a1 Nightly, latest 50.0a2 Aurora and 49.0 beta 8.

Tested with the attached testcase using the STR from comment 7.

Comment 31

[Marty]

In 52 nightly HTML5 video memory is not being released, either from clearing the cache or freeing memory in about:memory. The only memory cleared appears to be HTML entries. As a result, memory just keeps going up after watching HTML5 videos. 

This is in contrast to 45esr, which will release hundreds of mbs of ram (I only cache into memory and not the hard drive) when freeing memory from about:memory.

Should a separate bug be filed?

Comment 32

[JW Wang [:jwwang] [:jw_wang]]

Yes. Please file a new bug and describe the current behavior and the expected behavior. I believe this has something to do with e10s which is not enabled in 45.