User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/53.0.2785.143 Safari/537.36 Steps to reproduce: simplified repro outside of our app: 1. Open https://appcacherepro.azurewebsites.net/demo.aspx in Firefox 2. This page will install appcache and cache itself 3. Hit F5 and note the page generation time 4. Open https://appcacherepro.azurewebsites.net/demo.aspx?authRedirect=true, it will redirect to https://appcacherepro.azurewebsites.net/demo.aspx 5. Step 4's final request https://appcacherepro.azurewebsites.net/demo.aspx does not load from appcache. The page generation time will not match to that of step 3. Actual results: Example from production: 1. Navigating: https://outlook.office.com/owa/?authRedirect=true => returns 302 Redirect to https://outlook.office.com/owa/#authRedirect=true 2. https://outlook.office.com/owa/#authRedirect=true should load from appcache as the url is cached but it skips appcache and a request is made to server. 3. If you hit F5 again with url https://outlook.office.com/owa/#authRedirect=true the document successfully loads from appcache.

[Tracking Requested - why for this release]: Important performance regression, we should investigate before 50 goes live.

Appcache is about to be removed, we only fix critical bugs or serious regressions. This is probably not a bug, seems like we purposely don't look into appcache for results of redirects: https://dxr.mozilla.org/mozilla-central/rev/c845bfd0accb7e0c29b41713255963b08006e701/netwerk/protocol/http/HttpBaseChannel.cpp#3125 // transfer application cache information nsCOMPtr<nsIApplicationCacheChannel> appCacheChannel = do_QueryInterface(newChannel); if (appCacheChannel) { appCacheChannel->SetApplicationCache(mApplicationCache); appCacheChannel->SetInheritApplicationCache(mInheritApplicationCache); > // We purposely avoid transfering mChooseApplicationCache. } mChooseApplicationCache == true makes us to look the appcache up for the URL being loaded. In the redirect target channel it tho remains false. Unfortunately, the one who wrote that comment didn't say "why"... If other browsers treat this differently, let's reopen this bug and carry the flag. The fix should be easy, but I'm not sure if we have tests for this and can't estimate any regressions this could cause, a path I don't want to go... automated tests are now disabled for appcache (e10s, afaik).

can you give details on why this is listed as a regression in comment 1? Does 48 perform differently or has the content changed? The code honza cites didn't change in the last several years.

It's an issue only with e10s. I tested with FF49/52 and e10s turned off, I can't reproduce the issue. Same with the old FF40, it's reproducible only with e10s enabled. As automated tests for appcache are disabled with e10s, this issue has been probably not caught.

> It's an issue only with e10s. Then this at some level is a regression--sounds like our e10s code is busted. I hate to do this to you Honza...

I agree Jason, something to fix. Will take a look.

So, the reason why this works in non-e10s and not in e10s is that mChooseApplicationCache member on nsHttpChannel is set on redirect by docshell. In e10s this happens on the child process. However, we don't carry the flag back to the parent. The fix is easy.

Straightforward. I won't push to try because there are no tests for this anyway.

Comment on attachment 8805087 [details] [diff] [review] v1 (carry the choose app cache flag after redirect back to parent) Review of attachment 8805087 [details] [diff] [review]: ----------------------------------------------------------------- Awesome--thanks Honza.

Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/d65a9494010e Carry ChooseApplicationCache flag from child after redirect callback is done. r=jduell

Comment on attachment 8805087 [details] [diff] [review] v1 (carry the choose app cache flag after redirect back to parent) Review of attachment 8805087 [details] [diff] [review]: ----------------------------------------------------------------- This is breaking Microsoft Office online, and it's a simple, risk-free fix (just schlepping a boolean from the child to the parent). Please uplift as far up as possible.

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

Comment on attachment 8805087 [details] [diff] [review] v1 (carry the choose app cache flag after redirect back to parent) Fixes a regression in MS Office online, Aurora51+, Beta50+

This is hitting conflicts on the uplift to beta. Could we get a rebased patch if this needs uplifting?

This is it, builds locally for me.

Comment on attachment 8806475 [details] [diff] [review] v1 for m-b comment 12