Closed Bug 1118332 Opened 5 years ago Closed 4 years ago
Only load the HTML page from browser cache in the debugger
This bug is a continuation from https://bugzilla.mozilla.org/show_bug.cgi?id=905700#c57
> Since we load the source from the Debugger.Source.prototype.text property now, > I can't even remember when we get to this point and we *can* load from cache. We do not load from D.S.p.t for an HTML page and the source actor will not have `this.source`, but the cache should have the HTML page (and the most up to date version with respect to the page we are debugging) because the browser _just_ requested it in order to display it / run JS / etc. We cannot rely on the cache for source mapped sources because the browser will generally never request the original source files unless the debugger is open. Even if it was in the cache, it is from an old debugging session (and therefore can't be trusted to be up to date), because the browser doesn't request these files automatically, unlike non source mapped sources, or HTML pages, etc.
Renaming because I figured out what we need to do
Summary: Investigate loading source from cache behavior → Only load the HTML page from browser cache in the debugger
The old code always felt a little wrong to me, and I think it was. I think I made it so that getting the original contents of a sourcemapped source *was* loading from cache again, which is bad. It was `loadFromCache: !this.source` which means for sourcemapped sources (which don't have `source`) it would load from cache. We made it this was in the first place because the HTML page did not have a source, so this told it to load the HTML page from the cache. I made it a lot more explicit and easy to reason about. Sources are now explicitly marked as an inline source (basically, the SourceActor represents the HTML page) and we only load from cache is it is an inline source.
Attachment #8598213 - Flags: review?(nfitzgerald)
Comment on attachment 8598213 [details] [diff] [review] 1118332.patch Review of attachment 8598213 [details] [diff] [review]: ----------------------------------------------------------------- Great change, thanks! ::: toolkit/devtools/server/actors/script.js @@ +2433,5 @@ > + // there are inline sources). Otherwise, we can't trust the > + // cache because we are most likely here because we are > + // fetching the original text for sourcemapped code, and the > + // page hasn't requested it before (if it has, it was a > + // previous debugging session) Super nit: period
Attachment #8598213 - Flags: review?(nfitzgerald) → review+
Attachment #8598213 - Attachment is obsolete: true
asan build seem broken, saw that in other bugs too
Whiteboard: [devedition-40] → [devedition-40][fixed-in-fx-team]
Please file a new bug!
Whiteboard: [devedition-40] → [polish-backlog]
You need to log in before you can comment on or make changes to this bug.