bug 723510 - Workers: double-error reporting in location.toString and incorrect assumption about JS_GetInstancePrivate. r=bent

dom/workers/EventTarget.cpp

@@ -73,15 +73,13 @@ bool
 EnsureObjectIsEventTarget(JSContext* aCx, JSObject* aObj, char* aFunctionName)
 {
   JSClass* classPtr = JS_GET_CLASS(aCx, aObj);
-  if (classPtr &&
-      (ClassIsWorker(classPtr) || ClassIsWorkerGlobalScope(classPtr) ||
-       ClassIsXMLHttpRequest(classPtr))) {
+  if (ClassIsWorker(classPtr) || ClassIsWorkerGlobalScope(classPtr) ||
+      ClassIsXMLHttpRequest(classPtr)) {
     return true;
   }
 
   JS_ReportErrorNumber(aCx, js_GetErrorMessage, NULL, JSMSG_INCOMPATIBLE_PROTO,
-                       "EventTarget", aFunctionName,
-                        classPtr ? classPtr->name : "object");
+                       "EventTarget", aFunctionName, classPtr->name);
   return false;
 }
 

dom/workers/Events.cpp

@@ -214,20 +214,13 @@ protected:
   static Event*
   GetInstancePrivate(JSContext* aCx, JSObject* aObj, const char* aFunctionName)
   {
-    JSClass* classPtr = NULL;
-
-    if (aObj) {
-      Event* priv = GetPrivate(aCx, aObj);
-      if (priv) {
-        return priv;
-      }
-
-      classPtr = JS_GET_CLASS(aCx, aObj);
+    Event* priv = GetPrivate(aCx, aObj);
+    if (priv) {
+      return priv;
     }
-
     JS_ReportErrorNumber(aCx, js_GetErrorMessage, NULL,
                          JSMSG_INCOMPATIBLE_PROTO, sClass.name, aFunctionName,
-                         classPtr ? classPtr->name : "object");
+                         JS_GET_CLASS(aCx, aObj)->name);
     return NULL;
   }
 
@@ -533,20 +526,14 @@ private:
   static MessageEvent*
   GetInstancePrivate(JSContext* aCx, JSObject* aObj, const char* aFunctionName)
   {
-    // JS_GetInstancePrivate is ok to be called with a null aObj, so this should
-    // be too.
-    JSClass* classPtr = NULL;
-
-    if (aObj) {
-      classPtr = JS_GET_CLASS(aCx, aObj);
-      if (IsThisClass(classPtr)) {
-        return GetJSPrivateSafeish<MessageEvent>(aCx, aObj);
-      }
+    JSClass* classPtr = JS_GET_CLASS(aCx, aObj);
+    if (IsThisClass(classPtr)) {
+      return GetJSPrivateSafeish<MessageEvent>(aCx, aObj);
     }
 
     JS_ReportErrorNumber(aCx, js_GetErrorMessage, NULL,
                          JSMSG_INCOMPATIBLE_PROTO, sClass.name, aFunctionName,
-                         classPtr ? classPtr->name : "object");
+                         classPtr->name);
     return NULL;
   }
 
@@ -764,20 +751,14 @@ private:
   static ErrorEvent*
   GetInstancePrivate(JSContext* aCx, JSObject* aObj, const char* aFunctionName)
   {
-    // JS_GetInstancePrivate is ok to be called with a null aObj, so this should
-    // be too.
-    JSClass* classPtr = NULL;
-
-    if (aObj) {
-      classPtr = JS_GET_CLASS(aCx, aObj);
-      if (IsThisClass(classPtr)) {
-        return GetJSPrivateSafeish<ErrorEvent>(aCx, aObj);
-      }
+    JSClass* classPtr = JS_GET_CLASS(aCx, aObj);
+    if (IsThisClass(classPtr)) {
+      return GetJSPrivateSafeish<ErrorEvent>(aCx, aObj);
     }
 
     JS_ReportErrorNumber(aCx, js_GetErrorMessage, NULL,
                          JSMSG_INCOMPATIBLE_PROTO, sClass.name, aFunctionName,
-                         classPtr ? classPtr->name : "object");
+                         classPtr->name);
     return NULL;
   }
 
@@ -958,18 +939,14 @@ private:
   static ProgressEvent*
   GetInstancePrivate(JSContext* aCx, JSObject* aObj, const char* aFunctionName)
   {
-    JSClass* classPtr = NULL;
-
-    if (aObj) {
-      classPtr = JS_GET_CLASS(aCx, aObj);
-      if (classPtr == &sClass) {
-        return GetJSPrivateSafeish<ProgressEvent>(aCx, aObj);
-      }
+    JSClass* classPtr = JS_GET_CLASS(aCx, aObj);
+    if (classPtr == &sClass) {
+      return GetJSPrivateSafeish<ProgressEvent>(aCx, aObj);
     }
 
     JS_ReportErrorNumber(aCx, js_GetErrorMessage, NULL,
                          JSMSG_INCOMPATIBLE_PROTO, sClass.name, aFunctionName,
-                         classPtr ? classPtr->name : "object");
+                         classPtr->name);
     return NULL;
   }
 

