Change CSSParserContext to have a Document handle (vs UseCounter).

third_party/WebKit/Source/core/css/parser/CSSParserContext.h

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CSSParserContext_h
 #define CSSParserContext_h
 
 #include "core/CoreExport.h"
 #include "core/css/parser/CSSParserMode.h"
+#include "core/dom/Document.h"
+#include "core/frame/UseCounter.h"
 #include "platform/loader/fetch/ResourceLoaderOptions.h"
 #include "platform/weborigin/KURL.h"
 #include "platform/weborigin/Referrer.h"
 
 namespace blink {
 
 class CSSStyleSheet;
-class Document;
 class StyleSheetContents;
-class UseCounter;
 
 class CORE_EXPORT CSSParserContext
     : public GarbageCollectedFinalized<CSSParserContext> {
  public:
   // https://drafts.csswg.org/selectors/#profiles
   enum SelectorProfile { DynamicProfile, StaticProfile };
 
   // All three of these factories copy the context and override the current
-  // UseCounter handle.
+  // Document handle used for UseCounter.
   static CSSParserContext* createWithStyleSheet(const CSSParserContext*,
                                                 const CSSStyleSheet*);
   static CSSParserContext* createWithStyleSheetContents(
       const CSSParserContext*,
       const StyleSheetContents*);
   // FIXME: This constructor shouldn't exist if we properly piped the UseCounter
   // through the CSS subsystem. Currently the UseCounter life time is too crazy
   // and we need a way to override it.
-  static CSSParserContext* create(const CSSParserContext* other, UseCounter*);
+  static CSSParserContext* create(const CSSParserContext* other,
+                                  const Document* useCounterDocument);
 
   static CSSParserContext* create(CSSParserMode,
                                   SelectorProfile = DynamicProfile,
-                                  UseCounter* = nullptr);
-  // FIXME: We shouldn't need the UseCounter argument as we could infer it from
-  // the Document but some callers want to disable use counting (e.g. the
-  // WebInspector).
-  static CSSParserContext* create(const Document&, UseCounter*);
+                                  const Document* useCounterDocument = nullptr);
+  static CSSParserContext* create(const Document&,
+                                  const Document* useCounterDocument);
   static CSSParserContext* create(const Document&,
                                   const KURL& baseURLOverride = KURL(),
                                   const String& charset = emptyString,
                                   SelectorProfile = DynamicProfile,
-                                  UseCounter* = nullptr);
+                                  const Document* useCounterDocument = nullptr);
 
   bool operator==(const CSSParserContext&) const;
   bool operator!=(const CSSParserContext& other) const {
     return !(*this == other);
   }
 
   CSSParserMode mode() const { return m_mode; }
   CSSParserMode matchMode() const { return m_matchMode; }
   const KURL& baseURL() const { return m_baseURL; }
   const String& charset() const { return m_charset; }
@@ skipping 12 matching lines @@
   // FIXME: These setters shouldn't exist, however the current lifetime of
   // CSSParserContext is not well understood and thus we sometimes need to
   // override these fields.
   void setMode(CSSParserMode mode) { m_mode = mode; }
   void setBaseURL(const KURL& baseURL) { m_baseURL = baseURL; }
   void setCharset(const String& charset) { m_charset = charset; }
   void setReferrer(const Referrer& referrer) { m_referrer = referrer; }
 
   KURL completeURL(const String& url) const;
 
-  // This may return nullptr if counting is disabled.
-  // See comments on constructors.
-  UseCounter* useCounter() const { return m_useCounter; }
-  bool isUseCounterRecordingEnabled() const { return m_useCounter; }
+  void count(UseCounter::Feature) const;
+  void count(CSSParserMode, CSSPropertyID) const;
+  bool isUseCounterRecordingEnabled() const { return m_useCounterDocument; }
+  bool isUseCounterDocumentHandleEqual(const Document* other) const;
 
   ContentSecurityPolicyDisposition shouldCheckContentSecurityPolicy() const {
     return m_shouldCheckContentSecurityPolicy;
   }
 
-  DEFINE_INLINE_TRACE() {}
+  DECLARE_TRACE();
 
  private:
   CSSParserContext(const KURL& baseURL,
                    const String& charset,
                    CSSParserMode,
                    CSSParserMode matchMode,
                    SelectorProfile,
                    const Referrer&,
                    bool isHTMLDocument,
                    bool useLegacyBackgroundSizeShorthandBehavior,
                    ContentSecurityPolicyDisposition,
-                   UseCounter*);
+                   const Document* useCounterDocument);
 
   KURL m_baseURL;
   String m_charset;
   CSSParserMode m_mode;
   CSSParserMode m_matchMode;
   SelectorProfile m_profile = DynamicProfile;
   Referrer m_referrer;
   bool m_isHTMLDocument;
   bool m_useLegacyBackgroundSizeShorthandBehavior;
   ContentSecurityPolicyDisposition m_shouldCheckContentSecurityPolicy;
 
-  UseCounter* m_useCounter;
+  WeakMember<const Document> m_useCounterDocument;

[haraken, 2017/02/08 01:40:00]

Does this need to be weak? In other words, can CSSParserContext outlive the
Document?

If no, we should just use a Member.

[haraken, 2017/02/08 01:40:00]

Also I'd rename m_useCounterDocument to m_document, since it's just a document
associated with the CSSParserContext.

[Bret, 2017/02/09 01:53:33]

Rename done.

I believe it has to be a WeakMember. I originally made it a Member and
CSSLazyParsingTest.ChangeDocuments failed because m_cachedContents was holding
onto the CSSParserContext handle, which caused the Document to not be collected,
which caused CSSLazyParsingState's weak Document handle to still be valid, which
caused it to not refresh it from m_owningContents, which caused the
isDocumentHandleEqual call to return true, which caused it to use the old
UseCounter instance. Maybe something there can be simplified but I don't have
enough knowledge to know where.

[haraken, 2017/02/09 02:01:42]

Yeah, I think we should look into what's going on there and clean up.

This CL LGTM though.

 };
 
 CORE_EXPORT const CSSParserContext* strictCSSParserContext();
 
 }  // namespace blink
 
 #endif  // CSSParserContext_h