Enable ThreadRestrictionVerifier for StringImpl

Enable ThreadRestrictionVerifier for StringImpl

BUG=442246
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2624443003
Cr-Commit-Position: refs/heads/master@{#443753}
Committed: https://chromium.googlesource.com/chromium/src/+/67786570f5237e940df0356546f36ad230f5cd45

[Charlie Harrison, 2017-01-10 02:48:18]

The CQ bit was checked by csharrison@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-10 02:49:16]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-10 04:08:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-10 04:08:11]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Charlie Harrison, 2017-01-10 22:02:57]

Description was changed from

==========
Enable ThreadRestrictionVerifier for StringImpl

BUG=442246
==========

to

==========
Enable ThreadRestrictionVerifier for StringImpl

BUG=442246
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[Charlie Harrison, 2017-01-10 22:02:59]

The CQ bit was checked by csharrison@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-10 22:03:34]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-10 22:17:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-10 22:17:13]

Dry run: Try jobs failed on following builders:
  linux_optional_gpu_tests_rel on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_optional_...)

[Charlie Harrison, 2017-01-10 22:46:26]

The CQ bit was checked by csharrison@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-10 22:47:24]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-10 23:37:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-10 23:37:48]

Dry run: Try jobs failed on following builders:
  linux_optional_gpu_tests_rel on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_optional_...)

[Charlie Harrison, 2017-01-11 16:51:11]

The CQ bit was checked by csharrison@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-11 16:51:25]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-11 17:28:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-11 17:28:31]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Charlie Harrison, 2017-01-11 17:29:58]

The CQ bit was checked by csharrison@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-11 17:30:23]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-11 19:22:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-11 19:22:44]

Dry run: Try jobs failed on following builders:
  linux_optional_gpu_tests_rel on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_optional_...)

[Charlie Harrison, 2017-01-11 21:02:57]

The CQ bit was checked by csharrison@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-11 21:03:25]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Charlie Harrison, 2017-01-11 22:04:46]

csharrison@chromium.org changed reviewers:
+ esprehn@chromium.org, haraken@chromium.org

[Charlie Harrison, 2017-01-11 22:04:47]

Hey esprehn,
this is almost ready but is hitting lots of flaky layout test timeouts. It looks
like these are potentially flaky tests already but this CL makes it much worse.
The problem is wtf/Threading is way too slow.

For fun I tried to profile a release build with DCHECK always on for
storage/indexeddb/mozilla/indexes.html which seemed particularly bad. A *huge*
percent of the overhead is these syscalls (> 50% CPU time, I didn't add it all
up).

I chatted with +haraken about earlier about a one-off optimization who mentioned
that ThreadState::current() is much faster, but WTF is not merged into platform
yet and cannot use that code.

I think we should probably just port some of the ThreadState tricks like TLS
lookups into WTF to make currentThread() / isMainThread() faster. WDYT? This
will probably speed up existing code too. WDYT?

[esprehn, 2017-01-11 22:08:34]

On 2017/01/11 at 22:04:47, csharrison wrote:
> Hey esprehn,
> this is almost ready but is hitting lots of flaky layout test timeouts. It
looks like these are potentially flaky tests already but this CL makes it much
worse. The problem is wtf/Threading is way too slow.
> 
> For fun I tried to profile a release build with DCHECK always on for
storage/indexeddb/mozilla/indexes.html which seemed particularly bad. A *huge*
percent of the overhead is these syscalls (> 50% CPU time, I didn't add it all
up).
> 
> I chatted with +haraken about earlier about a one-off optimization who
mentioned that ThreadState::current() is much faster, but WTF is not merged into
platform yet and cannot use that code.
> 
> I think we should probably just port some of the ThreadState tricks like TLS
lookups into WTF to make currentThread() / isMainThread() faster. WDYT? This
will probably speed up existing code too. WDYT?

Yeah, If you can make that happen you'd be a perf hero. :)

Fwiw I think this is just a bug on a linux where TLS is syscall(__NR_gettid)? I
think this is much faster on other platforms.

[Charlie Harrison, 2017-01-11 22:18:23]

On 2017/01/11 22:08:34, esprehn wrote:
> On 2017/01/11 at 22:04:47, csharrison wrote:
> > Hey esprehn,
> > this is almost ready but is hitting lots of flaky layout test timeouts. It
> looks like these are potentially flaky tests already but this CL makes it much
> worse. The problem is wtf/Threading is way too slow.
> > 
> > For fun I tried to profile a release build with DCHECK always on for
> storage/indexeddb/mozilla/indexes.html which seemed particularly bad. A *huge*
> percent of the overhead is these syscalls (> 50% CPU time, I didn't add it all
> up).
> > 
> > I chatted with +haraken about earlier about a one-off optimization who
> mentioned that ThreadState::current() is much faster, but WTF is not merged
into
> platform yet and cannot use that code.
> > 
> > I think we should probably just port some of the ThreadState tricks like TLS
> lookups into WTF to make currentThread() / isMainThread() faster. WDYT? This
> will probably speed up existing code too. WDYT?
> 
> Yeah, If you can make that happen you'd be a perf hero. :)
> 
> Fwiw I think this is just a bug on a linux where TLS is syscall(__NR_gettid)?
I
> think this is much faster on other platforms.

