Revert my SetWindow(NULL) change in r9653.

I am told we don't want to change any NPAPI behavior on Windows, and that we should try to match Safari's behavior.

This change is neccessary for plugins on Linux, so the code will have to be forked and this patch reapplied.

This is a little more than a plain revert, since r9692 moved around some code.

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


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

3 files changed, 25 insertions, 13 deletions

diff --git a/webkit/glue/plugins/webplugin_delegate_impl.cc b/webkit/glue/plugins/webplugin_delegate_impl.cc
index bb3d527..1c990d6 100644
--- a/webkit/glue/plugins/webplugin_delegate_impl.cc
+++ b/webkit/glue/plugins/webplugin_delegate_impl.cc
@@ -184,6 +184,9 @@ WebPluginDelegateImpl::WebPluginDelegateImpl(
     quirks_ |= PLUGIN_QUIRK_DONT_CALL_WND_PROC_RECURSIVELY;
   } else if (plugin_info.name.find(L"VLC Multimedia Plugin") !=
              std::wstring::npos) {
+    // VLC hangs on NPP_Destroy if we call NPP_SetWindow with a null window
+    // handle
+    quirks_ |= PLUGIN_QUIRK_DONT_SET_NULL_WINDOW_HANDLE_ON_DESTROY;
     // VLC 0.8.6d and 0.8.6e crash if multiple instances are created.
     quirks_ |= PLUGIN_QUIRK_DONT_ALLOW_MULTIPLE_INSTANCES;
   } else if (filename == "npctrl.dll") {
@@ -301,11 +304,10 @@ void WebPluginDelegateImpl::DestroyInstance() {
     // instance uses the helper to do the download.
     instance_->CloseStreams();
 
-    // The MDC documentation suggests that you should call SetWindow(NULL) or
-    // SetWindow(window->window = NULL) when you destroy an instance.  However,
-    // Firefox does not do this, and hence many plugins don't expect it.  This
-    // leads to NULL pointer deference crashes, etc.  We will have to follow
-    // Firefox here and not call SetWindow during destruction.
+    window_.window = NULL;
+    if (!(quirks_ & PLUGIN_QUIRK_DONT_SET_NULL_WINDOW_HANDLE_ON_DESTROY)) {
+      instance_->NPP_SetWindow(&window_);
+    }
 
     instance_->NPP_Destroy();
 
diff --git a/webkit/glue/plugins/webplugin_delegate_impl_gtk.cc b/webkit/glue/plugins/webplugin_delegate_impl_gtk.cc
index 4236c25..cde9502 100644
--- a/webkit/glue/plugins/webplugin_delegate_impl_gtk.cc
+++ b/webkit/glue/plugins/webplugin_delegate_impl_gtk.cc
@@ -132,6 +132,15 @@ void WebPluginDelegateImpl::DestroyInstance() {
     // instance uses the helper to do the download.
     instance_->CloseStreams();
 
+    // TODO(evanm): I played with this for quite a while but couldn't
+    // figure out a way to make Flash not crash unless I didn't call
+    // NPP_SetWindow.  Perhaps it just should be marked with the quirk
+    // that wraps the NPP_SetWindow call.
+    // window_.window = NULL;
+    // if (!(quirks_ & PLUGIN_QUIRK_DONT_SET_NULL_WINDOW_HANDLE_ON_DESTROY)) {
+    //   instance_->NPP_SetWindow(&window_);
+    // }
+
     instance_->NPP_Destroy();
 
     instance_->set_web_plugin(NULL);
diff --git a/webkit/glue/webplugin_delegate.h b/webkit/glue/webplugin_delegate.h
index b1a17cf..254d94f 100644
--- a/webkit/glue/webplugin_delegate.h
+++ b/webkit/glue/webplugin_delegate.h
@@ -29,14 +29,15 @@ class WebPluginDelegate {
  public:
 #if defined(OS_WIN)
   enum PluginQuirks {
-    PLUGIN_QUIRK_SETWINDOW_TWICE = 1 << 0,
-    PLUGIN_QUIRK_THROTTLE_WM_USER_PLUS_ONE = 1 << 1,
-    PLUGIN_QUIRK_DONT_CALL_WND_PROC_RECURSIVELY = 1 << 2,
-    PLUGIN_QUIRK_DONT_ALLOW_MULTIPLE_INSTANCES = 1 << 3,
-    PLUGIN_QUIRK_DIE_AFTER_UNLOAD = 1 << 4,
-    PLUGIN_QUIRK_PATCH_TRACKPOPUP_MENU = 1 << 5,
-    PLUGIN_QUIRK_PATCH_SETCURSOR = 1 << 6,
-    PLUGIN_QUIRK_BLOCK_NONSTANDARD_GETURL_REQUESTS = 1 << 7,
+    PLUGIN_QUIRK_SETWINDOW_TWICE = 1,
+    PLUGIN_QUIRK_THROTTLE_WM_USER_PLUS_ONE = 2,
+    PLUGIN_QUIRK_DONT_CALL_WND_PROC_RECURSIVELY = 4,
+    PLUGIN_QUIRK_DONT_SET_NULL_WINDOW_HANDLE_ON_DESTROY = 8,
+    PLUGIN_QUIRK_DONT_ALLOW_MULTIPLE_INSTANCES = 16,
+    PLUGIN_QUIRK_DIE_AFTER_UNLOAD = 32,
+    PLUGIN_QUIRK_PATCH_TRACKPOPUP_MENU = 64,
+    PLUGIN_QUIRK_PATCH_SETCURSOR = 128,
+    PLUGIN_QUIRK_BLOCK_NONSTANDARD_GETURL_REQUESTS = 256,
   };
 #endif