UA:Mozilla/5.0 (Windows NT 10.0; WOW64; rv:40.0) Gecko/20100101 Firefox/40.0 Many Chinese live streaming websites don't work on Firefox 40 with Asynchronous Plugin Initialization. Sites list: http://live.bilibili.com/ Alexa Global Rank:306 Rank in China:60 http://www.douyutv.com/ Alexa Global Rank:679 Rank in China:97 Steps to reproduce: 1.Visit these websites on Firefox 40 with default profile. 2.The Flash player on websites loaded, but not load the streaming video. 3.Set dom.ipc.plugins.asyncInit false. Restart Firefox. 4.Visit the websites again, that works well.

Thanks for the report. I'll take a look.

Some streams are failing because the buffer is not being grown to accommodate additional incoming data. The buffer growth was predicated upon the stream being suspended. If the stream was not suspended, the buffer would not be able to hold new data and nothing would be read from the input stream, eventually triggering an error in Necko. I don't see any reason why the mIsSuspended predicate needs to be there, as the buffer needs to be large enough regardless.

Updated comments around affected code.

Comment on attachment 8652615 [details] [diff] [review] Patch Review of attachment 8652615 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/plugins/base/nsNPAPIPluginStreamListener.cpp @@ +560,4 @@ > uint32_t amountRead = 0; > rv = input->Read(mStreamBuffer + mStreamBufferByteCount, bytesToRead, > &amountRead); > NS_ENSURE_SUCCESS(rv, rv); looks like we would fall through to here and fail this read call due to lack of buffer? now we go ahead and reallocation to collect the data. This is ok for a suspended stream?

Comment on attachment 8652615 [details] [diff] [review] Patch Review of attachment 8652615 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/plugins/base/nsNPAPIPluginStreamListener.cpp @@ +548,4 @@ > mStreamBufferSize = mStreamBufferByteCount + length; > char *buf = (char*)PR_Realloc(mStreamBuffer, mStreamBufferSize); > if (!buf) > return NS_ERROR_OUT_OF_MEMORY; if this reallocation fails, mStreamBufferSize will not match mStreamBuffer. Is that ok? I realize this is a prior issue

(In reply to Jim Mathies [:jimm] from comment #4) > Comment on attachment 8652615 [details] [diff] [review] > Patch > > Review of attachment 8652615 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: dom/plugins/base/nsNPAPIPluginStreamListener.cpp > @@ +560,4 @@ > > uint32_t amountRead = 0; > > rv = input->Read(mStreamBuffer + mStreamBufferByteCount, bytesToRead, > > &amountRead); > > NS_ENSURE_SUCCESS(rv, rv); > > looks like we would fall through to here and fail this read call due to lack > of buffer? now we go ahead and reallocation to collect the data. This is ok > for a suspended stream? My understanding is that this is exactly what we want. While suspended, Necko still expects us to take the data as part of the API contract, so we need to read it into the buffer to hold it until the stream is resumed. (In reply to Vladan Djeric (:vladan) -- please needinfo! from comment #5) > Comment on attachment 8652615 [details] [diff] [review] > Patch > > Review of attachment 8652615 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: dom/plugins/base/nsNPAPIPluginStreamListener.cpp > @@ +548,4 @@ > > mStreamBufferSize = mStreamBufferByteCount + length; > > char *buf = (char*)PR_Realloc(mStreamBuffer, mStreamBufferSize); > > if (!buf) > > return NS_ERROR_OUT_OF_MEMORY; > > if this reallocation fails, mStreamBufferSize will not match mStreamBuffer. > Is that ok? > I realize this is a prior issue Stepping through this function's callers yesterday I saw very aggressive return code checking. Everything should shut down when that error code is returned.

Comment on attachment 8652615 [details] [diff] [review] Patch Approval Request Comment [Feature/regressing bug #]: NPAPI plugin streams [User impact if declined]: Gecko can't properly fulfill some network requests made by plugins. Seen more frequently with async init, but present without it too. [Describe test coverage new/current, TreeHerder]: Plugin test suite. Also tested against links reported in this bug as well as other bugs blocking bug 1195607. [Risks and why]: Low. Change effectively removes one predicate from an if statement which is quite trivial. [String/UUID change made/needed]: None

Comment on attachment 8652615 [details] [diff] [review] Patch Patch seems low risk, and the sooner we uplift to Beta we can get confirmation from our end-users that this works as expected. Aurora42+, Beta41+

I tried to reproduce with Firefox 40RC, the first link works fine and for the second one there are some streams that don't start playing (Stream not found is displayed in the web console). Using Firefox 41 beta 5, both links are playing under Win 7 64-bit and Mac OS X 10.9.5 (here live.bilibili.com needed to be refreshed several times in order to be loaded). Can you please use Firefox 41 beta 5 [1] and let us know if both links are correctly played now? Thank you! [1] https://ftp.mozilla.org/pub/mozilla.org/firefox/candidates/41.0b5-candidates/build1/

Verified on Win7, Win10 and Mac OSX with Firefox 41 beta 5, both links are playing well. Mozilla/5.0 (Windows NT 6.1; WOW64; rv:41.0) Gecko/20100101 Firefox/41.0 Mozilla/5.0 (Windows NT 10.0; WOW64; rv:41.0) Gecko/20100101 Firefox/41.0 Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:41.0) Gecko/20100101 Firefox/41.0 BuildID: 20150827142634