currentThread looks like this:

ThreadIdentifier currentThread() {
#if OS(MACOSX)
  return pthread_mach_thread_np(pthread_self());
#elif OS(LINUX)
  return syscall(__NR_gettid);
#elif OS(ANDROID)
  return gettid();
#else
  return reinterpret_cast<uintptr_t>(pthread_self());
#endif
}

I don't really know how fast the other calls are but you could be right and
Linux is the slow one.

[commit-bot: I haz the power, 2017-01-11 22:21:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-11 22:21:12]

Dry run: Try jobs failed on following builders:
  linux_optional_gpu_tests_rel on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_optional_...)

[esprehn, 2017-01-11 22:34:02]

https://codereview.chromium.org/2624443003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/Node.cpp (right):

https://codereview.chromium.org/2624443003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Node.cpp:1501: name.append(impl->is8Bit()
Why does't the thisElement.classNames() line below cause the same assert? How
can we change this line to be like those lines?

Also how is the impl thread calling debugName() like this? It's touching the DOM
from a background thread, that sounds like crash city. :(

https://codereview.chromium.org/2624443003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/wtf/text/StringImpl.cpp (right):

https://codereview.chromium.org/2624443003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/wtf/text/StringImpl.cpp:337: return debugString;
This function can be:

CString ascii = left(64).ascii();
return std::string(ascii.data(), ascii.length());

https://codereview.chromium.org/2624443003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/dom/Node.cpp (right):

https://codereview.chromium.org/2624443003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/dom/Node.cpp:1505: :
StringView(impl->characters16(), impl->length()));
Why do we need this but not below where we use AtomicString?

[haraken, 2017-01-11 23:57:57]

On 2017/01/11 22:08:34, esprehn wrote:
> On 2017/01/11 at 22:04:47, csharrison wrote:
> > Hey esprehn,
> > this is almost ready but is hitting lots of flaky layout test timeouts. It
> looks like these are potentially flaky tests already but this CL makes it much
> worse. The problem is wtf/Threading is way too slow.
> > 
> > For fun I tried to profile a release build with DCHECK always on for
> storage/indexeddb/mozilla/indexes.html which seemed particularly bad. A *huge*
> percent of the overhead is these syscalls (> 50% CPU time, I didn't add it all
> up).
> > 
> > I chatted with +haraken about earlier about a one-off optimization who
> mentioned that ThreadState::current() is much faster, but WTF is not merged
into
> platform yet and cannot use that code.
> > 
> > I think we should probably just port some of the ThreadState tricks like TLS
> lookups into WTF to make currentThread() / isMainThread() faster. WDYT? This
> will probably speed up existing code too. WDYT?
> 
> Yeah, If you can make that happen you'd be a perf hero. :)

Yes, this is a tracking bug:
https://bugs.chromium.org/p/chromium/issues/detail?id=621786

Just ping slangley@ if he's still working on it. If no, please take it :D

> 
> Fwiw I think this is just a bug on a linux where TLS is syscall(__NR_gettid)?
I
> think this is much faster on other platforms.

TLS look-up on Mac and Win is slow as well, although I haven't looked into it.

[Charlie Harrison, 2017-01-12 16:42:30]

Depending on the syscall speed in Win/Mac, we may not need oilpan's hack on top
of ThreadSpecific, we just need to use TLS storage for WTF::currentThread(). I
have a WIP CL doing that here:
https://codereview.chromium.org/2627153004/

As is, it looks like the timeout failures are coming from linux so we may just
need to speed up this operation on linux.

https://codereview.chromium.org/2624443003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/wtf/text/StringImpl.cpp (right):

https://codereview.chromium.org/2624443003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/wtf/text/StringImpl.cpp:337: return debugString;
On 2017/01/11 22:34:01, esprehn wrote:
> This function can be:
> 
> CString ascii = left(64).ascii();
> return std::string(ascii.data(), ascii.length());

Ah right, for some reason I was concerned with pulling in CString here but I
forget why.

https://codereview.chromium.org/2624443003/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/dom/Node.cpp (right):

https://codereview.chromium.org/2624443003/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/dom/Node.cpp:1505: :
StringView(impl->characters16(), impl->length()));
On 2017/01/11 22:34:02, esprehn wrote:
> Why do we need this but not below where we use AtomicString?

