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

Review URL: http://codereview.chromium.org/159746

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@22369 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 103 insertions, 10 deletions

diff --git a/webkit/glue/plugins/plugin_instance.cc b/webkit/glue/plugins/plugin_instance.cc
index d980e10..79b5b4f 100644
--- a/webkit/glue/plugins/plugin_instance.cc
+++ b/webkit/glue/plugins/plugin_instance.cc
@@ -373,12 +373,10 @@ void PluginInstance::DidReceiveManualResponse(const std::string& url,
     response_url = instance_url_.spec();
   }
 
-  bool cancel = false;
-
   plugin_data_stream_ = CreateStream(-1, url, mime_type, false, NULL);
 
   plugin_data_stream_->DidReceiveResponse(mime_type, headers, expected_length,
-                                          last_modified, true, &cancel);
+                                          last_modified, true);
 }
 
 void PluginInstance::DidReceiveManualData(const char* buffer, int length) {
diff --git a/webkit/glue/plugins/plugin_stream_url.cc b/webkit/glue/plugins/plugin_stream_url.cc
index b050411..2aa2f84 100644
--- a/webkit/glue/plugins/plugin_stream_url.cc
+++ b/webkit/glue/plugins/plugin_stream_url.cc
@@ -44,16 +44,18 @@ void PluginStreamUrl::DidReceiveResponse(const std::string& mime_type,
                                          const std::string& headers,
                                          uint32 expected_length,
                                          uint32 last_modified,
-                                         bool request_is_seekable,
-                                         bool* cancel) {
+                                         bool request_is_seekable) {
   bool opened = Open(mime_type,
                      headers,
                      expected_length,
                      last_modified,
                      request_is_seekable);
   if (!opened) {
+    CancelRequest();
     instance()->RemoveStream(this);
-    *cancel = true;
+  } else {
+    if (id_ > 0)
+      instance()->webplugin()->SetDeferResourceLoading(id_, false);
   }
 }
 
@@ -62,8 +64,11 @@ void PluginStreamUrl::DidReceiveData(const char* buffer, int length,
   if (!open())
     return;
 
-  if (length > 0)
+  if (length > 0) {
     Write(const_cast<char*>(buffer), length, data_offset);
+    if (id_ > 0)
+      instance()->webplugin()->SetDeferResourceLoading(id_, false);
+  }
 }
 
 void PluginStreamUrl::DidFinishLoading() {
diff --git a/webkit/glue/plugins/plugin_stream_url.h b/webkit/glue/plugins/plugin_stream_url.h
index db5d4a5..1f5fe57 100644
--- a/webkit/glue/plugins/plugin_stream_url.h
+++ b/webkit/glue/plugins/plugin_stream_url.h
@@ -48,8 +48,7 @@ class PluginStreamUrl : public PluginStream,
                           const std::string& headers,
                           uint32 expected_length,
                           uint32 last_modified,
-                          bool request_is_seekable,
-                          bool* cancel);
+                          bool request_is_seekable);
   void DidReceiveData(const char* buffer, int length, int data_offset);
   void DidFinishLoading();
   void DidFail();
diff --git a/webkit/glue/plugins/test/plugin_get_javascript_url_test.cc b/webkit/glue/plugins/test/plugin_get_javascript_url_test.cc
index 74d01f1..ebdd745 100644
--- a/webkit/glue/plugins/test/plugin_get_javascript_url_test.cc
+++ b/webkit/glue/plugins/test/plugin_get_javascript_url_test.cc
@@ -17,11 +17,19 @@
 // The maximum chunk size of stream data.
 #define STREAM_CHUNK 197
 
