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

17 files changed, 171 insertions, 168 deletions

diff --git a/o3d/core/cross/client.cc b/o3d/core/cross/client.cc
index e494f5d..e0fec9e 100644
--- a/o3d/core/cross/client.cc
+++ b/o3d/core/cross/client.cc
@@ -372,18 +372,11 @@ void Client::ClearPostRenderCallback() {
   post_render_callback_manager_.Clear();
 }
 
-void Client::SetRenderOnDemandCallback(
-    RenderOnDemandCallback* render_on_demand_callback) {
-  render_on_demand_callback_manager_.Set(render_on_demand_callback);
-}
-
-void Client::ClearRenderOnDemandCallback() {
-  render_on_demand_callback_manager_.Clear();
-}
-
 void Client::Render() {
   if (render_mode() == RENDERMODE_ON_DEMAND) {
-    render_on_demand_callback_manager_.Run();
+    if (renderer_.IsAvailable()) {
+      renderer_->set_need_to_render(true);
+    }
   }
 }
 
diff --git a/o3d/core/cross/client.h b/o3d/core/cross/client.h
index a41eeaa..0f20172 100644
--- a/o3d/core/cross/client.h
+++ b/o3d/core/cross/client.h
@@ -139,33 +139,12 @@ class Client {
                             //    (like uncovering part of a window.)
   };
 
-  typedef NonRecursiveClosureManager RenderOnDemandCallbackManager;
-  typedef RenderOnDemandCallbackManager::ClosureType RenderOnDemandCallback;
-
   RenderMode render_mode() const {
     return render_mode_;
   }
 
   void set_render_mode(RenderMode render_mode);
 
-  // Sets a callback for when the Client::Render() is called and the render
-  // mode is RENDERMODE_ON_DEMAND.
-  // NOTE: The client takes ownership of the RenderOnDemandCallback you
-  // pass in. It will be deleted if you call SetRenderOnDemandCallback a second
-  // time or if you call ClearRenderOnDemandCallback
-  //
-  // Parameters:
-  //   render_on_demand_callback: RenderOnDemandCallback to call when the
-  //        Client::Render is called.
-  void SetRenderOnDemandCallback(
-      RenderOnDemandCallback* render_on_demand_callback);
-
-  // Clears the render on demand callback.
-  // NOTE: The client takes ownership of the RenderOnDemandCallback you
-  // pass in. It will be deleted if you call SetRenderOnDemandCallback a second
-  // time or if you call ClearRenderOnDemandCallback
-  void ClearRenderOnDemandCallback();
-
   // Returns the rendergraph root render node.
   // Parameters:
   //  None.
@@ -476,9 +455,6 @@ class Client {
 
   RenderCallbackManager post_render_callback_manager_;
 
-  // Render On Demand Callback.
-  RenderOnDemandCallbackManager render_on_demand_callback_manager_;
-
   // Render Event to pass to the render callback.
   RenderEvent render_event_;
 
diff --git a/o3d/core/cross/gl/renderer_gl.cc b/o3d/core/cross/gl/renderer_gl.cc
index f18833b..7033cbc 100644
--- a/o3d/core/cross/gl/renderer_gl.cc
+++ b/o3d/core/cross/gl/renderer_gl.cc
@@ -1159,6 +1159,7 @@ void RendererGL::Clear(const Float4 &color,
             (depth_flag   ? GL_DEPTH_BUFFER_BIT   : 0) |
             (stencil_flag ? GL_STENCIL_BUFFER_BIT : 0));
   CHECK_GL_ERROR();
+  set_need_to_render(false);
 }
 
 // Updates the helper constant used for the D3D -> GL remapping.