dom/workers/Exceptions.cpp

@@ -125,11 +125,11 @@ private:
       return false;
     }
 
-    JSClass* classPtr;
-    if (!obj || ((classPtr = JS_GET_CLASS(aCx, obj)) != &sClass)) {
+    JSClass* classPtr = JS_GET_CLASS(aCx, obj);
+    if (classPtr != &sClass) {
       JS_ReportErrorNumber(aCx, js_GetErrorMessage, NULL,
                            JSMSG_INCOMPATIBLE_PROTO, sClass.name, "toString",
-                           classPtr ? classPtr->name : "object");
+                           classPtr->name);
       return false;
     }
 
@@ -166,12 +166,10 @@ private:
 
     JSClass* classPtr = JS_GET_CLASS(aCx, aObj);
 
-    if (classPtr != &sClass ||
-        !GetJSPrivateSafeish<DOMException>(aCx, aObj)) {
+    if (classPtr != &sClass || !GetJSPrivateSafeish<DOMException>(aCx, aObj)) {
       JS_ReportErrorNumber(aCx, js_GetErrorMessage, NULL,
                            JSMSG_INCOMPATIBLE_PROTO, sClass.name,
-                           sProperties[slot].name,
-                           classPtr ? classPtr->name : "object");
+                           sProperties[slot].name, classPtr->name);
       return false;
     }
 
@@ -344,12 +342,10 @@ private:
 
     JSClass* classPtr = JS_GET_CLASS(aCx, aObj);
 
-    if (classPtr != &sClass ||
-        !GetJSPrivateSafeish<FileException>(aCx, aObj)) {
+    if (classPtr != &sClass || !GetJSPrivateSafeish<FileException>(aCx, aObj)) {
       JS_ReportErrorNumber(aCx, js_GetErrorMessage, NULL,
                            JSMSG_INCOMPATIBLE_PROTO, sClass.name,
-                           sProperties[slot].name,
-                           classPtr ? classPtr->name : "object");
+                           sProperties[slot].name, classPtr->name);
       return false;
     }
 

dom/workers/File.cpp

@@ -101,20 +101,14 @@ private:
   static nsIDOMBlob*
   GetInstancePrivate(JSContext* aCx, JSObject* aObj, const char* aFunctionName)
   {
-    JSClass* classPtr = NULL;
-
-    if (aObj) {
-      nsIDOMBlob* blob = GetPrivate(aCx, aObj);
-      if (blob) {
-        return blob;
-      }
-
-      classPtr = JS_GET_CLASS(aCx, aObj);
+    nsIDOMBlob* blob = GetPrivate(aCx, aObj);
+    if (blob) {
+      return blob;
     }
 
     JS_ReportErrorNumber(aCx, js_GetErrorMessage, NULL,
                          JSMSG_INCOMPATIBLE_PROTO, sClass.name, aFunctionName,
-                         classPtr ? classPtr->name : "Object");
+                         JS_GET_CLASS(aCx, aObj)->name);
     return NULL;
   }
 
@@ -299,19 +293,14 @@ private:
   static nsIDOMFile*
   GetInstancePrivate(JSContext* aCx, JSObject* aObj, const char* aFunctionName)
   {
-    JSClass* classPtr = NULL;
-
-    if (aObj) {
-      nsIDOMFile* file = GetPrivate(aCx, aObj);
-      if (file) {
-        return file;
-      }
-      classPtr = JS_GET_CLASS(aCx, aObj);
+    nsIDOMFile* file = GetPrivate(aCx, aObj);
+    if (file) {
+      return file;
     }
 
     JS_ReportErrorNumber(aCx, js_GetErrorMessage, NULL,
                          JSMSG_INCOMPATIBLE_PROTO, sClass.name, aFunctionName,
-                         classPtr ? classPtr->name : "Object");
+                         JS_GET_CLASS(aCx, aObj)->name);
     return NULL;
   }
 

dom/workers/FileReaderSync.cpp

