Title: [176826] trunk/Source
- Revision
- 176826
- Author
- [email protected]
- Date
- 2014-12-04 17:03:15 -0800 (Thu, 04 Dec 2014)
Log Message
Make API::String copy the underlying strings if needed, for thread safety
https://bugs.webkit.org/show_bug.cgi?id=139261
Reviewed by Sam Weinig.
Source/WebKit2:
* Shared/API/c/WKString.cpp:
(WKStringCreateWithUTF8CString):
(WKStringCreateWithJSString):
(WKStringCopyJSString):
Move the implementations from API::String and directly into the API functions.
* Shared/APIString.h:
Add a create overload that takes an rvalue reference. Call it from the create overload
that takes an lvalue reference, but explicitly copy the string.
We call isolatedCopy() again on the string in the rvalue reference overload, but that is a no-op
if the string can be sent to another thread. Add assertions in the String constructor that we can
send the string to another thread.
Source/WTF:
* wtf/RefPtr.h:
(WTF::RefPtr<T>::leakRef):
Add a leakRef() to RefPtr so we don't have to go through PassRefPtr.
* wtf/text/WTFString.cpp:
(WTF::String::isSafeToSendToAnotherThread):
Check if the string is empty before checking whether it's in an atomic string table.
It's safe to send empty strings to other threads even if they're in the atomic string table
since they will never be deallocated.
Modified Paths
Diff
Modified: trunk/Source/WTF/ChangeLog (176825 => 176826)
--- trunk/Source/WTF/ChangeLog 2014-12-05 00:59:33 UTC (rev 176825)
+++ trunk/Source/WTF/ChangeLog 2014-12-05 01:03:15 UTC (rev 176826)
@@ -1,3 +1,20 @@
+2014-12-04 Anders Carlsson <[email protected]>
+
+ Make API::String copy the underlying strings if needed, for thread safety
+ https://bugs.webkit.org/show_bug.cgi?id=139261
+
+ Reviewed by Sam Weinig.
+
+ * wtf/RefPtr.h:
+ (WTF::RefPtr<T>::leakRef):
+ Add a leakRef() to RefPtr so we don't have to go through PassRefPtr.
+
+ * wtf/text/WTFString.cpp:
+ (WTF::String::isSafeToSendToAnotherThread):
+ Check if the string is empty before checking whether it's in an atomic string table.
+ It's safe to send empty strings to other threads even if they're in the atomic string table
+ since they will never be deallocated.
+
2014-12-04 Csaba Osztrogonác <[email protected]>
Fix cast-align warning in StringImpl.h
Modified: trunk/Source/WTF/wtf/RefPtr.h (176825 => 176826)
--- trunk/Source/WTF/wtf/RefPtr.h 2014-12-05 00:59:33 UTC (rev 176825)
+++ trunk/Source/WTF/wtf/RefPtr.h 2014-12-05 01:03:15 UTC (rev 176826)
@@ -64,6 +64,8 @@
PassRefPtr<T> release() { PassRefPtr<T> tmp = adoptRef(m_ptr); m_ptr = nullptr; return tmp; }
PassRef<T> releaseNonNull() { ASSERT(m_ptr); PassRef<T> tmp = adoptRef(*m_ptr); m_ptr = nullptr; return tmp; }
+ T* leakRef() WARN_UNUSED_RETURN;
+
T& operator*() const { return *m_ptr; }
ALWAYS_INLINE T* operator->() const { return m_ptr; }
@@ -107,6 +109,15 @@
derefIfNotNull(ptr);
}
+ template<typename T>
+ inline T* RefPtr<T>::leakRef()
+ {
+ T* ptr = m_ptr;
+ m_ptr = nullptr;
+
+ return ptr;
+ }
+
template<typename T> inline RefPtr<T>& RefPtr<T>::operator=(const RefPtr& o)
{
RefPtr ptr = o;
Modified: trunk/Source/WTF/wtf/text/WTFString.cpp (176825 => 176826)
--- trunk/Source/WTF/wtf/text/WTFString.cpp 2014-12-05 00:59:33 UTC (rev 176825)
+++ trunk/Source/WTF/wtf/text/WTFString.cpp 2014-12-05 01:03:15 UTC (rev 176826)
@@ -690,14 +690,14 @@
{
if (!impl())
return true;
+ if (isEmpty())
+ return true;
// AtomicStrings are not safe to send between threads as ~StringImpl()
// will try to remove them from the wrong AtomicStringTable.
if (impl()->isAtomic())
return false;
if (impl()->hasOneRef())
return true;
- if (isEmpty())
- return true;
return false;
}
Modified: trunk/Source/WebKit2/ChangeLog (176825 => 176826)
--- trunk/Source/WebKit2/ChangeLog 2014-12-05 00:59:33 UTC (rev 176825)
+++ trunk/Source/WebKit2/ChangeLog 2014-12-05 01:03:15 UTC (rev 176826)
@@ -1,3 +1,23 @@
+2014-12-04 Anders Carlsson <[email protected]>
+
+ Make API::String copy the underlying strings if needed, for thread safety
+ https://bugs.webkit.org/show_bug.cgi?id=139261
+
+ Reviewed by Sam Weinig.
+
+ * Shared/API/c/WKString.cpp:
+ (WKStringCreateWithUTF8CString):
+ (WKStringCreateWithJSString):
+ (WKStringCopyJSString):
+ Move the implementations from API::String and directly into the API functions.
+
+ * Shared/APIString.h:
+ Add a create overload that takes an rvalue reference. Call it from the create overload
+ that takes an lvalue reference, but explicitly copy the string.
+ We call isolatedCopy() again on the string in the rvalue reference overload, but that is a no-op
+ if the string can be sent to another thread. Add assertions in the String constructor that we can
+ send the string to another thread.
+
2014-12-04 Beth Dakin <[email protected]>
Clients disabling action menus sometimes still invoke action menu behaviors
Modified: trunk/Source/WebKit2/Shared/API/c/WKString.cpp (176825 => 176826)
--- trunk/Source/WebKit2/Shared/API/c/WKString.cpp 2014-12-05 00:59:33 UTC (rev 176825)
+++ trunk/Source/WebKit2/Shared/API/c/WKString.cpp 2014-12-05 01:03:15 UTC (rev 176826)
@@ -28,6 +28,9 @@
#include "WKStringPrivate.h"
#include "WKAPICast.h"
+#include <_javascript_Core/InitializeThreading.h>
+#include <_javascript_Core/JSStringRef.h>
+#include <_javascript_Core/OpaqueJSString.h>
using namespace WebKit;
@@ -38,7 +41,7 @@
WKStringRef WKStringCreateWithUTF8CString(const char* string)
{
- RefPtr<API::String> apiString = API::String::createFromUTF8String(string);
+ auto apiString = API::String::create(WTF::String::fromUTF8(string));
return toAPI(apiString.release().leakRef());
}
@@ -85,11 +88,13 @@
WKStringRef WKStringCreateWithJSString(JSStringRef jsStringRef)
{
- RefPtr<API::String> apiString = jsStringRef ? API::String::create(jsStringRef) : API::String::createNull();
+ auto apiString = jsStringRef ? API::String::create(jsStringRef->string()) : API::String::createNull();
+
return toAPI(apiString.release().leakRef());
}
JSStringRef WKStringCopyJSString(WKStringRef stringRef)
{
- return toImpl(stringRef)->createJSString();
+ JSC::initializeThreading();
+ return OpaqueJSString::create(toImpl(stringRef)->string()).leakRef();
}
Modified: trunk/Source/WebKit2/Shared/APIString.h (176825 => 176826)
--- trunk/Source/WebKit2/Shared/APIString.h 2014-12-05 00:59:33 UTC (rev 176825)
+++ trunk/Source/WebKit2/Shared/APIString.h 2014-12-05 01:03:15 UTC (rev 176826)
@@ -27,9 +27,6 @@
#define APIString_h
#include "APIObject.h"
-#include <_javascript_Core/InitializeThreading.h>
-#include <_javascript_Core/JSStringRef.h>
-#include <_javascript_Core/OpaqueJSString.h>
#include <wtf/PassRefPtr.h>
#include <wtf/text/StringView.h>
#include <wtf/text/WTFString.h>
@@ -39,26 +36,21 @@
class String final : public ObjectImpl<Object::Type::String> {
public:
- static PassRefPtr<String> createNull()
+ static RefPtr<String> createNull()
{
return adoptRef(new String);
}
- static PassRefPtr<String> create(const WTF::String& string)
+ static RefPtr<String> create(WTF::String&& string)
{
- return adoptRef(new String(string));
+ return adoptRef(new String(string.isNull() ? WTF::String(StringImpl::empty()) : string.isolatedCopy()));
}
- static PassRefPtr<String> create(JSStringRef jsStringRef)
+ static RefPtr<String> create(const WTF::String& string)
{
- return adoptRef(new String(jsStringRef->string()));
+ return create(string.isolatedCopy());
}
- static PassRefPtr<String> createFromUTF8String(const char* string)
- {
- return adoptRef(new String(WTF::String::fromUTF8(string)));
- }
-
virtual ~String()
{
}
@@ -101,24 +93,20 @@
const WTF::String& string() const { return m_string; }
- JSStringRef createJSString() const
- {
- JSC::initializeThreading();
- return OpaqueJSString::create(m_string).leakRef();
- }
-
private:
String()
: m_string()
{
}
- String(const WTF::String& string)
- : m_string(!string.impl() ? WTF::String(StringImpl::empty()) : string)
+ String(WTF::String&& string)
+ : m_string(WTF::move(string))
{
+ ASSERT(!m_string.isNull());
+ ASSERT(m_string.isSafeToSendToAnotherThread());
}
- WTF::String m_string;
+ const WTF::String m_string;
};
} // namespace WebKit
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
