Plugin no longer makes synchronous NPAPI calls from a Windows message handler. This fixes deadlocks and slowdown in Chrome. The approach is strange. It asynchronously opens the url data:, and then invokes Tick from the finish callback. This is the simplest approach I could think of that hide widespread browser support. NPN_PluginThreadAsyncCall would be ideal but it is supported by all browsers we currently support.

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

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

11 files changed, 122 insertions, 111 deletions

diff --git a/o3d/plugin/cross/main.h b/o3d/plugin/cross/main.h
index 003a63a..c5de7b6 100644
--- a/o3d/plugin/cross/main.h
+++ b/o3d/plugin/cross/main.h
@@ -99,19 +99,6 @@ extern "C" {
 
 namespace o3d {
 
-class RenderOnDemandCallbackHandler
-    : public o3d::Client::RenderOnDemandCallback {
- public:
-  explicit RenderOnDemandCallbackHandler(glue::_o3d::PluginObject* obj)
-      : obj_(obj) {
-  }
-
-  // This function is implemented for each platform.
-  virtual void Run();
- private:
-  glue::_o3d::PluginObject* obj_;
-};
-
 void WriteLogString(const char* text, int length);
 NPError NPP_Destroy(NPP instance, NPSavedData **save);
 NPError NPP_DestroyStream(NPP instance, NPStream *stream, NPReason reason);
diff --git a/o3d/plugin/cross/o3d_glue.cc b/o3d/plugin/cross/o3d_glue.cc
index 68565aa..65912ba 100644
--- a/o3d/plugin/cross/o3d_glue.cc
+++ b/o3d/plugin/cross/o3d_glue.cc
@@ -106,6 +106,7 @@ PluginObject::PluginObject(NPP npp)
       features_(NULL),
       fullscreen_region_valid_(false),
       renderer_init_status_(Renderer::UNINITIALIZED),
+      pending_ticks_(0),
 #ifdef OS_WIN
       hWnd_(NULL),
       fullscreen_hWnd_(NULL),
@@ -131,7 +132,6 @@ PluginObject::PluginObject(NPP npp)
       mac_agl_context_(0),
       mac_cgl_context_(0),
       last_mac_event_time_(0),
-      wants_redraw_(false),
       time_to_hide_overlay_(0.0),
 #endif
 #ifdef OS_LINUX
@@ -388,15 +388,6 @@ void PluginObject::PlatformSpecificSetCursor() {
   }
 }
 
-bool PluginObject::WantsRedraw() {
-  if (client()->render_mode() == o3d::Client::RENDERMODE_CONTINUOUS)
-    return true;
-
-  // If we're rendering on-demand, then a call to client->render() should
-  // only force a redraw one time
-  return wants_redraw_;
-}
-
 bool PluginObject::SetRendererIsSoftware(bool state) {
   renderer_is_software_ = state;
   ClientInfoManager* client_info_manager =
@@ -511,12 +502,14 @@ static void PluginDeallocate(NPObject *object) {
 static bool PluginHasMethod(NPObject *header, NPIdentifier name) {
   DebugScopedId id(name);
   PluginObject *plugin_object = static_cast<PluginObject *>(header);
-  if (name == method_ids[METHOD_EVAL]) {
-    return true;
-  } else {
-    NPObject *globals = plugin_object->globals_npobject();
-    return globals->_class->hasMethod(globals, name);
+  for (int i = 0; i < NUM_METHOD_IDS; ++i) {
+    if (name == method_ids[i]) {
+      return true;
+    }
   }
+
+  NPObject *globals = plugin_object->globals_npobject();
+  return globals->_class->hasMethod(globals, name);
 }
 
 static bool PluginInvoke(NPObject *header, NPIdentifier name,
@@ -829,6 +822,54 @@ void PluginObject::PlatformSpecificSetCursor() {
 
 #endif  // OS_LINUX
 
+void PluginObject::AsyncTick() {
+  if (pending_ticks_ >= 1)
+    return;
+
+  class TickCallback : public StreamManager::FinishedCallback {
+   public:
+    explicit TickCallback(PluginObject* plugin_object)
+        : plugin_object_(plugin_object) {
+    }
+
+    virtual void Run(DownloadStream*,
+                     bool,
+                     const std::string&,
+                     const std::string&) {
+      plugin_object_->Tick();
+    }
+
+   private:
+    PluginObject* plugin_object_;
+  };
+
+  ++pending_ticks_;
+
+  // Invoke Client::Tick and Client::RenderClient in a way that is asynchronous
+  // in Chrome. This avoids issues with making calls into the browser from a
+  // message handler.
+  // If NPN_PluginThreadAsyncCall worked in more browsers, it would be simpler
+  // to use that.
+  // We're calling LoadURL here with a URL that will return 0 bytes on browsers

+  // that support the "data:" protocol and fail in browsers that don't like IE.

+  // On browsers that support it, the side effect is to call the TickCallback.

+  if (!stream_manager_->LoadURL("data:,", NULL, NULL, NULL,
+                                new TickCallback(this), NP_NORMAL)) {
+    // Fallback on synchronous call if asynchronous load fails.
+    Tick();
+  }
+}
+
+void PluginObject::Tick() {
+  client_->Tick();
+  if (renderer_ && renderer_->need_to_render()) {
+    client_->RenderClient();
+  }
+
+  DCHECK(pending_ticks_ > 0);
+  --pending_ticks_;
+}
+
 }  // namespace _o3d
 
 namespace globals {
diff --git a/o3d/plugin/cross/o3d_glue.h b/o3d/plugin/cross/o3d_glue.h
index 016b58b..e67d8e6 100644
--- a/o3d/plugin/cross/o3d_glue.h
+++ b/o3d/plugin/cross/o3d_glue.h
@@ -140,6 +140,8 @@ class PluginObject: public NPObject {
   ClientNPObject *client_npobject_;
   std::string user_agent_;
   Renderer::InitStatus renderer_init_status_;
+  int pending_ticks_;
+
   // The current cursor type.
   o3d::Cursor::CursorType cursor_type_;
 
@@ -207,12 +209,6 @@ class PluginObject: public NPObject {
     return mac_fullscreen_window_;
   }
 
-  // Always returns |true| if RENDERMODE_CONTINUOUS, otherwise
-  // only if client->render() has been called and we haven't yet
-  // handled it
-  bool WantsRedraw();
-  void SetWantsRedraw(bool wants) { wants_redraw_ = wants; }
-
   bool ScrollIsInProgress() { return scroll_is_in_progress_; }
   void SetScrollIsInProgress(bool state) { scroll_is_in_progress_ = state; }
   bool scroll_is_in_progress_;
@@ -226,7 +222,6 @@ class PluginObject: public NPObject {
   WindowRef mac_window_;  // may be NULL in the Chrome case
   // these vars needed for the Safari tab switch detection hack
   CFDateRef last_mac_event_time_;
-  bool wants_redraw_;
   void * mac_cocoa_window_;
   void* mac_window_selected_tab_;
   bool mac_surface_hidden_;
@@ -362,6 +357,13 @@ class PluginObject: public NPObject {
   // Sets the cursor to whatever the current cursor is.
   void PlatformSpecificSetCursor();
 
+  // Asynchronously (if possible, synchronously otherwise) invoke Tick. No
+  // operation if an asynchronous tick is already pending.
+  void AsyncTick();
+
+  // Tick the client.
+  void Tick();
+
   const std::string& user_agent() const { return user_agent_; }
   bool IsFirefox() const {
     return user_agent_.find("Firefox") != user_agent_.npos;
diff --git a/o3d/plugin/linux/main_linux.cc b/o3d/plugin/linux/main_linux.cc
index b897bb7..af39342 100644
--- a/o3d/plugin/linux/main_linux.cc
+++ b/o3d/plugin/linux/main_linux.cc
@@ -70,10 +70,14 @@ void LinuxTimer(XtPointer data, XtIntervalId* id) {
   DCHECK(obj->xt_interval_ == *id);
   obj->client()->Tick();
   obj->draw_ = true;
-  if (obj->client()->render_mode() == o3d::Client::RENDERMODE_CONTINUOUS) {
-    // NOTE: this draws no matter what instead of just invalidating the region,
-    // which means it will execute even if the plug-in window is invisible.
-    DrawPlugin(obj);
+  if (obj->renderer()) {
+    if (obj->client()->render_mode() == o3d::Client::RENDERMODE_CONTINUOUS ||

+        obj->renderer()->need_to_render()) {

+      // NOTE: this draws no matter what instead of just invalidating the
+      // region, which means it will execute even if the plug-in window is
+      // invisible.
+      DrawPlugin(obj);
+    }
   }
   obj->xt_interval_ =
       XtAppAddTimeOut(obj->xt_app_context_, 10, LinuxTimer, obj);
@@ -563,9 +567,11 @@ static gboolean GtkTimeoutCallback(gpointer user_data) {
   PluginObject *obj = static_cast<PluginObject *>(user_data);
   obj->draw_ = true;
   obj->client()->Tick();
-  if (obj->client()->render_mode() ==
-      o3d::Client::RENDERMODE_CONTINUOUS) {
-    gtk_widget_queue_draw(obj->gtk_container_);
+  if (obj->renderer()) {
+    if (obj->client()->render_mode() == o3d::Client::RENDERMODE_CONTINUOUS ||

+        obj->renderer()->need_to_render()) {

+      gtk_widget_queue_draw(obj->gtk_container_);
+    }
   }
   return TRUE;
 }
@@ -647,10 +653,6 @@ NPError PlatformNPPGetValue(NPP instance, NPPVariable variable, void *value) {
   return NPERR_NO_ERROR;
 }
 
-void RenderOnDemandCallbackHandler::Run() {
-  DrawPlugin(obj_);
-}
-
 NPError NPP_New(NPMIMEType pluginType, NPP instance, uint16 mode, int16 argc,
                 char *argn[], char *argv[], NPSavedData *saved) {
   HANDLE_CRASHES;
@@ -761,8 +763,6 @@ NPError NPP_SetWindow(NPP instance, NPWindow *window) {
 
     obj->CreateRenderer(default_display);
     obj->client()->Init();
-    obj->client()->SetRenderOnDemandCallback(
-        new RenderOnDemandCallbackHandler(obj));
     obj->display_ = display;
     obj->window_ = xwindow;
   }
diff --git a/o3d/plugin/mac/main_mac.mm b/o3d/plugin/mac/main_mac.mm
index 3427505..1ef1d1c 100644
--- a/o3d/plugin/mac/main_mac.mm
+++ b/o3d/plugin/mac/main_mac.mm
@@ -70,25 +70,11 @@ namespace {
 // destroy it explicitly.
 scoped_ptr<base::AtExitManager> g_at_exit_manager;
 
-// if defined, in AGL mode we do double buffered drawing
-// #define USE_AGL_DOUBLE_BUFFER
-
 #define CFTIMER
 // #define DEFERRED_DRAW_ON_NULLEVENTS
 
-// currently drawing with the timer doesn't play well with USE_AGL_DOUBLE_BUFFER
-#ifdef CFTIMER
-#undef USE_AGL_DOUBLE_BUFFER
-#endif
-
 void DrawPlugin(PluginObject* obj) {
   obj->client()->RenderClient();
-#ifdef USE_AGL_DOUBLE_BUFFER
-  // In AGL mode, we have to call aglSwapBuffers to guarantee that our
-  // pixels make it to the screen.
-  if (obj->mac_agl_context_ != NULL)
-    aglSwapBuffers(obj->mac_agl_context_);
-#endif
 }
 
 unsigned char GetMacEventKeyChar(const EventRecord *the_event) {
@@ -809,10 +795,6 @@ bool HandleMacEvent(EventRecord* the_event, NPP instance) {
   return handled;
 }
 
-void RenderOnDemandCallbackHandler::Run() {
-  obj_->SetWantsRedraw(true);
-}
-
 NPError NPP_New(NPMIMEType pluginType, NPP instance, uint16 mode, int16 argc,
                 char* argn[], char* argv[], NPSavedData* saved) {
   HANDLE_CRASHES;
@@ -1124,8 +1106,6 @@ NPError NPP_SetWindow(NPP instance, NPWindow* window) {
   }
 
   obj->client()->Init();
-  obj->client()->SetRenderOnDemandCallback(
-      new RenderOnDemandCallbackHandler(obj));
 
   if (obj->renderer()) {
     obj->renderer()->SetClientOriginOffset(gl_x_origin, gl_y_origin);
diff --git a/o3d/plugin/mac/plugin_mac.mm b/o3d/plugin/mac/plugin_mac.mm
index 459f116..5df3b88 100644
--- a/o3d/plugin/mac/plugin_mac.mm
+++ b/o3d/plugin/mac/plugin_mac.mm
@@ -234,19 +234,20 @@ void RenderTimer::TimerCallback(CFRunLoopTimerRef timer, void* info) {
     bool plugin_visible = in_fullscreen ||
         (obj->last_buffer_rect_[2] > 1 && obj->last_buffer_rect_[3] > 1);
 
-    if (plugin_visible && obj->WantsRedraw()) {
-      obj->SetWantsRedraw(false);  // for on-demand drawing
-
-      // Force a sync to the VBL (once per timer callback)
-      // to avoid tearing
-      GLint sync = (i == 0);
-      if (obj->mac_cgl_context_) {
-        CGLSetParameter(obj->mac_cgl_context_, kCGLCPSwapInterval, &sync);
-      } else if (obj->mac_agl_context_) {
-        aglSetInteger(obj->mac_agl_context_, AGL_SWAP_INTERVAL, &sync);
-      }
+    if (plugin_visible && obj->renderer()) {
+      if (obj->client()->render_mode() == o3d::Client::RENDERMODE_CONTINUOUS ||
+          obj->renderer()->need_to_render()) {
+        // Force a sync to the VBL (once per timer callback)
+        // to avoid tearing
+        GLint sync = (i == 0);
+        if (obj->mac_cgl_context_) {
+          CGLSetParameter(obj->mac_cgl_context_, kCGLCPSwapInterval, &sync);
+        } else if (obj->mac_agl_context_) {
+          aglSetInteger(obj->mac_agl_context_, AGL_SWAP_INTERVAL, &sync);
+        }
 
-      obj->client()->RenderClient();
+        obj->client()->RenderClient();
+      }
     }
   }
 }
diff --git a/o3d/plugin/npapi_host_control/build.scons b/o3d/plugin/npapi_host_control/build.scons
index 74597a6..a53383f 100644
--- a/o3d/plugin/npapi_host_control/build.scons
+++ b/o3d/plugin/npapi_host_control/build.scons
@@ -44,6 +44,9 @@ env.Append(
     'win',
     '$SCONSTRUCT_DIR/plugin/npapi_host_control/win',
   ],
+  LIBS = [
+    'wininet',
+  ],
   LINKFLAGS = [
     '/DEF:$SCONSTRUCT_DIR/plugin/npapi_host_control/win/npapi_host_control.def'
   ],
diff --git a/o3d/plugin/npapi_host_control/win/np_browser_proxy.cc b/o3d/plugin/npapi_host_control/win/np_browser_proxy.cc
index 8d93be9..968e71e 100644
--- a/o3d/plugin/npapi_host_control/win/np_browser_proxy.cc
+++ b/o3d/plugin/npapi_host_control/win/np_browser_proxy.cc
@@ -50,20 +50,20 @@ namespace {
 
 // Helper routine that invokes the host-control stream request function.
 NPError OpenURL(NPBrowserProxy* browser_proxy,
-                const char *szURL,
-                const char *szTarget,
-                void *pNotifyData) {
+                const char *url,
+                const char *target,
+                void *notify_data) {
   CHostControl* host_control = browser_proxy->GetHostingControl();
 
   USES_CONVERSION;
-  HRESULT hr = host_control->OpenUrlStream(A2CW(szURL), pNotifyData);
+  HRESULT hr = host_control->OpenUrlStream(A2CW(url), notify_data);
   return SUCCEEDED(hr) ? NPERR_NO_ERROR : NPERR_GENERIC_ERROR;
 }
 }  // unnamed namespace
 
 NPNetscapeFuncs NPBrowserProxy::kNetscapeFunctions = {
   sizeof(kNetscapeFunctions),
-  NP_VERSION_MAJOR << 8 | NP_VERSION_MINOR,
+  NPVERS_HAS_NPOBJECT_ENUM,
   NPN_GetURL,
   NPN_PostURL,
   NPN_RequestRead,
diff --git a/o3d/plugin/npapi_host_control/win/np_plugin_proxy.cc b/o3d/plugin/npapi_host_control/win/np_plugin_proxy.cc
index 449e6b7..8fb2ca3 100644
--- a/o3d/plugin/npapi_host_control/win/np_plugin_proxy.cc
+++ b/o3d/plugin/npapi_host_control/win/np_plugin_proxy.cc
@@ -279,13 +279,7 @@ bool NPPluginProxy::Init(NPBrowserProxy* browser_proxy,
   }
   ATLASSERT(np_object);
 
-  HRESULT hr = NPObjectProxy::CreateInstance(&scriptable_object_);
-  ATLASSERT(SUCCEEDED(hr));
-
-  scriptable_object_->SetBrowserProxy(browser_proxy_);
-  scriptable_object_->SetHostedObject(np_object);
-
-  browser_proxy_->RegisterNPObjectProxy(np_object, scriptable_object_);
+  scriptable_object_ = browser_proxy_->GetDispatchObject(np_object);
 
   NPBrowserProxy::GetBrowserFunctions()->releaseobject(np_object);
 
diff --git a/o3d/plugin/npapi_host_control/win/stream_operation.cc b/o3d/plugin/npapi_host_control/win/stream_operation.cc
index 31f1c5c..599c93d 100644
--- a/o3d/plugin/npapi_host_control/win/stream_operation.cc
+++ b/o3d/plugin/npapi_host_control/win/stream_operation.cc
@@ -38,6 +38,7 @@
  *
  * ***** END LICENSE BLOCK ***** */
 
+#include <wininet.h>
 
 #include "plugin/npapi_host_control/win/stream_operation.h"
 #include "plugin/npapi_host_control/win/host_control.h"
@@ -488,6 +489,16 @@ HRESULT STDMETHODCALLTYPE StreamOperation::OnObjectAvailable(REFIID riid,
 HRESULT StreamOperation::OpenURL(NPPluginProxy *owning_plugin,
                                  const wchar_t *url,
                                  void *notify_data) {
+  // Validate the URL. If the URL is invalid there is no need to create a new
+  // thread only to have it immediately fail.
+  HRESULT hr;
+  URL_COMPONENTS components = { sizeof(URL_COMPONENTS) };
+  if (!InternetCrackUrl(url, 0, 0, &components))
+    return E_INVALIDARG;
+  if (components.nScheme == INTERNET_SCHEME_UNKNOWN) {
+    return E_INVALIDARG;
+  }
+
   // The StreamOperation instance is created with a ref-count of zero,
   // so we explicitly attach a CComPtr to the object to boost the count, and
   // manage the lifetime of the object.
@@ -507,7 +518,6 @@ HRESULT StreamOperation::OpenURL(NPPluginProxy *owning_plugin,
   stream_object->SetOwner(owning_plugin);
 
   CString full_path;
-  HRESULT hr;
   if (FAILED(hr = ConstructFullURLPath(*stream_object,
                                        base_url_moniker,
                                        &full_path))) {
diff --git a/o3d/plugin/win/main_win.cc b/o3d/plugin/win/main_win.cc
index 48c1be1..d2727d7 100644
--- a/o3d/plugin/win/main_win.cc
+++ b/o3d/plugin/win/main_win.cc
@@ -512,7 +512,7 @@ LRESULT CALLBACK WindowProc(HWND hWnd, UINT Msg, WPARAM wParam, LPARAM lParam) {
             ::SetPixelV(hdc, 0, 0, RGB(0, 0, 0));
           }
 
-          obj->client()->RenderClient();
+          obj->renderer()->set_need_to_render(true);
         } else {
           // If there Client has no Renderer associated with it, paint the draw
           // area gray.
@@ -552,24 +552,22 @@ LRESULT CALLBACK WindowProc(HWND hWnd, UINT Msg, WPARAM wParam, LPARAM lParam) {
       if (reentrance_count.get() > 1) {
         break;  // Ignore this message; we're reentrant.
       }
-      // DoOnFrameCallback(obj);
 
       // TODO: Only logging for windows until we figure out the proper
       //            mac way
       if (g_logger) g_logger->UpdateLogging();
 
-      obj->client()->Tick();
-      if (obj->client()->render_mode() ==
-          o3d::Client::RENDERMODE_CONTINUOUS) {
-        // Must invalidate GetHWnd()'s drawing area, no matter which window is
-        // receiving this event.  It turns out that we have to set the timer on
-        // the window we're using for drawing anyway, whichever that is, but
-        // it's possible that an extra event will slip through.
-        ::InvalidateRect(obj->GetHWnd(), NULL, TRUE);
+      // If rendering continuously, invalidate the window and force a paint if
+      // it is visible. The paint invalidates the renderer and Tick will later
+      // repaint the window.
+      if (obj->client()->render_mode() == o3d::Client::RENDERMODE_CONTINUOUS) {
+        InvalidateRect(obj->GetHWnd(), NULL, FALSE);
+        reentrance_count.decrement();
+        UpdateWindow(obj->GetHWnd());
       }
-      // Calling UpdateWindow to force a WM_PAINT here causes problems in
-      // Firefox 2 if rendering takes too long. WM_PAINT will be sent anyway
-      // when there are no other messages to process.
+
+      obj->AsyncTick();
+
       break;
     }
     case WM_NCDESTROY: {
@@ -815,9 +813,6 @@ NPError OSCALL NP_Shutdown(void) {
 }  // extern "C" / namespace o3d
 
 namespace o3d {
-void RenderOnDemandCallbackHandler::Run() {
-  ::InvalidateRect(obj_->GetHWnd(), NULL, TRUE);
-}
 
 NPError NPP_New(NPMIMEType pluginType,
                 NPP instance,
@@ -912,8 +907,6 @@ NPError NPP_SetWindow(NPP instance, NPWindow *window) {
 
   obj->CreateRenderer(default_display);
   obj->client()->Init();
-  obj->client()->SetRenderOnDemandCallback(
-      new RenderOnDemandCallbackHandler(obj));
 
   // we set the timer to 10ms or 100fps. At the time of this comment
   // the renderer does a vsync the max fps it will run will be the refresh