@@ -78,19 +78,17 @@ EnsureSucceededOrThrow(JSContext* aCx, nsresult rv)
 
 inline nsIDOMBlob*
 GetDOMBlobFromJSObject(JSContext* aCx, JSObject* aObj) {
-  JSClass* classPtr = NULL;
-
+  // aObj can be null as JS_ConvertArguments("o") successfully converts JS
+  // null to a null pointer to JSObject 
   if (aObj) {
     nsIDOMBlob* blob = file::GetDOMBlobFromJSObject(aCx, aObj);
     if (blob) {
       return blob;
     }
-
-    classPtr = JS_GET_CLASS(aCx, aObj);
   }
 
   JS_ReportErrorNumber(aCx, js_GetErrorMessage, NULL, JSMSG_UNEXPECTED_TYPE,
-                       classPtr ? classPtr->name : "Object", "not a Blob.");
+                       aObj ? JS_GET_CLASS(aCx, aObj)->name : "Object", "not a Blob.");
   return NULL;
 }
 
@@ -129,20 +127,14 @@ private:
   static FileReaderSyncPrivate*
   GetInstancePrivate(JSContext* aCx, JSObject* aObj, const char* aFunctionName)
   {
-    JSClass* classPtr = NULL;
-
-    if (aObj) {
-      FileReaderSyncPrivate* fileReader = GetPrivate(aCx, aObj);
-      if (fileReader) {
-        return fileReader;
-      }
-
-      classPtr = JS_GET_CLASS(aCx, aObj);
+    FileReaderSyncPrivate* fileReader = GetPrivate(aCx, aObj);
+    if (fileReader) {
+      return fileReader;
     }
 
     JS_ReportErrorNumber(aCx, js_GetErrorMessage, NULL,
                          JSMSG_INCOMPATIBLE_PROTO, sClass.name, aFunctionName,
-                         classPtr ? classPtr->name : "Object");
+                         JS_GET_CLASS(aCx, aObj)->name);
     return NULL;
   }
 

dom/workers/Location.cpp

