Closed Bug 1636823 Opened 4 years ago Closed 4 years ago

Mixed BLOB objects can't be downloaded correctly

Categories

(Core :: DOM: File, defect)

Product:
Core
Component:
DOM: File
Version:
76 Branch
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla79
Tracking Flags:
Tracking Status
firefox-esr68 --- unaffected
firefox-esr78 --- fixed
firefox76 --- wontfix
firefox77 --- wontfix
firefox78 --- wontfix
firefox79 --- verified
firefox80 --- verified

People

(Reporter: iperione.xix, Assigned: baku)

References

(Regression)

Details

(Keywords: regression)

Attachments

(4 files, 5 obsolete files)

Bug 1636823 - nsInputTransport must support blocking and async source - part 1 - Use NS_MakeAsyncNonBlockingInputStream, r?smaug
47 bytes, text/x-phabricator-request
RyanVM
: approval-mozilla-esr78+
Details | Review
Bug 1636823 - nsInputTransport must support blocking and async source - part 2 - implementation, r?smaug
47 bytes, text/x-phabricator-request
RyanVM
: approval-mozilla-esr78+
Details | Review
Bug 1636823 - nsInputTransport must support blocking and async source - part 3 - move gtests, r?smaug
47 bytes, text/x-phabricator-request
RyanVM
: approval-mozilla-esr78+
Details | Review
Bug 1636823 - nsInputTransport must support blocking and async source - part 4 - gtest for inputStreamTransport, r?smaug
47 bytes, text/x-phabricator-request
RyanVM
: approval-mozilla-esr78+
Details | Review

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:76.0) Gecko/20100101 Firefox/76.0

Steps to reproduce:

Tested on FireFox 76.0.1 (Windows 10). Trying to download a BLOB object, built with mixed sources, using its URL obtained via createObjectURL() and retrieved via on-the-fly link technique:

  var link = document.createElement ('a');
  link.download = a_filename;
  link.href = blob_fileurl;
  document.body.appendChild (link);
  link.click ();

Actual results:

When a BLOB object in javascript is created "mixed", specifically composed by Uint8Array and File objects (ex. new Blob ([array1, file, array2]);), then it can't be downloaded.

Expected results:

A normal download.

Here is a little example, https://jsfiddle.net/3r2yfj1L/

Just select any file and then try to click FULL button: it doesn't work. Works fine when BLOB is not mixed.

Because this bug's Severity is normal and has not been changed, and this bug's priority is -- (none,) indicating it has has not been previously triaged, the bug's Severity is being updated to -- (default, untriaged.)

Severity: normal → --

Using comment 0 testcase, tested on Windows 10:

[Affected:]

76.0.1 20200507114007
77.0b3 20200507233245
78.0a1 20200512212513

[Not affected:]

68.8.0esr 20200429190206

Regression range hints toward
Bug 1472158 - Broadcast BlobURLs only to processes with the same loaded origins, r=farre,asuth

Given Bug 1472158, I'm uncertain if this is a regression or intended behaviour. :baku, could you please take a look?

Blocks: 1472158
Severity: -- → S3
status-firefox76: --- → affected
status-firefox77: --- → affected
status-firefox78: --- → affected
status-firefox-esr68: --- → unaffected
Component: Untriaged → DOM: File
Flags: needinfo?(amarchesini)
Product: Firefox → Core
Status: UNCONFIRMED → NEW
Ever confirmed: true

Subhamoy, do you want to take a look?

Flags: needinfo?(amarchesini) → needinfo?(ssengupta)

I debugged this issue a bit more on nightly. It could be that the previous firefox versions were buggy because of the blobURL broadcasting, but in nightly, the issue is related to DocumentChannel. Disabling document channel, the downloading of "mixed" blobURLs works perfectly.

I haven't tested it too deeply but it seems that the DocumentChannel actor is not correctly propagated. I suspect the issue is related to how the nsIInputStream (async) is treated by DocumentChannel. Matt, do you mind to take a look?

