nsPluginHost has primary channel opening logic that should be entirely superceded by nsObjectLoadingContent, and should be cleaned up and removed

What is the plan here John?

So nsPluginHost is still handling streams for plugins that instantiate without considering the channel type. Opening the channel, then, is done by pluginHost - which is ultimately another unnecessary codepath. This is also the codepath that's leaking in bug 812629. This just moves all initial stream opening to ObjectLoadingContent, without changing the logic for selecting a plugin type.

Removes the code obsoleted by Part 1, as well as a ton of code that was obsoleted by refactoring ObjectLoadingContent, and a little that has been dead since ObjectLoadingContent was created...

Bonus whitespace changes

Comment on attachment 686893 [details] [diff] [review] Part 2 - Remove a ton of obsolete nsPluginHost code Review of attachment 686893 [details] [diff] [review]: ----------------------------------------------------------------- These patches are so beautiful, I think I'm tearing up a little.

Try run for 1c5c0a2f0086 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=1c5c0a2f0086 Results (out of 125 total builds): exception: 1 success: 83 warnings: 25 failure: 16 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jschoenick@mozilla.com-1c5c0a2f0086

Okay, new edition of this patch series. This patch just cleans up some logic and comments, and should be a no-op functionality wise.

So we the previous version of this patch caused us to always wait on channels to spawn plugins - but this regresses behavior of tags like: > <embed src="./pony.swf"> Because we will see .swf, spawn flash, and then create a pluginstreamlistener for it after the fact. In this version of the patch we change ObjectLoadingContent to be able to handle channels opened after the fact, so we can still "fast" spawn this type of plugin. This also would make it possible to make <object> behave this way when it has an explicit type, but I don't think we want to add more ways to synchronously spawn plugins. Compared to the earlier version of this patch, this makes OnStartRequest and LoadObject aware of these 'late' streams for plugins, and adds a helper to (re)create a nsPluginStreamListener for plugins that load this way, incidentally fixing bug 767627.

This fixes a bug I uncovered with the previous changes - if we call LoadObject (such as because we re-bound to tree) after mChannel has hit OnStopRequest(), we'll think we need to open a new channel and re-load because we can't inspect its content type. This adds a few checks to just re-use the existing type if our state didn't otherwise change.

Comment on attachment 690640 [details] [diff] [review] Part 2 - nsObjectLoadingContent should handle all initial streams for plugins Review of attachment 690640 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/plugins/base/nsPluginHost.h @@ -211,5 @@ > nsresult > TrySetUpPluginInstance(const char *aMimeType, nsIURI *aURL, nsPluginInstanceOwner *aOwner); > > - nsresult > - NewEmbeddedPluginStream(nsIURI* aURL, nsObjectLoadingContent *aContent, nsNPAPIPluginInstance* aInstance); I was confused about this at first but it seems you already removed the method and its callers in a previous patch, and just forgot this.

Folded removing NewEmbeddedPluginStream from the header into the proper patch (part 3)

Comment on attachment 690637 [details] [diff] [review] Part 1 - Cleanup nsObjectLoadingContent logic a bit https://hg.mozilla.org/integration/mozilla-inbound/rev/6aed5f5caecb https://hg.mozilla.org/integration/mozilla-inbound/rev/1b68ff48e22c https://hg.mozilla.org/integration/mozilla-inbound/rev/11341616d7cb https://hg.mozilla.org/integration/mozilla-inbound/rev/a2476e368fb6 https://hg.mozilla.org/integration/mozilla-inbound/rev/62389ec6712c

https://hg.mozilla.org/mozilla-central/rev/6aed5f5caecb https://hg.mozilla.org/mozilla-central/rev/1b68ff48e22c https://hg.mozilla.org/mozilla-central/rev/11341616d7cb https://hg.mozilla.org/mozilla-central/rev/a2476e368fb6 https://hg.mozilla.org/mozilla-central/rev/62389ec6712c

Thanks for the fixes! Is Mozilla20 the sames as Firefox 20? When can users download the updated/corrected version? Thanks in advance, Neil

It is available for testing in the next day or two on the experimental Nightly Channel. Firefox 20 will move to Aurora, and later to Beta. Firefox 20 is scheduled to be released to the general public on April 2. (See https://wiki.mozilla.org/RapidRelease/Calendar .)

I tried the nightly build today, and the moment I hit the 3D Stereo option to activate the plug-in, Firefox crashes to desktop. Do you know if the bug has been fixed in the currently available nightly build? Can you do a quick test of the plug-in to see if it works for you? Thanks in advance, Neil

(In reply to neils from comment #21) > I tried the nightly build today, and the moment I hit the 3D Stereo option > to activate the plug-in, Firefox crashes to desktop. Do you know if the bug > has been fixed in the currently available nightly build? > > Can you do a quick test of the plug-in to see if it works for you? > > Thanks in advance, > Neil This is probably unrelated to the changes in this bug, can you open a new bug report for this?

Try run for 1c5c0a2f0086 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=1c5c0a2f0086 Results (out of 126 total builds): exception: 1 success: 83 warnings: 25 failure: 17 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jschoenick@mozilla.com-1c5c0a2f0086