Another attempt at landing this patch....

Another attempt at landing this patch.

The reliability tests regressions caused by this patch have been addressed by the upstream bug fix https://bugs.webkit.org/show_bug.cgi?id=27769

The IPCs for carrying data requested by plugins have been changed from synchronous IPCs to asynchronous IPCs.

This fixes bug http://code.google.com/p/chromium/issues/detail?id=14323,
where the Flash plugin would not render content on the page if these IPCs were processed while the plugin waited for sync calls like NPN_Evaluate to return.

This CL also fixes the following bugs, which were crashes in reliability test runs when this patch was landed last time. 

http://code.google.com/p/chromium/issues/detail?id=18058
http://code.google.com/p/chromium/issues/detail?id=18059

The crash happens because of NPP_Write calls issued to the plugin while it is waiting for an NPN_Invoke call to return in the context of NPP_NewStream. Inspecting the safari plugin implementation revealed that they defer the resource load before calling the plugin and restore it on return.

We emulate this behavior via an IPC sent from the plugin which serves as an acknowledgement.

Test=covered by UI tests.

Bug=14323, 18058, 18059
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=22369

[ananta, 2009-07-31 23:06:55]

Hi John

This is a continuation of the original CL 21548
(http://codereview.chromium.org/159296) which was reverted due to reliability
test issues.

The new changes as per our discussion are support for deferring loads on the
resource loader when we call into the plugin. This helps avoid reentrancy and is
on the same lines as the Safari plugin implementation.

[jam, 2009-08-01 02:00:35]

http://codereview.chromium.org/159746/diff/1/9
File webkit/glue/webplugin_impl.cc (right):

http://codereview.chromium.org/159746/diff/1/9#newcode1011
Line 1011: // further loading until the plugin notifies us that it is ready to
accept
nit?  further -> stopping?

http://codereview.chromium.org/159746/diff/1/9#newcode1076
Line 1076: client_info->client = NULL;
Why is this needed now and not before?  Is it because you removed RemoveClient? 
Also, when does RemoveClient get called now?

http://codereview.chromium.org/159746/diff/1/9#newcode1314
Line 1314: } else {
nit: no need for else, just put the ++ on the next line.

[ananta, 2009-08-01 03:20:28]

http://codereview.chromium.org/159746/diff/1/9
File webkit/glue/webplugin_impl.cc (right):

http://codereview.chromium.org/159746/diff/1/9#newcode1011
Line 1011: // further loading until the plugin notifies us that it is ready to
accept
On 2009/08/01 02:00:35, John Abd-El-Malek wrote:
> nit?  further -> stopping?

Done.

http://codereview.chromium.org/159746/diff/1/9#newcode1076
Line 1076: client_info->client = NULL;
On 2009/08/01 02:00:35, John Abd-El-Malek wrote:
> Why is this needed now and not before?  Is it because you removed
RemoveClient? 
> Also, when does RemoveClient get called now?
Yes. Earlier it was a sync call and so it was correct to clean up the ClientInfo
here. Now RemoveClient gets called in CancelResource which is invoked when the
PluginStream gets closed.

http://codereview.chromium.org/159746/diff/1/9#newcode1314
Line 1314: } else {
On 2009/08/01 02:00:35, John Abd-El-Malek wrote:
> nit: no need for else, just put the ++ on the next line.

Done.

[jam, 2009-08-03 20:06:07]

lgtm