Flags: needinfo?(matt.woodrow)

It looks we're trying to create a channel in the parent process using the blob url, but the underlying data isn't fully available.

Looking at the blob serialization code here: https://searchfox.org/mozilla-central/rev/df4c90d4b8c92c99f76334acfe4813c573c12661/dom/file/ipc/IPCBlobUtils.cpp#168

It seems like we only serialize as an IPCFile if the blob is of type file, and for all others (including MultipartBlobImpl) we just serialize as an input stream.

Then the parent side trying to read from the blob fails with NS_BASE_STREAM_WOULD_BLOCK.

Do we need to handle serializing Multipart blobs using IPCFile for the file components for them to be available in the parent?

Flags: needinfo?(matt.woodrow) → needinfo?(amarchesini)

It seems like we only serialize as an IPCFile if the blob is of type file, and for all others (including MultipartBlobImpl) we just serialize as an input stream.

Right. This is correct. There is no "DOM File" involved here. A DOM File, by spec, is a blob with a name. The test does:

const full_blob = new Blob ([head, body, foot], { type: 'application/octet-stream' });

So full_blob is not a DOM File even if it contains a File. In order to have a DOM File the code should do: new File(...), but this is not relevant for the bug.
The serialization part is correct too and we end up sending a MultipartBlobImpl to the parent, because full_blob is a multipart blob composed by head + body + foot. I suspect that end up creating something like String + IPCBlobInputStream + String, or something similar.

Then the parent side trying to read from the blob fails with NS_BASE_STREAM_WOULD_BLOCK.

This is correct too because the MultipartBlobImpl has an async nsIInputStream. We should use nsIAsyncInputStream::AsyncWait().
Intenally, we create a pipe from content to parent process. The data is read step by step when needed.

Do we need to handle serializing Multipart blobs using IPCFile for the file components for them to be available in the parent?

No :) We just need to read the inputStream properly calling AsyncWait(). If we consider an error NS_BASE_STREAM_WOULD_BLOCK, the code is buggy. Can you tell me where this happen? Note that, if we want to have the size of the stream, we should not use Available() but the nsIInputStreamLength interface. Or, we can simply use the size of the blob from here: https://searchfox.org/mozilla-central/rev/df4c90d4b8c92c99f76334acfe4813c573c12661/dom/file/ipc/IPCBlobUtils.cpp#180

Flags: needinfo?(amarchesini) → needinfo?(matt.woodrow)

This is the callstack to NS_BASE_STREAM_WOULD_BLOCK being returned: https://paste.mozilla.org/F0Q16aug

It looks like we're trying to do async copy of the multiplex input stream (created from the multipart blob), and it's failing trying to copy the first segment ('head'), because the pipe is empty.

There are a crazy number of layers of abstraction here, so it's hard to understand what it's trying to do.

The outcome is that we create a channel using the BlobURL synchronized to the parent, and then it calls OnStartRequest with mStatus=NS_BASE_STREAM_WOULD_BLOCK.

Possibly some mix-up with blocking and non-blocking components combined within a single multiplex input stream?

Flags: needinfo?(matt.woodrow) → needinfo?(amarchesini)

nsMultiplexInputStream::IsNonBlocking is returning false (meaning we are a blocking stream), because we have 3 sub-streams, two of which are non-blocking, but one is blocking.

We're then running the copy on a background thread to do the blocking read.

nsMultiplexInputStream::Read calls into Read on the first sub-stream (which is non-blocking), and returns NS_BASE_STREAM_WOULD_BLOCK.

The copy code considers that an error, since it expected a blocking stream, and NS_BASE_STREAM_WOULD_BLOCK should never be returned.

It seems like we need nsMultiplexInputStream to handle the case where we have mixed blocking/non-blocking sub-streams, and present a consistent interface.