@@ -1256,6 +1257,8 @@ bool RendererGL::StartRendering() {
 // Returns true on success.
 bool RendererGL::BeginDraw() {
   DLOG_FIRST_N(INFO, 10) << "RendererGL BeginDraw";
+  set_need_to_render(true);
+
   MakeCurrentLazy();
 
   // Reset the viewport.
@@ -1319,10 +1322,14 @@ void RendererGL::EndDraw() {
   DLOG_FIRST_N(INFO, 10) << "RendererGL EndDraw";
   DCHECK(IsCurrent());
   SetChangedStates();
+  set_need_to_render(false);
 }
 
 // Swaps the buffers.
 void RendererGL::FinishRendering() {
+  if (need_to_render())
+    return;
+
   DLOG_FIRST_N(INFO, 10) << "RendererGL Present";
   DCHECK(IsCurrent());
   SetChangedStates();
@@ -1331,6 +1338,13 @@ void RendererGL::FinishRendering() {
 #ifdef OS_WIN
   ::SwapBuffers(device_context_);
 #endif
+#ifdef OS_MACOSX
+#ifdef USE_AGL_DOUBLE_BUFFER
+  if (mac_agl_context_) {
+    ::aglSwapBuffers(mac_agl_context_);
+  }
+#endif
+#endif
 #ifdef OS_LINUX
   ::glXSwapBuffers(display_, window_);
 #endif
diff --git a/o3d/core/cross/renderer.cc b/o3d/core/cross/renderer.cc
index e754f19..86407e6 100644
--- a/o3d/core/cross/renderer.cc
+++ b/o3d/core/cross/renderer.cc
@@ -98,6 +98,7 @@ Renderer::Renderer(ServiceLocator* service_locator)
       features_(service_locator),
       supports_npot_(false),
       clear_client_(true),
+      need_to_render_(true),
       current_render_surface_(NULL),
       current_depth_surface_(NULL),
       render_frame_count_(0),
diff --git a/o3d/core/cross/renderer.h b/o3d/core/cross/renderer.h
index 55ec2be..52d2148 100644
--- a/o3d/core/cross/renderer.h
+++ b/o3d/core/cross/renderer.h
@@ -178,6 +178,16 @@ class Renderer {
   // Presents the results of the draw calls for this frame.
   virtual void FinishRendering() = 0;
 
+  // Returns whether a render is required.
+  bool need_to_render() const {
+    return need_to_render_;
+  }
+
+  // Invalidate the last rendered frame.
+  void set_need_to_render(bool need_to_render) {
+    need_to_render_ = need_to_render;
+  }
+
   // Handles the plugin resize event.
   virtual void Resize(int width, int height) = 0;
 
@@ -558,6 +568,9 @@ class Renderer {
   // Whether we need to clear the entire client area next render.
   bool clear_client_;
 
+  // Whether a render is required.
+  bool need_to_render_;
+
   // The current render surfaces. NULL = no surface.
   RenderSurface* current_render_surface_;
   RenderDepthStencilSurface* current_depth_surface_;
diff --git a/o3d/core/win/d3d9/renderer_d3d9.cc b/o3d/core/win/d3d9/renderer_d3d9.cc
index aaf920e..a2b9470 100644
--- a/o3d/core/win/d3d9/renderer_d3d9.cc
+++ b/o3d/core/win/d3d9/renderer_d3d9.cc
@@ -338,9 +338,9 @@ bool ForceAntiAliasingOff(LPDIRECT3D9* d3d) {
 
   unsigned int vendor_id = identifier.VendorId;
   unsigned int device_id = identifier.DeviceId;
-  unsigned int product = HIWORD(identifier.DriverVersion.HighPart);

-  unsigned int version = LOWORD(identifier.DriverVersion.HighPart);

-  unsigned int subversion = HIWORD(identifier.DriverVersion.LowPart);

+  unsigned int product = HIWORD(identifier.DriverVersion.HighPart);
+  unsigned int version = LOWORD(identifier.DriverVersion.HighPart);
+  unsigned int subversion = HIWORD(identifier.DriverVersion.LowPart);
   unsigned int build = LOWORD(identifier.DriverVersion.LowPart);
 
   // Disable ATI drivers 6.14.10.x where x is 6800 or lower.
@@ -1091,6 +1091,7 @@ void RendererD3D9::Clear(const Float4 &color,
                              color[3]),
          depth,
          stencil));
+  set_need_to_render(false);
 }
 
 void RendererD3D9::SetViewportInPixels(int left,
@@ -1251,6 +1252,7 @@ bool RendererD3D9::RestoreDeviceObjects() {
 bool RendererD3D9::ResetDevice() {
   // First update the flag if it hasn't been set yet.
   have_device_ = false;
+  set_need_to_render(true);
 
   // Try to release all resources
   if (!InvalidateDeviceObjects())
@@ -1268,6 +1270,7 @@ bool RendererD3D9::ResetDevice() {
   SetInitialStates();
 
   // successful
+  have_device_ = true;
   return true;
 }
 
@@ -1293,23 +1296,21 @@ void RendererD3D9::TestLostDevice() {
   // In this case, we attempt to invalidate all resources in D3DPOOL_DEFAULT,
   // reset the device, and then restore the resources.
   // This should succeed and we set the have_device_ flag to true.
-  // If it fails, we do not set the flag to true so as
+  // If it fails, we do not set the flag to true.
 
   if (hr == D3DERR_DEVICELOST) {
     // We've lost the device, update the flag so that render calls don't
     // get called.
     have_device_ = false;
+    set_need_to_render(true);
     return;
   } else if (hr == D3DERR_DEVICENOTRESET) {
     // Direct3d tells us it is possible to reset the device now..
     // So let's attempt a reset!
-    ResetDevice();
+    if (ResetDevice()) {
+      lost_resources_callback_manager_.Run();
+    }
   }
-
-  // TestCooperativeLevel doesn't give us a device lost error or variant
-  // or the device has been successfully reset and reinitialized.
-  // We can safely set our have_device_ flag to true.
-  have_device_ = true;
 }
 
 // The window has been resized; change the size of our back buffer
@@ -1426,9 +1427,8 @@ bool RendererD3D9::StartRendering() {
   draw_elements_rendered_ = 0;
   primitives_rendered_ = 0;
 
-  // Attempt to reset the device if it is lost.
-  if (!have_device_)
-    TestLostDevice();
+  // Determine whether the device is lost, resetting if possible.
+  TestLostDevice();
 
   // Only perform ops with the device if we have it.
   if (have_device_) {
@@ -1448,6 +1448,7 @@ bool RendererD3D9::StartRendering() {
 bool RendererD3D9::BeginDraw() {
   // Only perform ops with the device if we have it.
   if (have_device_) {
+    set_need_to_render(true);
     if (!HR(d3d_device_->GetRenderTarget(0, &back_buffer_surface_)))
       return false;
     if (!HR(d3d_device_->GetDepthStencilSurface(&back_buffer_depth_surface_)))
@@ -1539,6 +1540,8 @@ void RendererD3D9::EndDraw() {
     }
     HR(d3d_device_->EndScene());
 
+    set_need_to_render(false);
+
     // Release the back-buffer references.
     back_buffer_surface_ = NULL;
     back_buffer_depth_surface_ = NULL;
@@ -1548,16 +1551,8 @@ void RendererD3D9::EndDraw() {
 void RendererD3D9::FinishRendering() {
   // No need to call Present(...) if we are rendering to an off-screen
   // target.
-  if (!off_screen_surface_) {
-    HRESULT hr = d3d_device_->Present(NULL, NULL, NULL, NULL);
-    // Test for lost device if Present fails.
-    if (hr != D3D_OK) {
-      TestLostDevice();
-      // TODO: This should only be called if some resources were
-      //    actually lost. In other words if there are no RenderSurfaces
-      //    then there is no reason to call this.
-      lost_resources_callback_manager_.Run();
-    }
+  if (have_device_ && !off_screen_surface_ && !need_to_render()) {
+    d3d_device_->Present(NULL, NULL, NULL, NULL);
   }
 }
 
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