Fix for http://code.google.com/p/chromium/issues/detail?id=5409.

A case study on why constants are bad.  The meaning of V8 NPObject wrapper
constants got rearranged, and we failed to detect them properly, resulting in
doubly-wrapped NPObjects.

BUG=5409
Review URL: http://codereview.chromium.org/14031

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

4 files changed, 16 insertions, 10 deletions

diff --git a/webkit/port/bindings/v8/np_v8object.cpp b/webkit/port/bindings/v8/np_v8object.cpp
index 7856fec..6828733 100644
--- a/webkit/port/bindings/v8/np_v8object.cpp
+++ b/webkit/port/bindings/v8/np_v8object.cpp
@@ -38,12 +38,14 @@
 #include "npruntime_priv.h"
 #include "PlatformString.h"
 #include "ScriptController.h"
+#include "v8_custom.h"
 #include "v8_helpers.h"
 #include "v8_np_utils.h"
 #include "v8_proxy.h"
 #include "DOMWindow.h"
 
 using WebCore::V8ClassIndex;
+using WebCore::V8Custom;
 using WebCore::V8Proxy;
 
 namespace {
@@ -100,9 +102,10 @@ NPClass* NPScriptObjectClass = &V8NPObjectClass;
 NPObject* NPN_CreateScriptObject(NPP npp, v8::Handle<v8::Object> object,
                                  WebCore::DOMWindow* root) {
   // Check to see if this object is already wrapped.
-  if (object->InternalFieldCount() == 3 &&
-      object->GetInternalField(1)->IsNumber() &&
-      object->GetInternalField(1)->Uint32Value() == V8ClassIndex::NPOBJECT) {
+  if (object->InternalFieldCount() == V8Custom::kNPObjectInternalFieldCount &&
+      object->GetInternalField(V8Custom::kDOMWrapperTypeIndex)->IsNumber() &&
+      object->GetInternalField(V8Custom::kDOMWrapperTypeIndex)->Uint32Value() ==
+          V8ClassIndex::NPOBJECT) {
     NPObject* rv = V8Proxy::ToNativeObject<NPObject>(V8ClassIndex::NPOBJECT,
                                                      object);
     NPN_RetainObject(rv);
diff --git a/webkit/port/bindings/v8/v8_custom.h b/webkit/port/bindings/v8/v8_custom.h
index 64a2351..08ea4bc 100644
--- a/webkit/port/bindings/v8/v8_custom.h
+++ b/webkit/port/bindings/v8/v8_custom.h
@@ -37,6 +37,9 @@ class V8Custom {
   static const int kDOMWrapperObjectIndex = 1;
   static const int kDefaultWrapperInternalFieldCount = 2;
 
+  static const int kNPObjectInternalFieldCount =
+                      kDefaultWrapperInternalFieldCount + 0;
+
   static const int kDocumentImplementationIndex =
                       kDefaultWrapperInternalFieldCount + 0;
   static const int kDocumentMinimumInternalFieldCount =
diff --git a/webkit/port/bindings/v8/v8_helpers.cpp b/webkit/port/bindings/v8/v8_helpers.cpp
index 466f810..be820c2 100644
--- a/webkit/port/bindings/v8/v8_helpers.cpp
+++ b/webkit/port/bindings/v8/v8_helpers.cpp
@@ -38,13 +38,10 @@
 
 #include "DOMWindow.h"
 
-void WrapNPObject(v8::Handle<v8::Object> obj, NPObject* npobj) {
-  ASSERT(obj->InternalFieldCount() >= 3);
+using WebCore::V8Custom;
 
+void WrapNPObject(v8::Handle<v8::Object> obj, NPObject* npobj) {
   WebCore::V8Proxy::SetDOMWrapper(obj, WebCore::V8ClassIndex::NPOBJECT, npobj);
-
-  // Create a JS object as a hash map for functions
-  obj->SetInternalField(2, v8::Object::New());
 }
 
 v8::Local<v8::Context> GetV8Context(NPP npp, NPObject* npobj) {
diff --git a/webkit/port/bindings/v8/v8_npobject.cpp b/webkit/port/bindings/v8/v8_npobject.cpp
index f26f510..4c43251 100644
--- a/webkit/port/bindings/v8/v8_npobject.cpp
+++ b/webkit/port/bindings/v8/v8_npobject.cpp
@@ -29,6 +29,7 @@
 
 #include "config.h"
 
+#include "v8_custom.h"
 #include "v8_helpers.h"
 #include "v8_npobject.h"
 #include "v8_np_utils.h"
@@ -72,7 +73,8 @@ static v8::Handle<v8::Value> NPObjectInvokeImpl(
   } else {
     // The holder object is not a subtype of HTMLPlugInElement, it
     // must be an NPObject which has three internal fields.
-    if (args.Holder()->InternalFieldCount() != 3) {
+    if (args.Holder()->InternalFieldCount() !=
+            V8Custom::kNPObjectInternalFieldCount) {
       V8Proxy::ThrowError(V8Proxy::REFERENCE_ERROR,
                           "NPMethod called on non-NPObject");
       return v8::Undefined();
@@ -329,7 +331,8 @@ v8::Local<v8::Object> CreateV8ObjectForNPObject(NPObject* object,
   if (np_object_desc.IsEmpty()) {
     np_object_desc =
         v8::Persistent<v8::FunctionTemplate>::New(v8::FunctionTemplate::New());
-    np_object_desc->InstanceTemplate()->SetInternalFieldCount(3);
+    np_object_desc->InstanceTemplate()->SetInternalFieldCount(
+        V8Custom::kNPObjectInternalFieldCount);
     np_object_desc->InstanceTemplate()->SetNamedPropertyHandler(
         NPObjectNamedPropertyGetter, NPObjectNamedPropertySetter);
     np_object_desc->InstanceTemplate()->SetIndexedPropertyHandler(