It's not obvious how to implement a blocking version of Read that waits on non-blocking sub-streams (unless we can spin the event loop, and wait on AsyncWait on another background thread).

Maybe nsMultiplexInputStream needs to report non-blocking if any of the substreams are non-blocking, and needs to manually handle proxying Read to a background thread (invoking NS_AsyncCopy?) for the blocking substreams.

Assignee: nobody → amarchesini
Status: NEW → ASSIGNED

This definitely needs automated tests to catch regressions.

Flags: needinfo?(amarchesini)
Attachment #9151851 - Attachment description: Bug 1636823 - nsMultiplexInputStream should not be blocking and nsIAsyncInputStream at the same time, r?smaug → Bug 1636823 - nsMultiplexInputStream should not be blocking and nsIAsyncInputStream at the same time - part 1 - implementation, r?smaug
Attachment #9151852 - Attachment description: Bug 1636823 - nsMultiplexInputStream doesn't need to use nsITellable interface for substreams, r?smaug → Bug 1636823 - nsMultiplexInputStream should not be blocking and nsIAsyncInputStream at the same time - part 2 - remove the use of nsITellable interface for substreams, r?smaug
Attachment #9151989 - Attachment description: Bug 1636823 - nsMultiplexInputStream with sync and async blocking and non-blocking streams - gtests, r?smaug → Bug 1636823 - nsMultiplexInputStream should not be blocking and nsIAsyncInputStream at the same time - part 3 - tests, r?smaug
Attachment #9152140 - Attachment description: Bug 1636823 - nsMultiplexInputStream should support Seek(END) correctly, r?smaug → Bug 1636823 - nsMultiplexInputStream should not be blocking and nsIAsyncInputStream at the same time - part 4 - support Seek(END) correctly, r?smaug
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3019a2171aed
nsMultiplexInputStream should not be blocking and nsIAsyncInputStream at the same time - part 1 - implementation, r=smaug
https://hg.mozilla.org/integration/autoland/rev/a388fd40baeb
nsMultiplexInputStream should not be blocking and nsIAsyncInputStream at the same time - part 2 - remove the use of nsITellable interface for substreams, r=smaug
https://hg.mozilla.org/integration/autoland/rev/2329c5e53493
nsMultiplexInputStream should not be blocking and nsIAsyncInputStream at the same time - part 3 - tests, r=smaug
https://hg.mozilla.org/integration/autoland/rev/52322ad2515e
nsMultiplexInputStream should not be blocking and nsIAsyncInputStream at the same time - part 4 - support Seek(END) correctly, r=smaug
Flags: needinfo?(amarchesini)
Attachment #9153113 - Attachment is obsolete: true
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5d46b483c59c
nsMultiplexInputStream should not be blocking and nsIAsyncInputStream at the same time - part 1 - implementation, r=smaug
https://hg.mozilla.org/integration/autoland/rev/930874501ab3
nsMultiplexInputStream should not be blocking and nsIAsyncInputStream at the same time - part 2 - remove the use of nsITellable interface for substreams, r=smaug
https://hg.mozilla.org/integration/autoland/rev/25455bb698d8
nsMultiplexInputStream should not be blocking and nsIAsyncInputStream at the same time - part 3 - tests, r=smaug
https://hg.mozilla.org/integration/autoland/rev/ebb1525ea063
nsMultiplexInputStream should not be blocking and nsIAsyncInputStream at the same time - part 4 - support Seek(END) correctly, r=smaug
Flags: needinfo?(amarchesini)

Comment on attachment 9151851 [details]
Bug 1636823 - nsMultiplexInputStream should not be blocking and nsIAsyncInputStream at the same time - part 1 - implementation, r?smaug

Revision D76901 was moved to bug 1643156. Setting attachment 9151851 [details] to obsolete.

Attachment #9151851 - Attachment is obsolete: true

Comment on attachment 9151852 [details]
Bug 1636823 - nsMultiplexInputStream should not be blocking and nsIAsyncInputStream at the same time - part 2 - remove the use of nsITellable interface for substreams, r?smaug

