Closed Bug 1413920 Opened 2 years ago Closed 2 years ago

nsMultiplexInputStream should call OnInputStreamReady on close

Categories

(Core :: DOM: Core & HTML, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: baku, Assigned: baku)

References

Details

Attachments

(1 file, 1 obsolete file)

+++ This bug was initially created as a clone of Bug #1413863 +++

We don't have valid tests for the asyncWait call. This bug is about implementing a test.
Attached patch multiplexTest.patch (obsolete) — Splinter Review
Note that if AsyncWait() is called without an EventTarget, nsMultiplexInputStream uses the main-thread.
Assignee: nobody → amarchesini
Attachment #8924546 - Flags: review?(bugs)
Comment on attachment 8924546 [details] [diff] [review]
multiplexTest.patch

This is changing some non-testing only code. Automatic r-.
Re-purpose this bug and change its title or something, but please no functionality changes in patches which are supposed to be about testing.

(bug 1404910 was weird enough)
Attachment #8924546 - Flags: review?(bugs) → review-
Summary: Test for nsMultiplexInputStream → nsMultiplexInputStream should call OnInputStreamReady on close
Attachment #8924546 - Flags: review- → review?(bugs)
Comment on attachment 8924546 [details] [diff] [review]
multiplexTest.patch

>+// AsyncWait - sync - closureOnly
>+TEST(TestMultiplexInputStream, AsyncWait_sync_closureOnly) {
>+  nsCOMPtr<nsIInputStream> is = CreateStreamHelper();
>+
>+  nsCOMPtr<nsIAsyncInputStream> ais = do_QueryInterface(is);
>+  ASSERT_TRUE(!!ais);
>+
>+  RefPtr<testing::InputStreamCallback> cb =
>+    new testing::InputStreamCallback();
>+
>+  ASSERT_EQ(NS_OK, ais->AsyncWait(cb, nsIAsyncInputStream::WAIT_CLOSURE_ONLY,
>+                                  0, nullptr));
>+  ASSERT_FALSE(cb->Called());
>+
>+  ais->CloseWithStatus(NS_ERROR_FAILURE);
>+  ASSERT_FALSE(cb->Called());
>+
>+  // Eventually it is called.
>+  MOZ_ALWAYS_TRUE(mozilla::SpinEventLoopUntil([&]() { return cb->Called(); }));
>+  ASSERT_TRUE(cb->Called());
>+}
>+
>+// AsyncWait - async
>+TEST(TestMultiplexInputStream, AsyncWait_async_closureOnly) {
>+  nsCOMPtr<nsIInputStream> is = CreateStreamHelper();
>+
>+  nsCOMPtr<nsIAsyncInputStream> ais = do_QueryInterface(is);
>+  ASSERT_TRUE(!!ais);
>+
>+  RefPtr<testing::InputStreamCallback> cb =
>+    new testing::InputStreamCallback();
>+  nsCOMPtr<nsIThread> thread = do_GetCurrentThread();
>+
>+  ASSERT_EQ(NS_OK, ais->AsyncWait(cb, nsIAsyncInputStream::WAIT_CLOSURE_ONLY,
>+                                  0, thread));
>+
>+  ASSERT_FALSE(cb->Called());
>+  ais->CloseWithStatus(NS_ERROR_FAILURE);
>+  ASSERT_FALSE(cb->Called());
>+
>+  // Eventually it is called.
>+  MOZ_ALWAYS_TRUE(mozilla::SpinEventLoopUntil([&]() { return cb->Called(); }));
>+  ASSERT_TRUE(cb->Called());
>+}
I don't undersatnd the difference here. What is sync in the sync case (the first case).
The only difference is whether thread is passed, but what is then sync and what is async.
If nothing else, add some comment what these are testing. It isn't clear now.
Attachment #8924546 - Flags: review?(bugs) → review-
Attachment #8924546 - Attachment is obsolete: true
Attachment #8925430 - Flags: review?(bugs)
Attachment #8925430 - Flags: review?(bugs) → review+
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7c79d729d27e
nsMultiplexInputStream should call OnInputStreamReady on close, r=smaug
https://hg.mozilla.org/mozilla-central/rev/7c79d729d27e
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.