<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[176826] trunk/Source</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/176826">176826</a></dd>
<dt>Author</dt> <dd>andersca@apple.com</dd>
<dt>Date</dt> <dd>2014-12-04 17:03:15 -0800 (Thu, 04 Dec 2014)</dd>
</dl>
<h3>Log Message</h3>
<pre>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.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWTFChangeLog">trunk/Source/WTF/ChangeLog</a></li>
<li><a href="#trunkSourceWTFwtfRefPtrh">trunk/Source/WTF/wtf/RefPtr.h</a></li>
<li><a href="#trunkSourceWTFwtftextWTFStringcpp">trunk/Source/WTF/wtf/text/WTFString.cpp</a></li>
<li><a href="#trunkSourceWebKit2ChangeLog">trunk/Source/WebKit2/ChangeLog</a></li>
<li><a href="#trunkSourceWebKit2SharedAPIcWKStringcpp">trunk/Source/WebKit2/Shared/API/c/WKString.cpp</a></li>
<li><a href="#trunkSourceWebKit2SharedAPIStringh">trunk/Source/WebKit2/Shared/APIString.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWTFChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WTF/ChangeLog (176825 => 176826)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,3 +1,20 @@
</span><ins>+2014-12-04 Anders Carlsson <andersca@apple.com>
+
+ 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.
+
</ins><span class="cx"> 2014-12-04 Csaba Osztrogonác <ossy@webkit.org>
</span><span class="cx">
</span><span class="cx"> Fix cast-align warning in StringImpl.h
</span></span></pre></div>
<a id="trunkSourceWTFwtfRefPtrh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WTF/wtf/RefPtr.h (176825 => 176826)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -64,6 +64,8 @@
</span><span class="cx"> PassRefPtr<T> release() { PassRefPtr<T> tmp = adoptRef(m_ptr); m_ptr = nullptr; return tmp; }
</span><span class="cx"> PassRef<T> releaseNonNull() { ASSERT(m_ptr); PassRef<T> tmp = adoptRef(*m_ptr); m_ptr = nullptr; return tmp; }
</span><span class="cx">
</span><ins>+ T* leakRef() WARN_UNUSED_RETURN;
+
</ins><span class="cx"> T& operator*() const { return *m_ptr; }
</span><span class="cx"> ALWAYS_INLINE T* operator->() const { return m_ptr; }
</span><span class="cx">
</span><span class="lines">@@ -107,6 +109,15 @@
</span><span class="cx"> derefIfNotNull(ptr);
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ template<typename T>
+ inline T* RefPtr<T>::leakRef()
+ {
+ T* ptr = m_ptr;
+ m_ptr = nullptr;
+
+ return ptr;
+ }
+
</ins><span class="cx"> template<typename T> inline RefPtr<T>& RefPtr<T>::operator=(const RefPtr& o)
</span><span class="cx"> {
</span><span class="cx"> RefPtr ptr = o;
</span></span></pre></div>
<a id="trunkSourceWTFwtftextWTFStringcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WTF/wtf/text/WTFString.cpp (176825 => 176826)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -690,14 +690,14 @@
</span><span class="cx"> {
</span><span class="cx"> if (!impl())
</span><span class="cx"> return true;
</span><ins>+ if (isEmpty())
+ return true;
</ins><span class="cx"> // AtomicStrings are not safe to send between threads as ~StringImpl()
</span><span class="cx"> // will try to remove them from the wrong AtomicStringTable.
</span><span class="cx"> if (impl()->isAtomic())
</span><span class="cx"> return false;
</span><span class="cx"> if (impl()->hasOneRef())
</span><span class="cx"> return true;
</span><del>- if (isEmpty())
- return true;
</del><span class="cx"> return false;
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre></div>
<a id="trunkSourceWebKit2ChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/ChangeLog (176825 => 176826)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,3 +1,23 @@
</span><ins>+2014-12-04 Anders Carlsson <andersca@apple.com>
+
+ 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.
+
</ins><span class="cx"> 2014-12-04 Beth Dakin <bdakin@apple.com>
</span><span class="cx">
</span><span class="cx"> Clients disabling action menus sometimes still invoke action menu behaviors
</span></span></pre></div>
<a id="trunkSourceWebKit2SharedAPIcWKStringcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/Shared/API/c/WKString.cpp (176825 => 176826)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -28,6 +28,9 @@
</span><span class="cx"> #include "WKStringPrivate.h"
</span><span class="cx">
</span><span class="cx"> #include "WKAPICast.h"
</span><ins>+#include <JavaScriptCore/InitializeThreading.h>
+#include <JavaScriptCore/JSStringRef.h>
+#include <JavaScriptCore/OpaqueJSString.h>
</ins><span class="cx">
</span><span class="cx"> using namespace WebKit;
</span><span class="cx">
</span><span class="lines">@@ -38,7 +41,7 @@
</span><span class="cx">
</span><span class="cx"> WKStringRef WKStringCreateWithUTF8CString(const char* string)
</span><span class="cx"> {
</span><del>- RefPtr<API::String> apiString = API::String::createFromUTF8String(string);
</del><ins>+ auto apiString = API::String::create(WTF::String::fromUTF8(string));
</ins><span class="cx"> return toAPI(apiString.release().leakRef());
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -85,11 +88,13 @@
</span><span class="cx">
</span><span class="cx"> WKStringRef WKStringCreateWithJSString(JSStringRef jsStringRef)
</span><span class="cx"> {
</span><del>- RefPtr<API::String> apiString = jsStringRef ? API::String::create(jsStringRef) : API::String::createNull();
</del><ins>+ auto apiString = jsStringRef ? API::String::create(jsStringRef->string()) : API::String::createNull();
+
</ins><span class="cx"> return toAPI(apiString.release().leakRef());
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> JSStringRef WKStringCopyJSString(WKStringRef stringRef)
</span><span class="cx"> {
</span><del>- return toImpl(stringRef)->createJSString();
</del><ins>+ JSC::initializeThreading();
+ return OpaqueJSString::create(toImpl(stringRef)->string()).leakRef();
</ins><span class="cx"> }
</span></span></pre></div>
<a id="trunkSourceWebKit2SharedAPIStringh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/Shared/APIString.h (176825 => 176826)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -27,9 +27,6 @@
</span><span class="cx"> #define APIString_h
</span><span class="cx">
</span><span class="cx"> #include "APIObject.h"
</span><del>-#include <JavaScriptCore/InitializeThreading.h>
-#include <JavaScriptCore/JSStringRef.h>
-#include <JavaScriptCore/OpaqueJSString.h>
</del><span class="cx"> #include <wtf/PassRefPtr.h>
</span><span class="cx"> #include <wtf/text/StringView.h>
</span><span class="cx"> #include <wtf/text/WTFString.h>
</span><span class="lines">@@ -39,26 +36,21 @@
</span><span class="cx">
</span><span class="cx"> class String final : public ObjectImpl<Object::Type::String> {
</span><span class="cx"> public:
</span><del>- static PassRefPtr<String> createNull()
</del><ins>+ static RefPtr<String> createNull()
</ins><span class="cx"> {
</span><span class="cx"> return adoptRef(new String);
</span><span class="cx"> }
</span><span class="cx">
</span><del>- static PassRefPtr<String> create(const WTF::String& string)
</del><ins>+ static RefPtr<String> create(WTF::String&& string)
</ins><span class="cx"> {
</span><del>- return adoptRef(new String(string));
</del><ins>+ return adoptRef(new String(string.isNull() ? WTF::String(StringImpl::empty()) : string.isolatedCopy()));
</ins><span class="cx"> }
</span><span class="cx">
</span><del>- static PassRefPtr<String> create(JSStringRef jsStringRef)
</del><ins>+ static RefPtr<String> create(const WTF::String& string)
</ins><span class="cx"> {
</span><del>- return adoptRef(new String(jsStringRef->string()));
</del><ins>+ return create(string.isolatedCopy());
</ins><span class="cx"> }
</span><span class="cx">
</span><del>- static PassRefPtr<String> createFromUTF8String(const char* string)
- {
- return adoptRef(new String(WTF::String::fromUTF8(string)));
- }
-
</del><span class="cx"> virtual ~String()
</span><span class="cx"> {
</span><span class="cx"> }
</span><span class="lines">@@ -101,24 +93,20 @@
</span><span class="cx">
</span><span class="cx"> const WTF::String& string() const { return m_string; }
</span><span class="cx">
</span><del>- JSStringRef createJSString() const
- {
- JSC::initializeThreading();
- return OpaqueJSString::create(m_string).leakRef();
- }
-
</del><span class="cx"> private:
</span><span class="cx"> String()
</span><span class="cx"> : m_string()
</span><span class="cx"> {
</span><span class="cx"> }
</span><span class="cx">
</span><del>- String(const WTF::String& string)
- : m_string(!string.impl() ? WTF::String(StringImpl::empty()) : string)
</del><ins>+ String(WTF::String&& string)
+ : m_string(WTF::move(string))
</ins><span class="cx"> {
</span><ins>+ ASSERT(!m_string.isNull());
+ ASSERT(m_string.isSafeToSendToAnotherThread());
</ins><span class="cx"> }
</span><span class="cx">
</span><del>- WTF::String m_string;
</del><ins>+ const WTF::String m_string;
</ins><span class="cx"> };
</span><span class="cx">
</span><span class="cx"> } // namespace WebKit
</span></span></pre>
</div>
</div>
</body>
</html>