Revision D76902 was moved to bug 1643156. Setting attachment 9151852 [details] to obsolete.

Attachment #9151852 - Attachment is obsolete: true

Comment on attachment 9151989 [details]
Bug 1636823 - nsMultiplexInputStream should not be blocking and nsIAsyncInputStream at the same time - part 3 - tests, r?smaug

Revision D76998 was moved to bug 1643156. Setting attachment 9151989 [details] to obsolete.

Attachment #9151989 - Attachment is obsolete: true

Comment on attachment 9152140 [details]
Bug 1636823 - nsMultiplexInputStream should not be blocking and nsIAsyncInputStream at the same time - part 4 - support Seek(END) correctly, r?smaug

Revision D77100 was moved to bug 1643156. Setting attachment 9152140 [details] to obsolete.

Attachment #9152140 - Attachment is obsolete: true
Flags: needinfo?(ssengupta)
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/43aa94cc6640
nsInputTransport must support blocking and async source - part 1 - Use NS_MakeAsyncNonBlockingInputStream, r=smaug,necko-reviewers,mayhemer
https://hg.mozilla.org/integration/autoland/rev/2f5c418cb7a9
nsInputTransport must support blocking and async source - part 2 - implementation, r=smaug,necko-reviewers,mayhemer
https://hg.mozilla.org/integration/autoland/rev/007bbe713359
nsInputTransport must support blocking and async source - part 3 - move gtests, r=smaug,necko-reviewers,mayhemer
https://hg.mozilla.org/integration/autoland/rev/1de589017a6b
nsInputTransport must support blocking and async source - part 4 - gtest for inputStreamTransport, r=smaug,necko-reviewers,mayhemer

Backed out for build bustages.

backout: https://hg.mozilla.org/integration/autoland/rev/3ff4697b6bdae870bc93dd4c4306b9dd57ec80b3

push: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedTaskRun=UGWZyVlLR2ifFfhGJtGi6w.0&revision=1de589017a6b1235e436eaa88a9c71a95378ba13&group_state=expanded

failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=306127703&repo=autoland&lineNumber=20238

