1248498 - Debugger shows stale HTML files if toolbox disabled caching

Status: VERIFIED FIXED

(DevTools :: Debugger, defect, P2)

Description

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

For HTML files with inline scripts, we currently load them from cache.  However, the toolbox default to disabling caching for the page.

When these concepts combine, we can end up loading stale content for HTML sources.

Comment 1

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: debugger-source-caching

I assume this should have a test...  Are there existing tests that toggle file caching like this?

Comment 2

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Invalid patch file?

Comment 3

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: debugger-source-caching

Let's try that again...

Comment 4

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Bug 1060732 might be related, we should re-test this after it lands.

Comment 5

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: Bug 1248498 - Check document cache status when loading HTML sources.

When getting the source for inline scripts in documents, we generally try to use
the cache.  However, the toolbox allows you to disable the HTTP cache while open
if you wish.

If the toolbox has disabled the cache, using `fetchFromCache: true` will give
stale data (from the most recent cache-enabled request).

We can avoid stale data here by checking the document's cache state.

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

Comment 6

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=458fd2c970979ee32b1eb1accd5e340dd9dcca8e

Comment 7

[Jason Laster [:jlast]]

Comment on attachment 8954998 [details]
Bug 1248498 - Check document cache status when loading HTML sources.

https://reviewboard.mozilla.org/r/224180/#review230224

Looks great!

Comment 8

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8954998 [details]
Bug 1248498 - Check document cache status when loading HTML sources.

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

Comment 9

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=e4e8f908acef6cee0728943837bc9b58b14c7b63

Comment 10

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=623b5647b1f0bdae513c21ed193a753dc5ff14f9

Comment 11

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8954998 [details]
Bug 1248498 - Check document cache status when loading HTML sources.

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

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f0a37e176064
Check document cache status when loading HTML sources. r=jlast

Comment 13

[Cristian Brindusan [:cbrindusan]]

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

Comment 14

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Erik, a fix for this is now included in Nightly 60.  Could you try it out[1] to check whether it resolves the issue you were seeing?

[1]: https://nightly.mozilla.org

Comment 15

[Erik Krause]

Seems to be fixed in nightly 60.0a1 (2018-03-06) (64-Bit) but not in developer edition 60.0b1 (64-Bit).

Comment 16

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Thanks!  I believe it should appear in Dev Ed in a few days.