I think we probably do and we're just not hitting that code path in tests.

[Charlie Harrison, 2017-01-12 22:44:34]

The CQ bit was checked by csharrison@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-12 22:45:26]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Charlie Harrison, 2017-01-12 22:50:08]

Ok I've rebased this CL on the TLS change.

Hopefully this will remove the flaky timeouts, as the overhead in this CL (for
platforms using TLS) should be orders of magnitude less.

If overhead is still too high on non TLS platforms, we can do the fast stack
hack from ThreadState::current in wtf.

https://codereview.chromium.org/2624443003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/wtf/text/StringImpl.cpp (right):

https://codereview.chromium.org/2624443003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/wtf/text/StringImpl.cpp:337: return debugString;
On 2017/01/11 22:34:01, esprehn wrote:
> This function can be:
> 
> CString ascii = left(64).ascii();
> return std::string(ascii.data(), ascii.length());

Actually, left is a function of String, so it would have to be
String(this).left(..)...

Unfortunately unless we const_cast this we'll lose the const correctness of
hasOneRef(). I've changed it to use isolatedCopy().

[esprehn, 2017-01-12 23:00:00]

https://codereview.chromium.org/2624443003/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/dom/Node.cpp (right):

https://codereview.chromium.org/2624443003/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/dom/Node.cpp:119: void
appendUnsafe(StringBuilder& builder, const String& offThreadString) {
This needs a TODO and a bug against the paint team. Accessing anything about the
DOM from another thread is a serious bug, calling getIdAttribute() or
classNames() isn't safe either. You can't touch anything across threads.

This seems like an okay hack for now, could we instead do:

builder.append(offThreadString.isolatedCopy()) ?

https://codereview.chromium.org/2624443003/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/wtf/text/StringImpl.cpp (right):

https://codereview.chromium.org/2624443003/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/wtf/text/StringImpl.cpp:332: CString ascii =
String(this->isolatedCopy()).left(128).ascii();
just use substring, that's all left() does anyway:

String(substring(0, 128)).ascii()

no need to copy.

[commit-bot: I haz the power, 2017-01-13 02:14:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-13 02:14:43]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[haraken, 2017-01-13 04:39:05]

LGTM on my side.

https://codereview.chromium.org/2624443003/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/dom/Node.cpp (right):

https://codereview.chromium.org/2624443003/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/dom/Node.cpp:119: void
appendUnsafe(StringBuilder& builder, const String& offThreadString) {
On 2017/01/12 23:00:00, esprehn wrote:
> This needs a TODO and a bug against the paint team. Accessing anything about
the
> DOM from another thread is a serious bug, calling getIdAttribute() or
> classNames() isn't safe either. You can't touch anything across threads.
> 
> This seems like an okay hack for now, could we instead do:
> 
> builder.append(offThreadString.isolatedCopy()) ?

+1 to removing the use case of appendUnsafe.

[Charlie Harrison, 2017-01-13 17:41:52]

The CQ bit was checked by csharrison@chromium.org to run a CQ dry run

[Charlie Harrison, 2017-01-13 17:42:25]

https://codereview.chromium.org/2624443003/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/dom/Node.cpp (right):

https://codereview.chromium.org/2624443003/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/dom/Node.cpp:119: void
appendUnsafe(StringBuilder& builder, const String& offThreadString) {
On 2017/01/13 04:39:04, haraken wrote:
> On 2017/01/12 23:00:00, esprehn wrote:
> > This needs a TODO and a bug against the paint team. Accessing anything about
> the
> > DOM from another thread is a serious bug, calling getIdAttribute() or
> > classNames() isn't safe either. You can't touch anything across threads.
> > 
> > This seems like an okay hack for now, could we instead do:
> > 
> > builder.append(offThreadString.isolatedCopy()) ?
> 
> +1 to removing the use case of appendUnsafe.

I have added the TODO(crbug.com/545926), but haven't removed this method call.

My reasoning is that for a couple of these callsites, we're dealing with
AtomicStrings, so rather than 
appendUnsafe(name, thisElement.classNames()[i]);

We need to write

name.append(StringView(thisElement.classNames()[i].impl()->isolatedCopy()));

I think appendUnsafe is more readable and it doesn't require an extra copy (in
potentially performance sensitive tracing code).

https://codereview.chromium.org/2624443003/diff/120001/third_party/WebKit/Sou...
File third_party/WebKit/Source/wtf/text/StringImpl.cpp (right):

https://codereview.chromium.org/2624443003/diff/120001/third_party/WebKit/Sou...
third_party/WebKit/Source/wtf/text/StringImpl.cpp:332: CString ascii =
String(this->isolatedCopy()).left(128).ascii();
On 2017/01/12 23:00:00, esprehn wrote:
> just use substring, that's all left() does anyway:
> 
> String(substring(0, 128)).ascii()
> 
> no need to copy.

substring is not const because it can ref |this| (afaict). We need const
correctness here. I think the isolated copy isn't a big deal because we only do
this when we crash.

[commit-bot: I haz the power, 2017-01-13 17:42:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[esprehn, 2017-01-13 17:45:22]

Why do we need const correctness here? When this gets called the whole process
is about to crash.

[Charlie Harrison, 2017-01-13 17:50:13]

On 2017/01/13 17:45:22, esprehn wrote:
> Why do we need const correctness here? When this gets called the whole process
> is about to crash.

hasOneRef() is a const method that calls asciiForDebugging(), and I think it
should stay const. I don't think there's a good solution for allowing calling a
non-const method from a const method, even if we're about to crash.

Maybe we could do something like
const_cast<StringImpl*>(this)->asciiForDebugging(), but I'm not sure about the
rules for const_cast or if that would result in any UB.

[esprehn, 2017-01-13 17:56:57]

On 2017/01/13 at 17:50:13, csharrison wrote:
> On 2017/01/13 17:45:22, esprehn wrote:
> > Why do we need const correctness here? When this gets called the whole
process
> > is about to crash.
> 
> hasOneRef() is a const method that calls asciiForDebugging(), and I think it
should stay const. I don't think there's a good solution for allowing calling a
non-const method from a const method, even if we're about to crash.
> 
> Maybe we could do something like
const_cast<StringImpl*>(this)->asciiForDebugging(), but I'm not sure about the
rules for const_cast or if that would result in any UB.

const is about being logically const, which this code is. ref() and deref() are
considered const, and asciiForDebugging() is still const if it refs a String.

[Charlie Harrison, 2017-01-13 19:23:46]

The CQ bit was checked by csharrison@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-13 19:24:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Charlie Harrison, 2017-01-13 19:36:48]

I added a minimal subset of consts to StringImpl such that substring() is const
and so that we can use RefPtr<const StringImpl>, which I've added a test for.

[esprehn, 2017-01-13 22:35:29]

lgtm w/ a nit. You can fix it here or in a follow up. We don't change string so
often that we're going to be caught off guard. :)

https://codereview.chromium.org/2624443003/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/wtf/text/StringImpl.cpp (right):

https://codereview.chromium.org/2624443003/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/wtf/text/StringImpl.cpp:58: #if !DCHECK_IS_ON()
I'd prefer we changed this to have a side for DCHECK builds and side without. We
run the CQ with DCHECK turned on, so I think that means the size checks get
skipped?

I think this is:

static_assert((sizeof(StringImpl) == 3 * sizeof(int)) +
sizeof(ThreadRestrictionVerifier), ...) 

?

[commit-bot: I haz the power, 2017-01-13 22:44:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-13 22:44:32]

Dry run: This issue passed the CQ dry run.

[Charlie Harrison, 2017-01-13 23:07:54]

The CQ bit was checked by csharrison@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-13 23:08:22]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Charlie Harrison, 2017-01-13 23:16:02]

