4.93 KB, patch
|Details | Diff | Splinter Review|
P2 Add a mochitest to verify the json viewer is shown for json intercepted by a service worker. r=jryans
4.72 KB, patch
|Details | Diff | Splinter Review|
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:57.0) Gecko/20100101 Firefox/57.0 Build ID: 20170915100121 Steps to reproduce: Tried to view a JSON response in a tab, it usually pretty prints the JSON but got raw text. Visit https://sarahtherad.com/wp-json/rad/home Everything is fine Visit https://sarahtherad.com/ Serviceworker is registered Visit https://sarahtherad.com/wp-json/rad/home again Actual results: It shows the JSON in raw text. If I unregister the serviceworker everything is fine. Expected results: Show JSON View even if a serviceworker is present.
2 years ago
Component: Untriaged → Developer Tools: JSON Viewer
This only happens with e10s enabled. Like bug 1397966, the problem is that getMIMETypeFromContent is not even called. So there is not much that can be done from the JSON Viewer point of view, the fix should happen deep in netwerk code.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Hi junior, This seems a serviceworker related bug, could you help find the right person to handle it?
nsDocShell::DoChannelLoad sets the loadflag LOAD_CALL_CONTENT_SNIFFERS but it is gone in the child process with InterceptedChannel Change the component first
Component: Developer Tools: JSON Viewer → DOM: Service Workers
Product: Firefox → Core
(In reply to Junior[:junior] from comment #3) > nsDocShell::DoChannelLoad sets the loadflag LOAD_CALL_CONTENT_SNIFFERS > but it is gone in the child process with InterceptedChannel > > Change the component first Mmm... not sure which component I should set. The load flag is set by docshell in content process, and the channel is intercepted. Hence, no IPC to parent process for "this" channel, neither the sniffer.
Can someone run mozregression to see where this broke?
I can reproduce the problem back in Nightly 45.0 2015-11-17. And if previous builds work that's probably because https://sarahtherad.com/ crashes and the serviceworker is not registered (I believe that's bug 1224941).
Hmm, comment 4 suggests FF56 worked as expected. Maybe this works in non-e10s mode but not e10s mode?
Yes, I already said this in comment 1.
Regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=7f1714325cb3e40427e40031a2191fe20cdc3241&tochange=6aa1cc819058deaa13f85278e2f5970b8ced0998 Regressed by: Bug 1290481 :tt, Your bunch of patch seems to cause the regression. Can you look into this?
OS: Unspecified → All
Could you try to run mozregression with a fresh profile? I guess you ran it with the same profile and it was broken due to downgrade. In theory, bug 1290481 changed the code related to DOM Cache and the QuotaManager. I'll remove the dependency for now because this issue is probably not related to bug 1290481. Please refer to comment Bug 1409115 Comment 12. The patch P2, P6 in Bug 1290481 did the major upgrade to the DOM Cache the QuotaManager. Thus, QuotaManager and IndexedDB and ServiceWorkers will be broken if you use the same profile to test with/without patches in Bug 1290481.
No longer blocks: 1290481
Flags: needinfo?(ttung) → needinfo?(alice0775)
(In reply to Tom Tung [:tt] from comment #11) > Could you try to run mozregression with a fresh profile? I guess you ran it > with the same profile and it was broken due to downgrade. Or you could try to upgrade to nightly with bug 1404344 [Note 1] and then switch to the build before bug 1290481 to see whether the issue still happens or not. Just like the suggestion in Bug 1409115 Comment 26. Note 1: you still need to go to the website to trigger the patches in bug 1404344.
I tried to find regression with clean profile each time. Regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=9136548f3de862cf471ead56053fbf7298c350ca&tochange=4970f731aeba44e98d91ae54915a80dca39a5716 Regressed by: 4970f731aeba Andrea Marchesini — Bug 1393360 - nsMultiplexInputStream should not QI each stream when checking if seekable, cloneable or IPC serializable, r=smaug 0d9988ed1c6e Andrea Marchesini — Bug 1392358 - Introduce XHR.sendInputStream(nsIInputStream) chrome-only, r=smaug 586a6514e046 Andrea Marchesini — Bug 1392358 - Remove XHR.send(nsIInputStream) and unify XHR.send(params) as the spec says, r=smaug :baku Your patch seems to cause the regression. Can you look into this?
Flags: needinfo?(alice0775) → needinfo?(amarchesini)
Alice, I'm sorry, but I probably should have removed the regressionwindow-wanted after comment 7. I can also reproduce this in FF56. It seems this has always been broken in e10s mode. I suspect mozregression was bouncing between e10s and non-e10s based on the experiment cohort stuff? Also, this now fails in FF58 non-e10s since I changed how we do intercepted there in bug 1391693. I think the solution here is that our intercept response pump code needs to perform this same code from nsHttpChannel: https://searchfox.org/mozilla-central/source/netwerk/protocol/http/nsHttpChannel.cpp#1610 I'll take this since I was just touching this code.
This patch fixes the problem for me locally in both e10s and non-e10s. Let me see if I can write a test.
While you are at it, maybe you can fix bug 1397966 (sniffers are not called when there is no content)?
(In reply to Oriol Brufau [:Oriol] from comment #16) > While you are at it, maybe you can fix bug 1397966 (sniffers are not called > when there is no content)? I'd rather leave that to a separate bug since it involves touching non-service-worker code. The issue there is that nsHttpChannel does not do: https://searchfox.org/mozilla-central/source/netwerk/base/nsBaseChannel.cpp#848 Its unclear to me how the necko team would like to fix that, though. There is a desire to make nsHttpChannel use nsBaseChannel, for example.
Jan, this test fails with current mozilla-central, but passes with the P1 patch. Its goal is to check that we properly trigger the jsonview even when the response is synthesized by a service worker.
Comment on attachment 8920603 [details] [diff] [review] P1 Respect LOAD_CALL_CONTENT_SNIFFERS when a channel is intercepted by a ServiceWorker. r=valentin Valentin, this patch allows us to correctly handle LOAD_CALL_CONTENT_SNIFFERS on intercepted channels. I moved some of the code in nsHttpChannel into HttpBaseChannel to avoid further code duplication.
Attachment #8920603 - Flags: review?(valentin.gosu)
Sorry, the previous patch was from the wrong tree. This one is correct.
Attachment #8920603 - Flags: review?(valentin.gosu) → review+
Comment on attachment 8920684 [details] [diff] [review] P2 Add a mochitest to verify the json viewer is shown for json intercepted by a service worker. r=honza Review of attachment 8920684 [details] [diff] [review]: ----------------------------------------------------------------- Thanks, this look good to me!
Attachment #8920684 - Flags: review?(odvarko) → review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/84cb594525ad P1 Respect LOAD_CALL_CONTENT_SNIFFERS when a channel is intercepted by a ServiceWorker. r=valentin https://hg.mozilla.org/integration/mozilla-inbound/rev/574b5b042c3c P2 Add a mochitest to verify the json viewer is shown for json intercepted by a service worker. r=jryans
Backed out for eslint failure in devtools/client/jsonview/test/browser_jsonview_serviceworker.js: https://hg.mozilla.org/integration/mozilla-inbound/rev/de6b2b70a3003daa8f82812568ec3e512a14c3d8 https://hg.mozilla.org/integration/mozilla-inbound/rev/beff4e58e183d639063b1334452add368f82d269 Push with failure: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=574b5b042c3c4964b1837fcbcb23557c29be51e9&filter-resultStatus=usercancel&filter-resultStatus=runnable&filter-resultStatus=retry&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=139638094&repo=mozilla-inbound TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/devtools/client/jsonview/test/browser_jsonview_serviceworker.js:12:24 | Missing space before function parentheses. (space-before-function-paren) ... TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/devtools/client/jsonview/test/passthrough-sw.js:1:1 | Use the global form of 'use strict'. (strict) TEST-UNEXPECTED-ERROR | /builds/worker/checkouts/gecko/devtools/client/jsonview/test/passthrough-sw.js:1:18 | Strings must use doublequote. (quotes)
Fix eslint issues. This passes eslint locally.
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/b6d464fd41c6 P1 Respect LOAD_CALL_CONTENT_SNIFFERS when a channel is intercepted by a ServiceWorker. r=valentin https://hg.mozilla.org/integration/mozilla-inbound/rev/39fb91da0ea7 P2 Add a mochitest to verify the json viewer is shown for json intercepted by a service worker. r=jryans
You need to log in before you can comment on or make changes to this bug.