- Revision
- 143441
- Author
- commit-qu...@webkit.org
- Date
- 2013-02-20 01:58:37 -0800 (Wed, 20 Feb 2013)
Log Message
[v8] ScriptValue has dangerous copy semantics
https://bugs.webkit.org/show_bug.cgi?id=110206
Patch by Dan Carney <dcar...@google.com> on 2013-02-20
Reviewed by Kentaro Hara.
Update ScriptValue to used a SharedPersistent,
making it impossible to return dead references.
No new tests. No change in functionality.
* bindings/v8/ScriptValue.cpp:
(WebCore::ScriptValue::serialize):
(WebCore::ScriptValue::getString):
(WebCore::ScriptValue::toString):
(WebCore::ScriptValue::toInspectorValue):
* bindings/v8/ScriptValue.h:
(WebCore::ScriptValue::ScriptValue):
(WebCore::ScriptValue::operator=):
(WebCore::ScriptValue::operator==):
(WebCore::ScriptValue::isEqual):
(WebCore::ScriptValue::isFunction):
(WebCore::ScriptValue::isNull):
(WebCore::ScriptValue::isUndefined):
(WebCore::ScriptValue::isObject):
(WebCore::ScriptValue::hasNoValue):
(WebCore::ScriptValue::clear):
(ScriptValue):
(WebCore::ScriptValue::v8Value):
(WebCore::ScriptValue::v8ValueRaw):
* bindings/v8/SharedPersistent.h:
* bindings/v8/custom/V8InjectedScriptHostCustom.cpp:
(WebCore::InjectedScriptHost::scriptValueAsNode):
* bindings/v8/custom/V8MessageEventCustom.cpp:
(WebCore::V8MessageEvent::dataAttrGetterCustom):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (143440 => 143441)
--- trunk/Source/WebCore/ChangeLog 2013-02-20 09:44:37 UTC (rev 143440)
+++ trunk/Source/WebCore/ChangeLog 2013-02-20 09:58:37 UTC (rev 143441)
@@ -1,3 +1,40 @@
+2013-02-20 Dan Carney <dcar...@google.com>
+
+ [v8] ScriptValue has dangerous copy semantics
+ https://bugs.webkit.org/show_bug.cgi?id=110206
+
+ Reviewed by Kentaro Hara.
+
+ Update ScriptValue to used a SharedPersistent,
+ making it impossible to return dead references.
+
+ No new tests. No change in functionality.
+
+ * bindings/v8/ScriptValue.cpp:
+ (WebCore::ScriptValue::serialize):
+ (WebCore::ScriptValue::getString):
+ (WebCore::ScriptValue::toString):
+ (WebCore::ScriptValue::toInspectorValue):
+ * bindings/v8/ScriptValue.h:
+ (WebCore::ScriptValue::ScriptValue):
+ (WebCore::ScriptValue::operator=):
+ (WebCore::ScriptValue::operator==):
+ (WebCore::ScriptValue::isEqual):
+ (WebCore::ScriptValue::isFunction):
+ (WebCore::ScriptValue::isNull):
+ (WebCore::ScriptValue::isUndefined):
+ (WebCore::ScriptValue::isObject):
+ (WebCore::ScriptValue::hasNoValue):
+ (WebCore::ScriptValue::clear):
+ (ScriptValue):
+ (WebCore::ScriptValue::v8Value):
+ (WebCore::ScriptValue::v8ValueRaw):
+ * bindings/v8/SharedPersistent.h:
+ * bindings/v8/custom/V8InjectedScriptHostCustom.cpp:
+ (WebCore::InjectedScriptHost::scriptValueAsNode):
+ * bindings/v8/custom/V8MessageEventCustom.cpp:
+ (WebCore::V8MessageEvent::dataAttrGetterCustom):
+
2013-02-20 Andrey Lushnikov <lushni...@chromium.org>
Web Inspector: highlight undefined word in _javascript_
Modified: trunk/Source/WebCore/bindings/v8/ScriptValue.cpp (143440 => 143441)
--- trunk/Source/WebCore/bindings/v8/ScriptValue.cpp 2013-02-20 09:44:37 UTC (rev 143440)
+++ trunk/Source/WebCore/bindings/v8/ScriptValue.cpp 2013-02-20 09:58:37 UTC (rev 143441)
@@ -47,13 +47,13 @@
PassRefPtr<SerializedScriptValue> ScriptValue::serialize(ScriptState* scriptState)
{
ScriptScope scope(scriptState);
- return SerializedScriptValue::create(v8Value());
+ return SerializedScriptValue::create(v8ValueRaw());
}
PassRefPtr<SerializedScriptValue> ScriptValue::serialize(ScriptState* scriptState, MessagePortArray* messagePorts, ArrayBufferArray* arrayBuffers, bool& didThrow)
{
ScriptScope scope(scriptState);
- return SerializedScriptValue::create(v8Value(), messagePorts, arrayBuffers, didThrow);
+ return SerializedScriptValue::create(v8ValueRaw(), messagePorts, arrayBuffers, didThrow);
}
ScriptValue ScriptValue::deserialize(ScriptState* scriptState, SerializedScriptValue* value)
@@ -64,20 +64,20 @@
bool ScriptValue::getString(String& result) const
{
- if (m_value.isEmpty())
+ if (hasNoValue())
return false;
- if (!m_value->IsString())
+ if (!v8ValueRaw()->IsString())
return false;
- result = toWebCoreString(m_value.get());
+ result = toWebCoreString(v8ValueRaw());
return true;
}
String ScriptValue::toString(ScriptState*) const
{
v8::TryCatch block;
- v8::Handle<v8::String> string = m_value->ToString();
+ v8::Handle<v8::String> string = v8ValueRaw()->ToString();
if (block.HasCaught())
return String();
return v8StringToWebCoreString<String>(string, DoNotExternalize);
@@ -142,7 +142,7 @@
v8::HandleScope handleScope;
// v8::Object::GetPropertyNames() expects current context to be not null.
v8::Context::Scope contextScope(scriptState->context());
- return v8ToInspectorValue(m_value.get(), InspectorValue::maxDepth);
+ return v8ToInspectorValue(v8ValueRaw(), InspectorValue::maxDepth);
}
#endif
Modified: trunk/Source/WebCore/bindings/v8/ScriptValue.h (143440 => 143441)
--- trunk/Source/WebCore/bindings/v8/ScriptValue.h 2013-02-20 09:44:37 UTC (rev 143440)
+++ trunk/Source/WebCore/bindings/v8/ScriptValue.h 2013-02-20 09:58:37 UTC (rev 143441)
@@ -31,8 +31,8 @@
#ifndef ScriptValue_h
#define ScriptValue_h
-#include "ScopedPersistent.h"
#include "ScriptState.h"
+#include "SharedPersistent.h"
#include <v8.h>
#include <wtf/PassRefPtr.h>
#include <wtf/RefPtr.h>
@@ -60,47 +60,37 @@
ScriptValue() { }
virtual ~ScriptValue();
- ScriptValue(v8::Handle<v8::Value> value)
+ ScriptValue(v8::Handle<v8::Value> value)
+ : m_value(value.IsEmpty() ? 0 : SharedPersistent<v8::Value>::create(value))
{
- if (value.IsEmpty())
- return;
- m_value.set(value);
}
- ScriptValue(const ScriptValue& value)
+ ScriptValue(const ScriptValue& value)
+ : m_value(value.m_value)
{
- if (value.hasNoValue())
- return;
- m_value.set(value.m_value.get());
}
ScriptValue& operator=(const ScriptValue& value)
{
- if (this == &value)
- return *this;
-
- m_value.clear();
-
- if (value.hasNoValue())
- return *this;
-
- m_value.set(value.m_value.get());
+ if (this != &value)
+ m_value = value.m_value;
return *this;
}
bool operator==(const ScriptValue& value) const
{
- return m_value.get() == value.m_value.get();
+ return v8ValueRaw() == value.v8ValueRaw();
}
bool isEqual(ScriptState*, const ScriptValue& value) const
{
- return m_value.get() == value.m_value.get();
+ return operator==(value);
}
bool isFunction() const
{
- return m_value->IsFunction();
+ ASSERT(!hasNoValue());
+ return v8ValueRaw()->IsFunction();
}
bool operator!=(const ScriptValue& value) const
@@ -110,22 +100,25 @@
bool isNull() const
{
- return m_value->IsNull();
+ ASSERT(!hasNoValue());
+ return v8ValueRaw()->IsNull();
}
bool isUndefined() const
{
- return m_value->IsUndefined();
+ ASSERT(!hasNoValue());
+ return v8ValueRaw()->IsUndefined();
}
bool isObject() const
{
- return m_value->IsObject();
+ ASSERT(!hasNoValue());
+ return v8ValueRaw()->IsObject();
}
bool hasNoValue() const
{
- return m_value.isEmpty();
+ return !m_value.get() || m_value->get().IsEmpty();
}
PassRefPtr<SerializedScriptValue> serialize(ScriptState*);
@@ -134,11 +127,20 @@
void clear()
{
- m_value.clear();
+ m_value = 0;
}
- v8::Handle<v8::Value> v8Value() const { return m_value.get(); }
+ v8::Handle<v8::Value> v8Value() const
+ {
+ return v8::Local<v8::Value>::New(v8ValueRaw());
+ }
+ // FIXME: This function should be private.
+ v8::Handle<v8::Value> v8ValueRaw() const
+ {
+ return m_value.get() ? m_value->get() : v8::Handle<v8::Value>();
+ }
+
bool getString(ScriptState*, String& result) const { return getString(result); }
bool getString(String& result) const;
String toString(ScriptState*) const;
@@ -146,7 +148,7 @@
PassRefPtr<InspectorValue> toInspectorValue(ScriptState*) const;
private:
- ScopedPersistent<v8::Value> m_value;
+ RefPtr<SharedPersistent<v8::Value> > m_value;
};
} // namespace WebCore
Modified: trunk/Source/WebCore/bindings/v8/SharedPersistent.h (143440 => 143441)
--- trunk/Source/WebCore/bindings/v8/SharedPersistent.h 2013-02-20 09:44:37 UTC (rev 143440)
+++ trunk/Source/WebCore/bindings/v8/SharedPersistent.h 2013-02-20 09:58:37 UTC (rev 143441)
@@ -38,7 +38,6 @@
namespace WebCore {
- // FIXME: Remove this class.
template <typename T>
class SharedPersistent : public RefCounted<SharedPersistent<T> > {
public:
Modified: trunk/Source/WebCore/bindings/v8/custom/V8InjectedScriptHostCustom.cpp (143440 => 143441)
--- trunk/Source/WebCore/bindings/v8/custom/V8InjectedScriptHostCustom.cpp 2013-02-20 09:44:37 UTC (rev 143440)
+++ trunk/Source/WebCore/bindings/v8/custom/V8InjectedScriptHostCustom.cpp 2013-02-20 09:58:37 UTC (rev 143441)
@@ -65,7 +65,7 @@
{
if (!value.isObject() || value.isNull())
return 0;
- return V8Node::toNative(v8::Handle<v8::Object>::Cast(value.v8Value()));
+ return V8Node::toNative(v8::Handle<v8::Object>::Cast(value.v8ValueRaw()));
}
ScriptValue InjectedScriptHost::nodeAsScriptValue(ScriptState* state, Node* node)
Modified: trunk/Source/WebCore/bindings/v8/custom/V8MessageEventCustom.cpp (143440 => 143441)
--- trunk/Source/WebCore/bindings/v8/custom/V8MessageEventCustom.cpp 2013-02-20 09:44:37 UTC (rev 143440)
+++ trunk/Source/WebCore/bindings/v8/custom/V8MessageEventCustom.cpp 2013-02-20 09:58:37 UTC (rev 143441)
@@ -53,7 +53,7 @@
if (scriptValue.hasNoValue())
result = v8Null(info.GetIsolate());
else
- result = v8::Local<v8::Value>::New(scriptValue.v8Value());
+ result = scriptValue.v8Value();
break;
}