Take a look and see if you look the static_asserts.

I felt nervous about static_asserting based on compiler-dependent padding, so
strict equality was changed to <=.

[commit-bot: I haz the power, 2017-01-14 00:50:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-14 00:50:57]

Dry run: This issue passed the CQ dry run.

[esprehn, 2017-01-14 00:52:48]

lgtm

[esprehn, 2017-01-14 00:52:55]

lgtm

[Charlie Harrison, 2017-01-14 00:58:23]

The CQ bit was checked by csharrison@chromium.org

[Charlie Harrison, 2017-01-14 00:58:23]

The patchset sent to the CQ was uploaded after l-g-t-m from haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/2624443003/#ps180001
(title: "static assert")

[commit-bot: I haz the power, 2017-01-14 00:58:42]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-14 01:05:40]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1484355503619380,
"parent_rev": "ed3331c9e4e51f65867acbc3fc38bfe7cb57400f", "commit_rev":
"67786570f5237e940df0356546f36ad230f5cd45"}

[commit-bot: I haz the power, 2017-01-14 01:07:14]

Description was changed from

==========
Enable ThreadRestrictionVerifier for StringImpl

BUG=442246
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Enable ThreadRestrictionVerifier for StringImpl

BUG=442246
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2624443003
Cr-Commit-Position: refs/heads/master@{#443753}
Committed:
https://chromium.googlesource.com/chromium/src/+/67786570f5237e940df0356546f3...
==========

[commit-bot: I haz the power, 2017-01-14 01:07:16]

Committed patchset #10 (id:180001) as
https://chromium.googlesource.com/chromium/src/+/67786570f5237e940df0356546f3...