[LazyParseCSS] Ensure UseCounting has parity with strict parsing

third_party/WebKit/Source/core/css/parser/CSSLazyParsingState.cpp

Index: third_party/WebKit/Source/core/css/parser/CSSLazyParsingState.cpp
diff --git a/third_party/WebKit/Source/core/css/parser/CSSLazyParsingState.cpp b/third_party/WebKit/Source/core/css/parser/CSSLazyParsingState.cpp
index 1a231c6dc85ded6e06f0492cf21242bf60b8b075..b3d2ec5c849fbab7c3acbfb27b276ed83b41b87b 100644
--- a/third_party/WebKit/Source/core/css/parser/CSSLazyParsingState.cpp
+++ b/third_party/WebKit/Source/core/css/parser/CSSLazyParsingState.cpp
@@ -5,6 +5,7 @@
 #include "core/css/parser/CSSLazyParsingState.h"
 #include "core/css/parser/CSSLazyPropertyParserImpl.h"
 #include "core/css/parser/CSSParserTokenRange.h"
+#include "core/dom/Document.h"
 #include "core/frame/UseCounter.h"
 #include "platform/Histogram.h"
 
@@ -21,7 +22,8 @@ CSSLazyParsingState::CSSLazyParsingState(const CSSParserContext& context,
       m_parsedStyleRules(0),
       m_totalStyleRules(0),
       m_styleRulesNeededForNextMilestone(0),
-      m_usage(UsageGe0) {
+      m_usage(UsageGe0),
+      m_shouldUseCount(!!m_context.useCounter()) {
   recordUsageMetrics();
 }
 
@@ -33,9 +35,21 @@ CSSLazyPropertyParserImpl* CSSLazyParsingState::createLazyParser(
 
 const CSSParserContext& CSSLazyParsingState::context() {
   DCHECK(m_owningContents);
-  UseCounter* sheetCounter = UseCounter::getFrom(m_owningContents);
-  if (sheetCounter != m_context.useCounter())
-    m_context = CSSParserContext(m_context, sheetCounter);
+  if (!m_shouldUseCount) {
+    DCHECK(!m_context.useCounter());
+    return m_context;
+  }
+
+  // Try as best as possible to grab a valid UseCounter if the underlying
+  // document has gone away.
+  if (!m_document)
+    m_document = m_owningContents->anyOwnerDocument();
+
+  // Always refresh the UseCounter, as the Document can outlive its
+  // underlying frame host causing a use-after-free of m_context's counter.
+  UseCounter* useCounter = UseCounter::getFrom(m_document);

[rune, 2016/12/05 22:21:05]

If we always retrieve the UseCounter, could we simple just return
CSSParserContext(m_context, UseCounter::getFrom(m_document)) instead? Or would
that be more expensive?

[Charlie Harrison, 2016/12/05 22:27:59]

I don't have the trace now, but I remember seeing the context initialization in
profiles before. It isn't a ton, but we have do a KURL copy, a String copy, and
a Referrer copy. These mostly do things like bumping ref counts and copying
simple structs like url::Parsed, but I'd like to avoid it if possible to keep
overhead low and consistent.

[rune, 2016/12/05 22:31:53]

Acknowledged.

+  if (useCounter != m_context.useCounter())
+    m_context = CSSParserContext(m_context, useCounter);
   return m_context;
 }
 
@@ -106,4 +120,9 @@ void CSSLazyParsingState::recordUsageMetrics() {
   usageHistogram.count(m_usage);
 }
 
+DEFINE_TRACE(CSSLazyParsingState) {
+  visitor->trace(m_owningContents);
+  visitor->trace(m_document);
+}
+
 }  // namespace blink