Changeset 117377 in webkit

Timestamp:

May 16, 2012 6:36:11 PM (12 years ago)

Author:

jsbell@chromium.org

Message:

SerializedScriptValue: lazy initialization of static nullValue not threadsafe
​https://bugs.webkit.org/show_bug.cgi?id=70833

Reviewed by Kentaro Hara.

Remove unsafe lazy initialization of static |null| SSV. None of the callers
appeared to be in performance-critical areas - most were preparing an event to
be dispatched to script - so no per-call-site caching was added.

No new tests - no functional changes.

• bindings/js/SerializedScriptValue.cpp: Mint a new one each time.

(WebCore::SerializedScriptValue::nullValue):

• bindings/js/SerializedScriptValue.h: Return via PassRefPtr.
• bindings/v8/SerializedScriptValue.cpp: Mint a new one each time.

(WebCore::SerializedScriptValue::nullValue):

• bindings/v8/SerializedScriptValue.h: Return via PassRefPtr.

(SerializedScriptValue):

• dom/Document.cpp:

(WebCore::Document::statePopped): Change to PassRefPtr to maintain refcount.

• dom/Document.h:

(Document):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• bindings/js/SerializedScriptValue.cpp (modified) (1 diff)
• bindings/js/SerializedScriptValue.h (modified) (1 diff)
• bindings/v8/SerializedScriptValue.cpp (modified) (1 diff)
• bindings/v8/SerializedScriptValue.h (modified) (1 diff)
• dom/Document.cpp (modified) (1 diff)
• dom/Document.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-05-16  Joshua Bell  <jsbell@chromium.org>
+
+        SerializedScriptValue: lazy initialization of static nullValue not threadsafe
+        https://bugs.webkit.org/show_bug.cgi?id=70833
+
+        Reviewed by Kentaro Hara.
+
+        Remove unsafe lazy initialization of static |null| SSV. None of the callers
+        appeared to be in performance-critical areas - most were preparing an event to
+        be dispatched to script - so no per-call-site caching was added.
+
+        No new tests - no functional changes.
+
+        * bindings/js/SerializedScriptValue.cpp: Mint a new one each time.
+        (WebCore::SerializedScriptValue::nullValue):
+        * bindings/js/SerializedScriptValue.h: Return via PassRefPtr.
+        * bindings/v8/SerializedScriptValue.cpp: Mint a new one each time.
+        (WebCore::SerializedScriptValue::nullValue):
+        * bindings/v8/SerializedScriptValue.h: Return via PassRefPtr.
+        (SerializedScriptValue):
+        * dom/Document.cpp:
+        (WebCore::Document::statePopped): Change to PassRefPtr to maintain refcount.
+        * dom/Document.h:
+        (Document):
+
 2012-05-16  James Robinson  <jamesr@chromium.org>
 

trunk/Source/WebCore/bindings/js/SerializedScriptValue.cpp

@@ -1860 +1860 @@
 }
 
-SerializedScriptValue* SerializedScriptValue::nullValue()
-{
-    DEFINE_STATIC_LOCAL(RefPtr<SerializedScriptValue>, emptyValue, (SerializedScriptValue::create()));
-    return emptyValue.get();
+PassRefPtr<SerializedScriptValue> SerializedScriptValue::nullValue()
+{
+    return SerializedScriptValue::create();
 }
 

trunk/Source/WebCore/bindings/js/SerializedScriptValue.h

@@ -79 +79 @@
 
     static PassRefPtr<SerializedScriptValue> create();
-    static SerializedScriptValue* nullValue();
+    static PassRefPtr<SerializedScriptValue> nullValue();
     static PassRefPtr<SerializedScriptValue> undefinedValue();
     static PassRefPtr<SerializedScriptValue> booleanValue(bool value);

trunk/Source/WebCore/bindings/v8/SerializedScriptValue.cpp

@@ -2099 +2099 @@
 }
 
-SerializedScriptValue* SerializedScriptValue::nullValue(v8::Isolate* isolate)
+PassRefPtr<SerializedScriptValue> SerializedScriptValue::nullValue(v8::Isolate* isolate)
 {
-    // FIXME: This is not thread-safe. Move caching to callers.
-    // https://bugs.webkit.org/show_bug.cgi?id=70833
-    DEFINE_STATIC_LOCAL(RefPtr<SerializedScriptValue>, nullValue, (0));
-    if (!nullValue) {
-        Writer writer(isolate);
-        writer.writeNull();
-        String wireData = StringImpl::adopt(writer.data());
-        nullValue = adoptRef(new SerializedScriptValue(wireData));
-    }
-    return nullValue.get();
+    Writer writer(isolate);
+    writer.writeNull();
+    String wireData = StringImpl::adopt(writer.data());
+    return adoptRef(new SerializedScriptValue(wireData));
 }
 

trunk/Source/WebCore/bindings/v8/SerializedScriptValue.h

@@ -59 +59 @@
     static PassRefPtr<SerializedScriptValue> create();
 
-    static SerializedScriptValue* nullValue(v8::Isolate* = 0);
+    static PassRefPtr<SerializedScriptValue> nullValue(v8::Isolate* = 0);
     static PassRefPtr<SerializedScriptValue> undefinedValue(v8::Isolate* = 0);
     static PassRefPtr<SerializedScriptValue> booleanValue(bool value, v8::Isolate* = 0);

trunk/Source/WebCore/dom/Document.cpp

@@ -5061 +5061 @@
 }
 
-void Document::statePopped(SerializedScriptValue* stateObject)
+void Document::statePopped(PassRefPtr<SerializedScriptValue> stateObject)
 {
     if (!frame())

trunk/Source/WebCore/dom/Document.h

@@ -1051 +1051 @@
 
     void updateURLForPushOrReplaceState(const KURL&);
-    void statePopped(SerializedScriptValue*);
+    void statePopped(PassRefPtr<SerializedScriptValue>);
 
     bool processingLoadEvent() const { return m_processingLoadEvent; }