[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - In file included from Unified_cpp_netwerk_test_gtest1.cpp:2:
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - /builds/worker/checkouts/gecko/netwerk/test/gtest/TestReadStreamToString.cpp(13,20): error: use of undeclared identifier 'NS_NewCStringInputStream'; did you mean 'NS_NewLocalFileInputStream'?
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - ASSERT_EQ(NS_OK, NS_NewCStringInputStream(getter_AddRefs(stream), buffer));
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - ^~~~~~~~~~~~~~~~~~~~~~~~
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - NS_NewLocalFileInputStream
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - /builds/worker/workspace/obj-build/dist/include/gtest/gtest.h(2001,54): note: expanded from macro 'ASSERT_EQ'
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - # define ASSERT_EQ(val1, val2) GTEST_ASSERT_EQ(val1, val2)
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - ^
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - /builds/worker/workspace/obj-build/dist/include/gtest/gtest.h(1985,29): note: expanded from macro 'GTEST_ASSERT_EQ'
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - val1, val2)
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - ^
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - /builds/worker/workspace/obj-build/dist/include/gtest/gtest_pred_impl.h(165,40): note: expanded from macro 'ASSERT_PRED_FORMAT2'
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - GTEST_PRED_FORMAT2_(pred_format, v1, v2, GTEST_FATAL_FAILURE_)
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - ^
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - /builds/worker/workspace/obj-build/dist/include/gtest/gtest_pred_impl.h(146,43): note: expanded from macro 'GTEST_PRED_FORMAT2_'
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - GTEST_ASSERT_(pred_format(#v1, #v2, v1, v2),
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - ^
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - /builds/worker/workspace/obj-build/dist/include/gtest/gtest_pred_impl.h(76,52): note: expanded from macro 'GTEST_ASSERT_'
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - if (const ::testing::AssertionResult gtest_ar = (expression))
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - ^
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - /builds/worker/checkouts/gecko/netwerk/base/nsNetUtil.h(462,10): note: 'NS_NewLocalFileInputStream' declared here
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - nsresult NS_NewLocalFileInputStream(nsIInputStream** result, nsIFile* file,
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - ^
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - In file included from Unified_cpp_netwerk_test_gtest1.cpp:2:
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - /builds/worker/checkouts/gecko/netwerk/test/gtest/TestReadStreamToString.cpp(13,69): error: no viable conversion from 'nsCString' (aka 'nsTString<char>') to 'nsIFile '
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - ASSERT_EQ(NS_OK, NS_NewCStringInputStream(getter_AddRefs(stream), buffer));
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - ^~~~~~
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - /builds/worker/workspace/obj-build/dist/include/gtest/gtest.h(2001,54): note: expanded from macro 'ASSERT_EQ'
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - # define ASSERT_EQ(val1, val2) GTEST_ASSERT_EQ(val1, val2)
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - ^~~~
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - /builds/worker/workspace/obj-build/dist/include/gtest/gtest.h(1985,29): note: expanded from macro 'GTEST_ASSERT_EQ'
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - val1, val2)
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - ^~~~
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - /builds/worker/workspace/obj-build/dist/include/gtest/gtest_pred_impl.h(165,40): note: expanded from macro 'ASSERT_PRED_FORMAT2'
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - GTEST_PRED_FORMAT2_(pred_format, v1, v2, GTEST_FATAL_FAILURE_)
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - ^~
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - /builds/worker/workspace/obj-build/dist/include/gtest/gtest_pred_impl.h(146,43): note: expanded from macro 'GTEST_PRED_FORMAT2_'
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - GTEST_ASSERT_(pred_format(#v1, #v2, v1, v2),
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - ^~
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - /builds/worker/workspace/obj-build/dist/include/gtest/gtest_pred_impl.h(76,52): note: expanded from macro 'GTEST_ASSERT_'
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - if (const ::testing::AssertionResult gtest_ar = (expression))
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - ^~~~~~~~~~
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - /builds/worker/workspace/obj-build/dist/include/nsTSubstring.h(977,3): note: candidate template ignored: could not match 'mozilla::Span<unsigned char, 4294967295>' against 'nsIFile '
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - operator mozilla::Span<uint8_t>() {
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - ^
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - /builds/worker/workspace/obj-build/dist/include/nsTSubstring.h(983,3): note: candidate template ignored: could not match 'mozilla::Span<const unsigned char, 4294967295>' against 'nsIFile '
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - operator mozilla::Span<const uint8_t>() const {
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - ^
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - /builds/worker/workspace/obj-build/dist/include/nsTSubstring.h(940,3): note: candidate function
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - operator mozilla::Span<char_type>() {
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - ^
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - /builds/worker/workspace/obj-build/dist/include/nsTSubstring.h(944,3): note: candidate function
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - operator mozilla::Span<const char_type>() const {
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - ^
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - /builds/worker/checkouts/gecko/netwerk/base/nsNetUtil.h(462,71): note: passing argument to parameter 'file' here
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - nsresult NS_NewLocalFileInputStream(nsIInputStream result, nsIFile file,
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - ^
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - In file included from Unified_cpp_netwerk_test_gtest1.cpp:2:
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - /builds/worker/checkouts/gecko/netwerk/test/gtest/TestReadStreamToString.cpp(36,20): error: use of undeclared identifier 'NS_NewCStringInputStream'; did you mean 'NS_NewLocalFileInputStream'?
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - ASSERT_EQ(NS_OK, NS_NewCStringInputStream(getter_AddRefs(stream), buffer));
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - ^~~~~~~~~~~~~~~~~~~~~~~~
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - NS_NewLocalFileInputStream
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - /builds/worker/workspace/obj-build/dist/include/gtest/gtest.h(2001,54): note: expanded from macro 'ASSERT_EQ'
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - # define ASSERT_EQ(val1, val2) GTEST_ASSERT_EQ(val1, val2)
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - ^
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - /builds/worker/workspace/obj-build/dist/include/gtest/gtest.h(1985,29): note: expanded from macro 'GTEST_ASSERT_EQ'
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - val1, val2)
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - ^
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - /builds/worker/workspace/obj-build/dist/include/gtest/gtest_pred_impl.h(165,40): note: expanded from macro 'ASSERT_PRED_FORMAT2'
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - GTEST_PRED_FORMAT2_(pred_format, v1, v2, GTEST_FATAL_FAILURE_)
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - ^
[task 2020-06-12T15:39:08.156Z] 15:39:08 INFO - /builds/worker/workspace/obj-build/dist/include/gtest/gtest_pred_impl.h(146,43): note: expanded from macro 'GTEST_PRED_FORMAT2_'
[task 2020-06-12T15:39:08.157Z] 15:39:08 INFO - GTEST_ASSERT_(pred_format(#v1, #v2, v1, v2),
[task 2020-06-12T15:39:08.157Z] 15:39:08 INFO - ^
[task 2020-06-12T15:39:08.157Z] 15:39:08 INFO - /builds/worker/workspace/obj-build/dist/include/gtest/gtest_pred_impl.h(76,52): note: expanded from macro 'GTEST_ASSERT_'
[task 2020-06-12T15:39:08.157Z] 15:39:08 INFO - if (const ::testing::AssertionResult gtest_ar = (expression))
[task 2020-06-12T15:39:08.157Z] 15:39:08 INFO - ^
[task 2020-06-12T15:39:08.157Z] 15:39:08 INFO - /builds/worker/checkouts/gecko/netwerk/base/nsNetUtil.h(462,10): note: 'NS_NewLocalFileInputStream' declared here
[task 2020-06-12T15:39:08.157Z] 15:39:08 INFO - nsresult NS_NewLocalFileInputStream(nsIInputStream** result, nsIFile* file,
[task 2020-06-12T15:39:08.157Z] 15:39:08 INFO - ^
[task 2020-06-12T15:39:08.157Z] 15:39:08 INFO - In file included from Unified_cpp_netwerk_test_gtest1.cpp:2:
[task 2020-06-12T15:39:08.157Z] 15:39:08 INFO - /builds/worker/checkouts/gecko/netwerk/test/gtest/TestReadStreamToString.cpp(36,69): error: no viable conversion from 'nsCString' (aka 'nsTString<char>') to 'nsIFile *'
[task 2020-06-12T15:39:08.157Z] 15:39:08 INFO - ASSERT_EQ(NS_OK, NS_NewCStringInputStream(getter_AddRefs(stream), buffer));
[task 2020-06-12T15:39:08.157Z] 15:39:08 INFO - ^~~~~~
[task 2020-06-12T15:39:08.157Z] 15:39:08 INFO - /builds/worker/workspace/obj-build/dist/include/gtest/gtest.h(2001,54): note: expanded from macro 'ASSERT_EQ'

Flags: needinfo?(amarchesini)
Flags: needinfo?(amarchesini)
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7d220fd9ac49
nsInputTransport must support blocking and async source - part 1 - Use NS_MakeAsyncNonBlockingInputStream, r=smaug,necko-reviewers,mayhemer
https://hg.mozilla.org/integration/autoland/rev/9cdc2e31ee04
nsInputTransport must support blocking and async source - part 2 - implementation, r=smaug,necko-reviewers,mayhemer
https://hg.mozilla.org/integration/autoland/rev/50dd67f96214
nsInputTransport must support blocking and async source - part 3 - move gtests, r=smaug,necko-reviewers,mayhemer
https://hg.mozilla.org/integration/autoland/rev/13ab343d2c45
nsInputTransport must support blocking and async source - part 4 - gtest for inputStreamTransport, r=smaug,necko-reviewers,mayhemer
Flags: needinfo?(amarchesini)
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/51d94f5560cb
nsInputTransport must support blocking and async source - part 1 - Use NS_MakeAsyncNonBlockingInputStream, r=smaug,necko-reviewers,mayhemer
https://hg.mozilla.org/integration/autoland/rev/36cb91ca77a0
nsInputTransport must support blocking and async source - part 2 - implementation, r=smaug,necko-reviewers,mayhemer
https://hg.mozilla.org/integration/autoland/rev/7b9663a7bdf8
nsInputTransport must support blocking and async source - part 3 - move gtests, r=smaug,necko-reviewers,mayhemer
https://hg.mozilla.org/integration/autoland/rev/fe147898a052
nsInputTransport must support blocking and async source - part 4 - gtest for inputStreamTransport, r=smaug,necko-reviewers,mayhemer

The patch landed in nightly and beta is affected.
:baku, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Flags: needinfo?(amarchesini)

Comment on attachment 9154112 [details]
Bug 1636823 - nsInputTransport must support blocking and async source - part 1 - Use NS_MakeAsyncNonBlockingInputStream, r?smaug

Beta/Release Uplift Approval Request

  • User impact if declined: Some DOM Files cannot be downloaded.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Medium
  • Why is the change risky/not risky? (and alternatives if risky): This set of patches allows the reading of data from a pipe of a blocking nsIAsyncInputStream.
    It touches one of the core functionality of the network communication, and to ensure there are no regression, I added several tests. Because of this, medium risk.
  • String changes made/needed: none
Flags: needinfo?(amarchesini)
Attachment #9154112 - Flags: approval-mozilla-beta?
Attachment #9154113 - Flags: approval-mozilla-beta?
Attachment #9154114 - Flags: approval-mozilla-beta?
Attachment #9154115 - Flags: approval-mozilla-beta?

We've shipped with this bug already, and it's late in beta78, so I think I'd prefer to let this ride to 79. We can consider uplift to 78esr at some point though.

No longer blocks: 1472158
status-firefox78: affected → wontfix
status-firefox-esr78: --- → affected
Regressed by: 1472158
Has Regression Range: --- → yes
Attachment #9154112 - Flags: approval-mozilla-beta? → approval-mozilla-esr78?
Attachment #9154113 - Flags: approval-mozilla-beta? → approval-mozilla-esr78?
Attachment #9154114 - Flags: approval-mozilla-beta? → approval-mozilla-esr78?
Attachment #9154115 - Flags: approval-mozilla-beta? → approval-mozilla-esr78?
Flags: qe-verify+

I managed to reproduce the issue using an older version of Nightly (2020-05-08). I retested everything on macOS 10.13, Windows 10 x64 and Ubuntu 18.04x64 using the latest Nightly 80.a1 and Firefox 79.0b3. The issue is not reproducing anymore.

Status: RESOLVED → VERIFIED
status-firefox79: fixed → verified
status-firefox80: --- → verified
Flags: qe-verify+

Comment on attachment 9154112 [details]
Bug 1636823 - nsInputTransport must support blocking and async source - part 1 - Use NS_MakeAsyncNonBlockingInputStream, r?smaug

Seems like a pretty important compatibility bug. Thanks for including new test coverage and QA verification on Nightly and Beta. Approved for 78.1esr.

Attachment #9154112 - Flags: approval-mozilla-esr78? → approval-mozilla-esr78+
Attachment #9154113 - Flags: approval-mozilla-esr78? → approval-mozilla-esr78+
Attachment #9154114 - Flags: approval-mozilla-esr78? → approval-mozilla-esr78+
Attachment #9154115 - Flags: approval-mozilla-esr78? → approval-mozilla-esr78+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: