Brendan Dahl [:bdahl] Reporter
	Description • 9 years ago

STR:
1) Load https://people.mozilla.org/~bdahl/sandbox/service_worker_cache_bug/
2) Observe the message on the page "okay now do a regular reload"
3) Reload the page (not force/shift reload)

Result:
After reload the message becomes "waiting for xhr" and never does anything else.

Expected:
After reload the message becomes
"waiting for xhr
xhr progress
xhr load
success"

Notes from conversation with nsm:
- The test works if I comment out sw.js line "cache.put(event.request, responseToCache);"
- The test also works if I wait to return the response until after the cache.put e.g. |return caches.open('...').then(() => { return caches.put(...).then(() { return response; }|

	Ben Kelly [:bkelly, not reviewing] Assignee
	Comment 1 • 9 years ago

It could be bug 1134372.  Can you try with a resource under 64kb in size?

	Ben Kelly [:bkelly, not reviewing] Assignee
	Comment 2 • 9 years ago

It also works in e10s mode.  Interesting.

It could still be bug 1134372, but I'm not sure why the two different halves of the clone would be reading at different rates.

	Ben Kelly [:bkelly, not reviewing] Assignee
	Comment 3 • 9 years ago

Hmm, its unlikely to be bug 1134372.  I opened this in a local build and looked at the files in the profile.  The full pdf was downloaded AFAICT.

	Ben Kelly [:bkelly, not reviewing] Assignee
	Comment 4 • 9 years ago

And it does not work in e10s.  It just works if it was previously run, stopped, and then re-run since the Cache file is already there.

	Ben Kelly [:bkelly, not reviewing] Assignee
	Comment 5 • 9 years ago

I think this is a pipe bug:

  [13388] ###!!! ASSERTION: already have an input event: '!mInputCallback', file c:/devel/mozilla-cent
ral/xpcom/io/nsPipe3.cpp, line 84

This happens when an nsPipeEvents RAII object tries to register more than one callback.  This previously was impossible until I changed it to support multiple input streams for cloning.  Now multiple streams can be woken up simultaneously for a single write to the pipe.

	Ben Kelly [:bkelly, not reviewing] Assignee
	Comment 6 • 9 years ago

This fixes the problem for me locally.

	Ben Kelly [:bkelly, not reviewing] Assignee
	Comment 7 • 9 years ago

This test causes our current tree to hit the same assertion as comment 5.  With the P1 fix we pass the test.

And now we actually have a gtest for nsPipeInputStream::AsyncRead(). :-\

	Nathan Froyd [:froydnj]
	Comment 8 • 9 years ago

Comment on attachment 8659641 [details] [diff] [review]
P1 Allow pipe to wake up multiple streams at the same time. r=froydnj

Review of attachment 8659641 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with the template cleverness removed.

::: xpcom/io/nsPipe3.cpp
@@ +93,5 @@
>    }
>  
>  private:
> +  template <typename S, typename C>
> +  struct Entry

Why are we getting all clever with templates here?  Is this because we're thinking about:

Entry<nsIAsyncOutputStream, nsIOutputStreamCallback>

?  I don't think you need to worry about that...

	Nathan Froyd [:froydnj]
	Comment 9 • 9 years ago

Comment on attachment 8659644 [details] [diff] [review]
P2 Add gtests for nsPipeInputStream AsyncRead(). r=froydnj

Review of attachment 8659644 [details] [diff] [review]:
-----------------------------------------------------------------

Hooray for tests.

	Ben Kelly [:bkelly, not reviewing] Assignee
	Comment 10 • 9 years ago

These patches were included in this try build:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d2bffecb1472

	Pulsebot
	Comment 11 • 9 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/6d24b09bb144
https://hg.mozilla.org/integration/mozilla-inbound/rev/e2cdb157f476

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 12 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/6d24b09bb144
https://hg.mozilla.org/mozilla-central/rev/e2cdb157f476