Closed
Bug 1397331
Opened 7 years ago
Closed 7 years ago
window.URL.createObjectURL() no longer works in Firefox 57.0a1
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox55 | --- | unaffected |
firefox56 | --- | unaffected |
firefox57 | + | fixed |
People
(Reporter: dw-dev, Assigned: baku)
References
Details
(Keywords: regression, regressionwindow-wanted)
Attachments
(1 file, 1 obsolete file)
7.96 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:55.0) Gecko/20100101 Firefox/55.0 Build ID: 20170809080026 Steps to reproduce: I tested using my Save Page WE add-on with a large web page: https://www.reddit.com/r/IAmA/comments/66ajul/i_am_bill_nye_and_im_here_to_dare_i_say_it_save/ I tested with Chrome and with Firefox 55, 56 & latest 57. Actual results: Chrome: Save Page WE saved the page correctly. Firefox 55 & 56: Save Page WE saved the page correctly. Firefox 57: Firefox hangs when Save Page WE is trying to create a Blob URL: htmlBlob = new Blob( htmlStrings, { type: "text/html" }); objectURL = window.URL.createObjectURL(htmlBlob); As far as I can see, no exception was thrown. Up until a few days ago, the page was saved correctly and Firefox 57 did not hang. Expected results: Firefox 57: Save Page WE should have saved the page correctly and Firefox should not have hung.
Comment 1•7 years ago
|
||
Please use mozregression to find the regression range: https://mozilla.github.io/mozregression/
Updated•7 years ago
|
Flags: needinfo?(amarchesini)
I'm not familiar with the regression tool and don't really have the time to get up to speed. However, I have done a manual regression: Save Page WE works with: Nightly 2017-08-25 10-01-26 Save Page WE fails with: Nightly 2017-08-26 10-04-18
Comment 3•7 years ago
|
||
Regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=9136548f3de862cf471ead56053fbf7298c350ca&tochange=4970f731aeba44e98d91ae54915a80dca39a5716 Suspect: Bug 1393360
Blocks: 1393360
status-firefox55:
--- → unaffected
status-firefox56:
--- → unaffected
status-firefox57:
--- → affected
status-firefox-esr52:
--- → unaffected
Updated•7 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
This bug really needs to be fixed in Firefox 57. I have spent a considerable amount of time developing Save Page WE as a replacement for the Mozilla Archive Format and UnMHT add-ons, so that Firefox users still have the ability to save a complete web page as a single file. Users who are migrating to Save Page WE have the expectation that it will work with Firefox 57, especially since it already works with Firefox 55 & 56.
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → amarchesini
Flags: needinfo?(amarchesini)
Assignee | ||
Comment 6•7 years ago
|
||
Attachment #8905473 -
Flags: review?(bugs)
Comment 7•7 years ago
|
||
What kind of test would have caught the regression?
Comment 8•7 years ago
|
||
Comment on attachment 8905473 [details] [diff] [review] multiplex.patch I need some explanation here what causes the bug and how this fixes it before I can review. And a testcase would be really nice.
Attachment #8905473 -
Flags: review?(bugs)
Assignee | ||
Comment 9•7 years ago
|
||
Comment on attachment 8905473 [details] [diff] [review] multiplex.patch The issue here is that we QI each sub-stream each time a new one is added or removed to/from a nsMultiplexInputStream. Following the STR, firefox ends up creating a Blob with a nsMultiplexInputStream object containing more than 6000 substreams (probably because there are nested blobs). When that nsMultiplexInputStream is cloned, a new nsMultiplexInputStream is created and, for each sub-stream, a cloned version of them is appended. For each append, we do a QI of any other sub-stream. This is horrible. The approach I'm suggesting is to have a simple data struct containing what a stream can do (is it cloneable? serializable? and so on). When the conditional QI is executed, this data struct is used.
Attachment #8905473 -
Flags: review?(bugs)
Comment 10•7 years ago
|
||
Comment on attachment 8905473 [details] [diff] [review] multiplex.patch > Mutex mLock; // Protects access to all data members. >- nsTArray<nsCOMPtr<nsIInputStream>> mStreams; >+ >+ struct StreamData { { goes to its one line >+ nsCOMPtr<nsIInputStream> mStream; >+ bool mIsSeekable; >+ bool mIsIPCSerializable; >+ bool mIsCloneable; >+ bool mIsAsyncInputStream; >+ >+ void Popolate(nsIInputStream* aStream); Popolate? I guess you mean Populate > NS_INTERFACE_MAP_BEGIN(nsMultiplexInputStream) > NS_INTERFACE_MAP_ENTRY(nsIMultiplexInputStream) > NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsIInputStream, nsIMultiplexInputStream) >- NS_INTERFACE_MAP_ENTRY_CONDITIONAL(nsISeekableStream, mIsSeekable) >+ NS_INTERFACE_MAP_ENTRY_CONDITIONAL(nsISeekableStream, IsSeekable()) > NS_INTERFACE_MAP_ENTRY_CONDITIONAL(nsIIPCSerializableInputStream, >- mIsIPCSerializable) >+ IsIPCSerializable()) > NS_INTERFACE_MAP_ENTRY_CONDITIONAL(nsICloneableInputStream, >- mIsCloneable) >+ IsCloneable()) > NS_INTERFACE_MAP_ENTRY_CONDITIONAL(nsIAsyncInputStream, >- mIsAsyncInputStream) >+ IsAsyncInputStream()) Those Is* methods to iterate arrays, so can be a bit slow. Could we rather just have counters for each optional interface type?
Attachment #8905473 -
Flags: review?(bugs)
Assignee | ||
Comment 11•7 years ago
|
||
Attachment #8905473 -
Attachment is obsolete: true
Attachment #8905514 -
Flags: review?(bugs)
Comment 12•7 years ago
|
||
Comment on attachment 8905514 [details] [diff] [review] multiplex2.patch Please make NS_WARNING("ERROR: A nsIInputStream changed QI map when stored in a nsMultiplexInputStream!"); \ a MOZ_ASSERT The macro is a bit odd. I'd let that to do the QIing based on the params passed to it. I'd fix that.
Attachment #8905514 -
Flags: review?(bugs) → review+
Comment 13•7 years ago
|
||
Pushed by amarchesini@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/8338a5c1ef21 Reduce the number of QI done in nsMultiplexInputStream using counters, r=smaug
Comment 14•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/8338a5c1ef21
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Updated•7 years ago
|
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•