This crash looks like a stack overflow caused by this infinite loop.

WTF::StringImpl::asciiForDebugging() Line 338
 -> WTF::StringImpl::substring(unsigned int start, unsigned int length) Line 529
  -> WTF::PassRefPtr<WTF::StringImpl>::PassRefPtr<WTF::StringImpl>(WTF::StringImpl * ptr) Line 74
   -> WTF::refIfNotNull<WTF::StringImpl>(WTF::StringImpl * ptr) Line 59
    -> WTF::StringImpl::ref() Line 313
     -> WTF::StringImpl::asciiForDebugging()

StringImpl::asciiForDebugging() was introduced by csharrison@.
https://codereview.chromium.org/2624443003

csharrison@
Is it an intentional crash caused by ThreadRestrictionVerifier?

Do you have the beginning of the stack trace?

This should be an intentional crash, but the infinite recursion is definitely *not* intentional and should be removed. If you replace the first line of asciiWithDebugging() code with
 "CString ascii = String(isolatedCopy().substring(0, 128)).ascii();" it should give you a much nicer crash.

StringImpl::asciiForDebugging() is called from ServiceWorkerContextClient::NavigationPreloadRequest::OnReceiveResponse().

content::ServiceWorkerContextClient::NavigationPreloadRequest::OnReceiveResponse(const content::ResourceResponseHead & response_head, mojo::InterfacePtr<content::mojom::DownloadedTempFile> downloaded_file)
 -> content::WebURLLoaderImpl::PopulateURLResponse(const GURL & url, const content::ResourceResponseInfo & info, blink::WebURLResponse * response, bool report_security_info)
  -> blink::WebURLResponse::addHTTPHeaderField(const blink::WebString & name, const blink::WebString & value)
   -> blink::ResourceResponse::addHTTPHeaderField(const WTF::AtomicString & name, const WTF::AtomicString & value)
    -> blink::ResourceResponse::updateHeaderParsedState(const WTF::AtomicString & name)
     -> WTF::StringView::StringView(const WTF::AtomicString & string)
      -> WTF::StringView::StringView(const WTF::StringImpl * impl)
       -> WTF::RefPtr<WTF::StringImpl>::RefPtr<WTF::StringImpl>(WTF::StringImpl * ptr)
        -> WTF::refIfNotNull<WTF::StringImpl>(WTF::StringImpl * ptr)
         -> WTF::StringImpl::ref()
          -> WTF::StringImpl::asciiForDebugging()


I verified that this crash doesn't happen before https://codereview.chromium.org/2624443003 (67786570f5237e940df0356546f36ad230f5cd45).

OK #443752 ed3331c9e4e51f65867acbc3fc38bfe7cb57400f
NG #443753 67786570f5237e940df0356546f36ad230f5cd45

The check failing looks like this:
[1:14:0116/114721.587823:3270075445135:FATAL:StringImpl.h(313)] Check failed: isStatic() || m_verifier.onRef(m_refCount). age
#0 0x7f2f6d4ba95e base::debug::StackTrace::StackTrace()
#1 0x7f2f6d4dee5b logging::LogMessage::~LogMessage()
#2 0x7f2f6a6cba09 WTF::StringView::StringView()
#3 0x7f2f6a878d8a blink::ResourceResponse::updateHeaderParsedState()
#4 0x7f2f6a87961d blink::ResourceResponse::addHTTPHeaderField()
#5 0x7f2f6a73f339 blink::WebURLResponse::addHTTPHeaderField()
#6 0x7f2f6dc6bde8 content::WebURLLoaderImpl::PopulateURLResponse()
#7 0x7f2f6e66be79 content::ServiceWorkerContextClient::NavigationPreloadRequest::OnReceiveResponse()
#8 0x7f2f6dd7e748 content::mojom::URLLoaderClientStubDispatch::Accept()
#9 0x7f2f6c8b8dbd mojo::InterfaceEndpointClient::HandleValidatedMessage()
#10 0x7f2f6c8b85e6 mojo::FilterChain::Accept()
#11 0x7f2f6c8b9e7e mojo::InterfaceEndpointClient::HandleIncomingMessage()
#12 0x7f2f6c8c10aa mojo::internal::MultiplexRouter::ProcessIncomingMessage()
#13 0x7f2f6c8c0a0a mojo::internal::MultiplexRouter::Accept()
#14 0x7f2f6c8b85e6 mojo::FilterChain::Accept()
#15 0x7f2f6c8b3ea2 mojo::Connector::ReadSingleMessage()
#16 0x7f2f6c8b4501 mojo::Connector::OnHandleReadyInternal()
#17 0x7f2f6c89953a mojo::Watcher::OnHandleReady()
#18 0x7f2f6c899664 _ZN4base8internal13FunctorTraitsIMN4mojo7WatcherEFvjEvE6InvokeIRKNS_7WeakPtrIS3_EEJRKjEEEvS5_OT_DpOT0_
#19 0x7f2f6d4bb52e base::debug::TaskAnnotator::RunTask()
#20 0x7f2f6a88aae0 blink::scheduler::TaskQueueManager::ProcessTaskFromWorkQueue()
#21 0x7f2f6a8893d4 blink::scheduler::TaskQueueManager::DoWork()

