Closed Bug 1193911 Opened 6 years ago Closed 6 years ago

synthetic image response fails in non-e10s mode on https://flaki.github.io/bpg2jpg/test-sw.html

Categories

(Core :: DOM: Service Workers, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla43
Tracking Status
e10s - ---
firefox43 --- fixed

People

(Reporter: bkelly, Assigned: nsm)

References

Details

Attachments

(1 file)

Flaki has a test service worker page here:

  https://flaki.github.io/bpg2jpg/test-sw.html

It processes data in the service worker thread and then returns the result as a synthetic response.  The page loads this as an <img> tag.

In e10s this works fine.  In non-e10s I see the following:

[3412] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80040111: file c:/devel/mozilla-centr
al/netwerk/cache2/CacheEntry.cpp, line 1172
[3412] WARNING: failed to parse security-info: file c:/devel/mozilla-central/netwerk/protocol/http/n
sHttpChannel.cpp, line 3843
[3412] WARNING: NS_ENSURE_SUCCESS(rv, BadImage(newImage)) failed with result 0x80004005: file c:/dev
el/mozilla-central/image/ImageFactory.cpp, line 248

This seems very similar (although opposite e10s-vs-non-e10s pattern) to bug 1172992.
Do we know which particular branch of fetchBpgCacheJpg is returning the content that triggers this behaviour?
From the browser console messages, I'm fairly certain its this:

  https://github.com/flaki/bpg2jpg/blob/master/sw.js#L52
Assignee: nobody → nsm.nikhil
Bug 1193911 - Ensure synthetic Responses gets a valid channel info. r?ehsan

Right now, synthetic Responses did not have a valid channel info. When these
were saved in the Cache, and then restored, the restored Response did have
a ChannelInfo, but that ChannelInfo did not have a valid security info.
Passing this to respondWith() then caused the interception to fail.

This patch modifies Response::Constructor() to initialize its ChannelInfo from
the global. ChannelInfo can now initialize itself from a nsIDocument. All
workers now store their ChannelInfo on the WorkerLoadInfo.
Ehsan, I'm not sure if it is ok to set mRedirected = false in InitFromDocument.
Comment on attachment 8649115 [details]
MozReview Request: Bug 1193911 - Ensure synthetic Responses gets a valid channel info. r?ehsan

https://reviewboard.mozilla.org/r/16355/#review14671

::: dom/fetch/ChannelInfo.cpp:33
(Diff revision 1)
> +  mRedirected = false;

This is fine, but you need to explain why it's fine.  Please add a comment that explains the mRedirected flag (and also mRedirectedURISpec) are only important for maintaining the channel's redirected status.  If the ChannelInfo is initialized from a document, that document has already asked the channel from which it was loaded about the current channel URI, so it won't matter if a future ResurrectInfoOnChannel() call misses whether the channel was redirected.
Attachment #8649115 - Flags: review?(ehsan)
Comment on attachment 8649115 [details]
MozReview Request: Bug 1193911 - Ensure synthetic Responses gets a valid channel info. r?ehsan

Sorry, MozReview is stupid.  That was an r+.
Attachment #8649115 - Flags: review+
url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/db15edc4cc63f4d9a98f700a9498c52e63659aac
changeset:  db15edc4cc63f4d9a98f700a9498c52e63659aac
user:       Nikhil Marathe <nsm.nikhil@gmail.com>
date:       Mon Aug 17 15:08:58 2015 -0700
description:
Bug 1193911 - Ensure synthetic Responses gets a valid channel info. r=ehsan

Right now, synthetic Responses did not have a valid channel info. When these
were saved in the Cache, and then restored, the restored Response did have
a ChannelInfo, but that ChannelInfo did not have a valid security info.
Passing this to respondWith() then caused the interception to fail.

This patch modifies Response::Constructor() to initialize its ChannelInfo from
the global. ChannelInfo can now initialize itself from a nsIDocument. All
workers now store their ChannelInfo on the WorkerLoadInfo.
https://hg.mozilla.org/mozilla-central/rev/db15edc4cc63
Status: NEW → RESOLVED
Closed: 6 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in before you can comment on or make changes to this bug.