@@ -148,12 +148,15 @@ private:
   ToString(JSContext* aCx, uintN aArgc, jsval* aVp)
   {
     JSObject* obj = JS_THIS_OBJECT(aCx, aVp);
+    if (!obj) {
+      return false;
+    }
 
-    JSClass* classPtr;
-    if (!obj || ((classPtr = JS_GET_CLASS(aCx, obj)) != &sClass)) {
+    JSClass* classPtr = JS_GET_CLASS(aCx, obj);
+    if (classPtr != &sClass) {
       JS_ReportErrorNumber(aCx, js_GetErrorMessage, NULL,
                            JSMSG_INCOMPATIBLE_PROTO, sClass.name, "toString",
-                           classPtr ? classPtr->name : "object");
+                           classPtr);
       return false;
     }
 
@@ -170,11 +173,11 @@ private:
   static JSBool
   GetProperty(JSContext* aCx, JSObject* aObj, jsid aIdval, jsval* aVp)
   {
-    JSClass* classPtr;
-    if (!aObj || ((classPtr = JS_GET_CLASS(aCx, aObj)) != &sClass)) {
+    JSClass* classPtr = JS_GET_CLASS(aCx, aObj);
+    if (classPtr != &sClass) {
       JS_ReportErrorNumber(aCx, js_GetErrorMessage, NULL,
                            JSMSG_INCOMPATIBLE_PROTO, sClass.name, "GetProperty",
-                           classPtr ? classPtr->name : "object");
+                           classPtr->name);
       return false;
     }
 

dom/workers/Navigator.cpp

@@ -158,11 +158,11 @@ private:
   static JSBool
   GetProperty(JSContext* aCx, JSObject* aObj, jsid aIdval, jsval* aVp)
   {
-    JSClass* classPtr;
-    if (!aObj || ((classPtr = JS_GET_CLASS(aCx, aObj)) != &sClass)) {
+    JSClass* classPtr = JS_GET_CLASS(aCx, aObj);
+    if (classPtr != &sClass) {
       JS_ReportErrorNumber(aCx, js_GetErrorMessage, NULL,
                            JSMSG_INCOMPATIBLE_PROTO, sClass.name, "GetProperty",
-                           classPtr ? classPtr->name : "object");
+                           classPtr->name);
       return false;
     }
 

dom/workers/Worker.cpp

@@ -427,18 +427,13 @@ WorkerPrivate*
 Worker::GetInstancePrivate(JSContext* aCx, JSObject* aObj,
                            const char* aFunctionName)
 {
-  JSClass* classPtr = NULL;
-
-  if (aObj) {
-    classPtr = JS_GET_CLASS(aCx, aObj);
-    if (classPtr == &sClass || classPtr == ChromeWorker::Class()) {
-      return GetJSPrivateSafeish<WorkerPrivate>(aCx, aObj);
-    }
+  JSClass* classPtr = JS_GET_CLASS(aCx, aObj);
+  if (classPtr == &sClass || classPtr == ChromeWorker::Class()) {
+    return GetJSPrivateSafeish<WorkerPrivate>(aCx, aObj);
   }
 
   JS_ReportErrorNumber(aCx, js_GetErrorMessage, NULL, JSMSG_INCOMPATIBLE_PROTO,
-                       sClass.name, aFunctionName,
-                       classPtr ? classPtr->name : "object");
+                       sClass.name, aFunctionName, classPtr->name);
   return NULL;
 }
 

dom/workers/WorkerScope.cpp

@@ -722,20 +722,14 @@ private:
   static DedicatedWorkerGlobalScope*
   GetInstancePrivate(JSContext* aCx, JSObject* aObj, const char* aFunctionName)
   {
-    // JS_GetInstancePrivate is ok to be called with a null aObj, so this should
-    // be too.
-    JSClass* classPtr = NULL;
-
-    if (aObj) {
-      classPtr = JS_GET_CLASS(aCx, aObj);
-      if (classPtr == &sClass) {
-        return GetJSPrivateSafeish<DedicatedWorkerGlobalScope>(aCx, aObj);
-      }
+    JSClass* classPtr = JS_GET_CLASS(aCx, aObj);
+    if (classPtr == &sClass) {
+      return GetJSPrivateSafeish<DedicatedWorkerGlobalScope>(aCx, aObj);
     }
 
     JS_ReportErrorNumber(aCx, js_GetErrorMessage, NULL,
                          JSMSG_INCOMPATIBLE_PROTO, sClass.name, aFunctionName,
-                         classPtr ? classPtr->name : "object");
+                         classPtr->name);
     return NULL;
   }
 
@@ -833,21 +827,13 @@ WorkerGlobalScope*
 WorkerGlobalScope::GetInstancePrivate(JSContext* aCx, JSObject* aObj,
                                       const char* aFunctionName)
 {
-  // JS_GetInstancePrivate is ok to be called with a null aObj, so this should
-  // be too.
-  JSClass* classPtr = NULL;
-
-  if (aObj) {
-    classPtr = JS_GET_CLASS(aCx, aObj);
-    if (classPtr == &sClass ||
-        classPtr == DedicatedWorkerGlobalScope::Class()) {
-      return GetJSPrivateSafeish<WorkerGlobalScope>(aCx, aObj);
-    }
+  JSClass* classPtr = JS_GET_CLASS(aCx, aObj);
+  if (classPtr == &sClass || classPtr == DedicatedWorkerGlobalScope::Class()) {
+    return GetJSPrivateSafeish<WorkerGlobalScope>(aCx, aObj);
   }
 
   JS_ReportErrorNumber(aCx, js_GetErrorMessage, NULL, JSMSG_INCOMPATIBLE_PROTO,
-                       sClass.name, aFunctionName,
-                       classPtr ? classPtr->name : "object");
+                       sClass.name, aFunctionName, classPtr->name);
   return NULL;
 }
 

dom/workers/XMLHttpRequest.cpp

@@ -148,20 +148,14 @@ private:
   static XMLHttpRequestUpload*
   GetInstancePrivate(JSContext* aCx, JSObject* aObj, const char* aFunctionName)
   {
-    JSClass* classPtr = NULL;
-
-    if (aObj) {
-      XMLHttpRequestUpload* priv = GetPrivate(aCx, aObj);
-      if (priv) {
-        return priv;
-      }
-
-      classPtr = JS_GET_CLASS(aCx, aObj);
+    XMLHttpRequestUpload* priv = GetPrivate(aCx, aObj);
+    if (priv) {
+      return priv;
     }
 
     JS_ReportErrorNumber(aCx, js_GetErrorMessage, NULL,
                          JSMSG_INCOMPATIBLE_PROTO, sClass.name, aFunctionName,
-                         classPtr ? classPtr->name : "object");
+                         JS_GET_CLASS(aCx, aObj)->name);
     return NULL;
   }
 
@@ -373,20 +367,14 @@ private:
   static XMLHttpRequestPrivate*
   GetInstancePrivate(JSContext* aCx, JSObject* aObj, const char* aFunctionName)
   {
-    JSClass* classPtr = NULL;
-
-    if (aObj) {
-      XMLHttpRequestPrivate* priv = GetPrivate(aCx, aObj);
-      if (priv) {
-        return priv;
-      }
-
-      classPtr = JS_GET_CLASS(aCx, aObj);
+    XMLHttpRequestPrivate* priv = GetPrivate(aCx, aObj);
+    if (priv) {
+      return priv;
     }
 
     JS_ReportErrorNumber(aCx, js_GetErrorMessage, NULL,
                          JSMSG_INCOMPATIBLE_PROTO, sClass.name, aFunctionName,
-                         classPtr ? classPtr->name : "object");
+                         JS_GET_CLASS(aCx, aObj)->name);
     return NULL;
   }