What thread does the NavigationPreloadRequest::OnReceiveResponse live on? It looks like we are probably using the static local AtomicStrings incorrectly in updateHeaderParsedState.

horo: If you would like AtomicString optimization on an off-thread. Those strings should be declared static, like httpAtom/httpsAtom.

NavigationPreloadRequest::OnReceiveResponse lives on the service worker thread.

Ah, yes. I think ResourceResponse::updateHeaderParsedState() is called on the worker thread only when NavigationPreload feature is enabled.

csharrison@

Are httpAtom/httpsAtom usable from the worker threads?
The comments in AtomicString.h says:
// These are only usable from the main thread.
https://chromium.googlesource.com/chromium/src/+/e827c1a/third_party/WebKit/Source/wtf/text/AtomicString.h#267

Patch avoiding infinite recursion is here:
https://codereview.chromium.org/2635033002/

Probably a separate patch should be made updating AtomicStrings for headers. I think there are more problematic cases in that file from a brief look. I think we have two options:
1. Make them static
2. Dynamically add them to the AtomicStringTable if we are not on the main thread.

I think (1) is better but if there are tons of these we should be cautious in adding too many static strings.

Sorry for some reason monorail is letting us talk past each other :/

I think the comment is out of date. If you look at StringStatics::init the strings which are initialized as static should be safe to access from any thread. If that is not the case then we have existing bugs, as httpAtom/httpsAtom is used for every KURL construction on any thread.

Thank you.
I will create a cl to move AtomicStrings in ResourceResponse.cpp to static.

LayoutTests added by https://codereview.chromium.org/2627023002/ are crashing in Debug build bots because of this issue.

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=webkit_tests&tests=http%2Ftests%2Fserviceworker%2Fchromium%2Fnavigation-preload&showLargeExpectations=true

Created the CL: https://codereview.chromium.org/2638623004/

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ad218510d6560dbc2b87d878f4e8b2ed5ac9730a

commit ad218510d6560dbc2b87d878f4e8b2ed5ac9730a
Author: horo <horo@chromium.org>
Date: Tue Jan 17 09:57:53 2017

Stop using DEFINE_STATIC_LOCAL in ResourceResponse.cpp.

There are several AtomicStrings defined using DEFINE_STATIC_LOCAL in ResourceResponse.cpp.
When those strings are touched on the worker thread, the ThreadRestrictionVerifier check in StringImpl.h fails.
 https://crbug.com/681581#c5 

To avoid this check failure, this CL makes those strings static.

BUG= 681581 

Review-Url: https://codereview.chromium.org/2638623004
Cr-Commit-Position: refs/heads/master@{#444016}

[modify] https://crrev.com/ad218510d6560dbc2b87d878f4e8b2ed5ac9730a/third_party/WebKit/Source/platform/BUILD.gn
[modify] https://crrev.com/ad218510d6560dbc2b87d878f4e8b2ed5ac9730a/third_party/WebKit/Source/platform/network/HTTPParsers.cpp
[modify] https://crrev.com/ad218510d6560dbc2b87d878f4e8b2ed5ac9730a/third_party/WebKit/Source/platform/network/ResourceResponse.cpp
[modify] https://crrev.com/ad218510d6560dbc2b87d878f4e8b2ed5ac9730a/third_party/WebKit/Source/platform/network/ResourceResponseTest.cpp

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/965f6c6f71cd704f638caf664c591986a5d8df53

commit 965f6c6f71cd704f638caf664c591986a5d8df53
Author: csharrison <csharrison@chromium.org>
Date: Tue Jan 17 13:04:17 2017

Avoid infinite recursion in StringImpl::asciiForDebugging

If any of the DCHECKs are hit in ref/deref/hasOneRef, then calling
substring() on the underlying impl can also trigger DCHECKs, causing
infinite recursion.

BUG= 681581 

Review-Url: https://codereview.chromium.org/2635033002
Cr-Commit-Position: refs/heads/master@{#444035}

[modify] https://crrev.com/965f6c6f71cd704f638caf664c591986a5d8df53/third_party/WebKit/Source/wtf/text/StringImpl.cpp