Match Firefox by not calling SetWindow(NULL) on plugin destruction.

This matches the behavior of Firefox on Windows and Linux.  This change is especially important for plugin compatability on Linux.  This also nicely removes a quirk mode.

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

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

3 files changed, 13 insertions, 25 deletions

diff --git a/webkit/glue/plugins/webplugin_delegate_impl.cc b/webkit/glue/plugins/webplugin_delegate_impl.cc
index b024bac..839cbd5 100644
--- a/webkit/glue/plugins/webplugin_delegate_impl.cc
+++ b/webkit/glue/plugins/webplugin_delegate_impl.cc
@@ -184,9 +184,6 @@ 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") {
@@ -304,10 +301,11 @@ void WebPluginDelegateImpl::DestroyInstance() {
     // instance uses the helper to do the download.
     instance_->CloseStreams();
 
-    window_.window = NULL;
-    if (!(quirks_ & PLUGIN_QUIRK_DONT_SET_NULL_WINDOW_HANDLE_ON_DESTROY)) {
-      instance_->NPP_SetWindow(&window_);
-    }
+    // 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.
 
     instance_->NPP_Destroy();
 
diff --git a/webkit/glue/plugins/webplugin_delegate_impl.h b/webkit/glue/plugins/webplugin_delegate_impl.h
index 68ca12e..f196c6e 100644
--- a/webkit/glue/plugins/webplugin_delegate_impl.h
+++ b/webkit/glue/plugins/webplugin_delegate_impl.h
@@ -88,15 +88,14 @@ class WebPluginDelegateImpl : public WebPluginDelegate {
 
 #if defined(OS_WIN)
   enum PluginQuirks {
-    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,
+    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,
   };
 #endif
 
diff --git a/webkit/glue/plugins/webplugin_delegate_impl_gtk.cc b/webkit/glue/plugins/webplugin_delegate_impl_gtk.cc
index 58207c6..58d853e 100644
--- a/webkit/glue/plugins/webplugin_delegate_impl_gtk.cc
+++ b/webkit/glue/plugins/webplugin_delegate_impl_gtk.cc
@@ -132,15 +132,6 @@ 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);