+const int kNPNEvaluateTimerID = 100;
+const int kNPNEvaluateTimerElapse = 50;
+
+
 namespace NPAPIClient {
 
 ExecuteGetJavascriptUrlTest::ExecuteGetJavascriptUrlTest(NPP id, NPNetscapeFuncs *host_functions)
   : PluginTest(id, host_functions),
-    test_started_(false) {
+    test_started_(false),
+#ifdef OS_WIN
+    window_(NULL),
+#endif
+    npn_evaluate_context_(false) {
 }
 
 NPError ExecuteGetJavascriptUrlTest::SetWindow(NPWindow* pNPWindow) {
@@ -30,15 +38,64 @@ NPError ExecuteGetJavascriptUrlTest::SetWindow(NPWindow* pNPWindow) {
     HostFunctions()->geturlnotify(id(), url.c_str(), "_top",
                                   reinterpret_cast<void*>(SELF_URL_STREAM_ID));
     test_started_ = true;
+
+#ifdef OS_WIN
+    HWND window_handle = reinterpret_cast<HWND>(pNPWindow->window);
+    if (!::GetProp(window_handle, L"Plugin_Instance")) {
+      ::SetProp(window_handle, L"Plugin_Instance", this);
+      // We attempt to retreive the NPObject for the plugin instance identified
+      // by the NPObjectLifetimeTestInstance2 class as it may not have been
+      // instantiated yet.
+      SetTimer(window_handle, kNPNEvaluateTimerID, kNPNEvaluateTimerElapse,
+               TimerProc);
+    }
+    window_ = window_handle;
+#endif
   }
+
   return NPERR_NO_ERROR;
 }
 
+#ifdef OS_WIN
+void CALLBACK ExecuteGetJavascriptUrlTest::TimerProc(
+    HWND window, UINT message, UINT timer_id, unsigned long elapsed_time) {
+  ExecuteGetJavascriptUrlTest* this_instance =
+      reinterpret_cast<ExecuteGetJavascriptUrlTest*>
+          (::GetProp(window, L"Plugin_Instance"));
+
+  NPObject *window_obj = NULL;
+  this_instance->HostFunctions()->getvalue(this_instance->id(),
+                                           NPNVWindowNPObject,
+                                           &window_obj);
+  if (!window_obj) {
+    this_instance->SetError("Failed to get NPObject for plugin instance2");
+    this_instance->SignalTestCompleted();
+    return;
+  }
+
+  std::string script = "javascript:window.location";
+  NPString script_string;
+  script_string.UTF8Characters = script.c_str();
+  script_string.UTF8Length = static_cast<unsigned int>(script.length());
+  NPVariant result_var;
+
+  this_instance->npn_evaluate_context_ = true;
+  NPError result = this_instance->HostFunctions()->evaluate(
+      this_instance->id(), window_obj, &script_string, &result_var);
+  this_instance->npn_evaluate_context_ = false;
+}
+#endif
+
 NPError ExecuteGetJavascriptUrlTest::NewStream(NPMIMEType type, NPStream* stream,
                               NPBool seekable, uint16* stype) {
   if (stream == NULL)
     SetError("NewStream got null stream");
 
+  if (npn_evaluate_context_) {
+    SetError("NewStream received in context of NPN_Evaluate");
+    return NPERR_NO_ERROR;
+  }
+
   COMPILE_ASSERT(sizeof(unsigned long) <= sizeof(stream->notifyData),
                  cast_validity_check);
   unsigned long stream_id = reinterpret_cast<unsigned long>(stream->notifyData);
@@ -53,6 +110,10 @@ NPError ExecuteGetJavascriptUrlTest::NewStream(NPMIMEType type, NPStream* stream
 }
 
 int32 ExecuteGetJavascriptUrlTest::WriteReady(NPStream *stream) {
+  if (npn_evaluate_context_) {
+    SetError("WriteReady received in context of NPN_Evaluate");
+    return NPERR_NO_ERROR;
+  }
   return STREAM_CHUNK;
 }
 
@@ -63,6 +124,11 @@ int32 ExecuteGetJavascriptUrlTest::Write(NPStream *stream, int32 offset, int32 l
   if (len < 0 || len > STREAM_CHUNK)
     SetError("Write got bogus stream chunk size");
 
+  if (npn_evaluate_context_) {
+    SetError("Write received in context of NPN_Evaluate");
+    return len;
+  }
+
   COMPILE_ASSERT(sizeof(unsigned long) <= sizeof(stream->notifyData),
                  cast_validity_check);
   unsigned long stream_id = reinterpret_cast<unsigned long>(stream->notifyData);
@@ -83,6 +149,15 @@ NPError ExecuteGetJavascriptUrlTest::DestroyStream(NPStream *stream, NPError rea
   if (stream == NULL)
     SetError("NewStream got null stream");
 
+#ifdef OS_WIN
+  KillTimer(window_, kNPNEvaluateTimerID);
+#endif
+
+  if (npn_evaluate_context_) {
+    SetError("DestroyStream received in context of NPN_Evaluate");
+    return NPERR_NO_ERROR;
+  }
+
   COMPILE_ASSERT(sizeof(unsigned long) <= sizeof(stream->notifyData),
                  cast_validity_check);
   unsigned long stream_id = reinterpret_cast<unsigned long>(stream->notifyData);
@@ -100,6 +175,12 @@ NPError ExecuteGetJavascriptUrlTest::DestroyStream(NPStream *stream, NPError rea
 void ExecuteGetJavascriptUrlTest::URLNotify(const char* url, NPReason reason, void* data) {
   COMPILE_ASSERT(sizeof(unsigned long) <= sizeof(data),
                  cast_validity_check);
+
+  if (npn_evaluate_context_) {
+    SetError("URLNotify received in context of NPN_Evaluate");
+    return;
+  }
+
   unsigned long stream_id = reinterpret_cast<unsigned long>(data);
   switch (stream_id) {
     case SELF_URL_STREAM_ID:
diff --git a/webkit/glue/plugins/test/plugin_get_javascript_url_test.h b/webkit/glue/plugins/test/plugin_get_javascript_url_test.h
index cff6775..5c2540d 100644
--- a/webkit/glue/plugins/test/plugin_get_javascript_url_test.h
+++ b/webkit/glue/plugins/test/plugin_get_javascript_url_test.h
@@ -28,8 +28,18 @@ class ExecuteGetJavascriptUrlTest : public PluginTest {
   virtual void    URLNotify(const char* url, NPReason reason, void* data);
 
  private:
+#if defined(OS_WIN)
+  static void CALLBACK TimerProc(HWND window, UINT message, UINT timer_id,
+                                 unsigned long elapsed_time);
+#endif
   bool test_started_;
+  // This flag is set to true in the context of the NPN_Evaluate call.
+  bool npn_evaluate_context_;
   std::string self_url_;
+
+#if defined(OS_WIN)
+  HWND window_;
+#endif
 };
 
 } // namespace NPAPIClient