Use WeakPtrs to communicate between the HTMLDocumentParser and the BackgroundHTMLParser
authorabarth@webkit.org <abarth@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 Feb 2013 19:30:54 +0000 (19:30 +0000)
committerabarth@webkit.org <abarth@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 Feb 2013 19:30:54 +0000 (19:30 +0000)
https://bugs.webkit.org/show_bug.cgi?id=107190

Reviewed by Eric Seidel.

Source/WebCore:

This patch replaces the parser map with WeakPtr. We now use WeakPtrs to
communicate from the main thread to the background thread. (We were
already using WeakPtrs to communicate from the background thread to the
main thread.) This change lets us remove a bunch of boilerplate code.

* html/parser/BackgroundHTMLParser.cpp:
(WebCore::BackgroundHTMLParser::BackgroundHTMLParser):
(WebCore::BackgroundHTMLParser::stop):
(WebCore):
* html/parser/BackgroundHTMLParser.h:
(WebCore::BackgroundHTMLParser::create):
(BackgroundHTMLParser):
* html/parser/HTMLDocumentParser.cpp:
(WebCore::HTMLDocumentParser::didFailSpeculation):
(WebCore::HTMLDocumentParser::startBackgroundParser):
(WebCore::HTMLDocumentParser::stopBackgroundParser):
(WebCore::HTMLDocumentParser::append):
(WebCore::HTMLDocumentParser::finish):
* html/parser/HTMLDocumentParser.h:
(WebCore):
(HTMLDocumentParser):

Source/WTF:

Add the ability to create an unbound weak reference. This facility lets
you start sending messages to a WeakPtr on another thread before the
object backing the WeakPtr has actually been created.

* wtf/WeakPtr.h:
(WTF::WeakReference::createUnbound):
(WTF::WeakReference::bindTo):
(WeakReference):
(WTF::WeakReference::WeakReference):
(WTF::WeakPtr::WeakPtr):
(WeakPtr):
(WTF::WeakPtrFactory::WeakPtrFactory):
(WeakPtrFactory):
(WTF::WeakPtrFactory::revokeAll):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@142305 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WTF/ChangeLog
Source/WTF/wtf/WeakPtr.h
Source/WebCore/ChangeLog
Source/WebCore/html/parser/BackgroundHTMLParser.cpp
Source/WebCore/html/parser/BackgroundHTMLParser.h
Source/WebCore/html/parser/HTMLDocumentParser.cpp
Source/WebCore/html/parser/HTMLDocumentParser.h

index abc7815ae22efd800944b0987810196f13ed2729..7521402692212ea1b35f67cf3b5e7ea5d9f6505e 100644 (file)
@@ -1,3 +1,25 @@
+2013-02-08  Adam Barth  <abarth@webkit.org>
+
+        Use WeakPtrs to communicate between the HTMLDocumentParser and the BackgroundHTMLParser
+        https://bugs.webkit.org/show_bug.cgi?id=107190
+
+        Reviewed by Eric Seidel.
+
+        Add the ability to create an unbound weak reference. This facility lets
+        you start sending messages to a WeakPtr on another thread before the
+        object backing the WeakPtr has actually been created.
+
+        * wtf/WeakPtr.h:
+        (WTF::WeakReference::createUnbound):
+        (WTF::WeakReference::bindTo):
+        (WeakReference):
+        (WTF::WeakReference::WeakReference):
+        (WTF::WeakPtr::WeakPtr):
+        (WeakPtr):
+        (WTF::WeakPtrFactory::WeakPtrFactory):
+        (WeakPtrFactory):
+        (WTF::WeakPtrFactory::revokeAll):
+
 2013-02-08  Martin Robinson  <mrobinson@igalia.com>
 
         [GTK] Add an experimental gyp build
index 289a10c18cae0808e38942dbc9d206b180611d88..aa15318a4b53410cf9bd302710989ad3e5420b6a 100644 (file)
 
 namespace WTF {
 
-namespace Internal {
-
 template<typename T>
 class WeakReference : public ThreadSafeRefCounted<WeakReference<T> > {
     WTF_MAKE_NONCOPYABLE(WeakReference<T>);
     WTF_MAKE_FAST_ALLOCATED;
 public:
     static PassRefPtr<WeakReference<T> > create(T* ptr) { return adoptRef(new WeakReference(ptr)); }
+    static PassRefPtr<WeakReference<T> > createUnbound() { return adoptRef(new WeakReference()); }
 
     T* get() const
     {
@@ -55,7 +54,18 @@ public:
         m_ptr = 0;
     }
 
+    void bindTo(T* ptr)
+    {
+        ASSERT(!m_ptr);
+#ifndef NDEBUG
+        m_boundThread = currentThread();
+#endif
+        m_ptr = ptr;
+    }
+
 private:
+    WeakReference() : m_ptr(0) { }
+
     explicit WeakReference(T* ptr)
         : m_ptr(ptr)
 #ifndef NDEBUG
@@ -70,19 +80,17 @@ private:
 #endif
 };
 
-}
-
 template<typename T>
 class WeakPtr {
     WTF_MAKE_FAST_ALLOCATED;
 public:
     WeakPtr() { }
-    WeakPtr(PassRefPtr<Internal::WeakReference<T> > ref) : m_ref(ref) { }
+    WeakPtr(PassRefPtr<WeakReference<T> > ref) : m_ref(ref) { }
 
     T* get() const { return m_ref->get(); }
 
 private:
-    RefPtr<Internal::WeakReference<T> > m_ref;
+    RefPtr<WeakReference<T> > m_ref;
 };
 
 template<typename T>
@@ -90,7 +98,14 @@ class WeakPtrFactory {
     WTF_MAKE_NONCOPYABLE(WeakPtrFactory<T>);
     WTF_MAKE_FAST_ALLOCATED;
 public:
-    explicit WeakPtrFactory(T* ptr) { m_ref = Internal::WeakReference<T>::create(ptr); }
+    explicit WeakPtrFactory(T* ptr) : m_ref(WeakReference<T>::create(ptr)) { }
+
+    WeakPtrFactory(PassRefPtr<WeakReference<T> > ref, T* ptr)
+        : m_ref(ref)
+    {
+        m_ref->bindTo(ptr);
+    }
+
     ~WeakPtrFactory() { m_ref->clear(); }
 
     // We should consider having createWeakPtr populate m_ref the first time createWeakPtr is called.
@@ -101,16 +116,17 @@ public:
         T* ptr = m_ref->get();
         m_ref->clear();
         // We create a new WeakReference so that future calls to createWeakPtr() create nonzero WeakPtrs.
-        m_ref = Internal::WeakReference<T>::create(ptr);
+        m_ref = WeakReference<T>::create(ptr);
     }
 
 private:
-    RefPtr<Internal::WeakReference<T> > m_ref;
+    RefPtr<WeakReference<T> > m_ref;
 };
 
 } // namespace WTF
 
 using WTF::WeakPtr;
 using WTF::WeakPtrFactory;
+using WTF::WeakReference;
 
 #endif
index a2a9a44b9bf2bbacbd8a1c1241fac1ec89a3e05c..842983780dea9f6aabe7bf834dfb190c875f806f 100644 (file)
@@ -1,3 +1,32 @@
+2013-02-08  Adam Barth  <abarth@webkit.org>
+
+        Use WeakPtrs to communicate between the HTMLDocumentParser and the BackgroundHTMLParser
+        https://bugs.webkit.org/show_bug.cgi?id=107190
+
+        Reviewed by Eric Seidel.
+
+        This patch replaces the parser map with WeakPtr. We now use WeakPtrs to
+        communicate from the main thread to the background thread. (We were
+        already using WeakPtrs to communicate from the background thread to the
+        main thread.) This change lets us remove a bunch of boilerplate code.
+
+        * html/parser/BackgroundHTMLParser.cpp:
+        (WebCore::BackgroundHTMLParser::BackgroundHTMLParser):
+        (WebCore::BackgroundHTMLParser::stop):
+        (WebCore):
+        * html/parser/BackgroundHTMLParser.h:
+        (WebCore::BackgroundHTMLParser::create):
+        (BackgroundHTMLParser):
+        * html/parser/HTMLDocumentParser.cpp:
+        (WebCore::HTMLDocumentParser::didFailSpeculation):
+        (WebCore::HTMLDocumentParser::startBackgroundParser):
+        (WebCore::HTMLDocumentParser::stopBackgroundParser):
+        (WebCore::HTMLDocumentParser::append):
+        (WebCore::HTMLDocumentParser::finish):
+        * html/parser/HTMLDocumentParser.h:
+        (WebCore):
+        (HTMLDocumentParser):
+
 2013-02-07  Roger Fong  <roger_fong@apple.com>
 
         VS2010 WebCore TestSupport project.
index fe8e331bb3cfc83ae3449e3a0eeb0c85540d251a..c0676b509596b9e5e7bf9a4bbaada99a0d1c1546 100644 (file)
@@ -58,22 +58,9 @@ static void checkThatTokensAreSafeToSendToAnotherThread(const CompactHTMLTokenSt
 // FIXME: Tune this constant based on a benchmark. The current value was choosen arbitrarily.
 static const size_t pendingTokenLimit = 4000;
 
-ParserMap& parserMap()
-{
-    // This initialization assumes that this will be initialize on the main thread before
-    // any parser thread is started.
-    static ParserMap* sParserMap = new ParserMap;
-    return *sParserMap;
-}
-
-ParserMap::BackgroundParserMap& ParserMap::backgroundParsers()
-{
-    ASSERT(HTMLParserThread::shared()->threadId() == currentThread());
-    return m_backgroundParsers;
-}
-
-BackgroundHTMLParser::BackgroundHTMLParser(const HTMLParserOptions& options, const WeakPtr<HTMLDocumentParser>& parser, PassOwnPtr<XSSAuditor> xssAuditor)
+BackgroundHTMLParser::BackgroundHTMLParser(PassRefPtr<WeakReference<BackgroundHTMLParser> > reference, const HTMLParserOptions& options, const WeakPtr<HTMLDocumentParser>& parser, PassOwnPtr<XSSAuditor> xssAuditor)
     : m_inForeignContent(false)
+    , m_weakFactory(reference, this)
     , m_token(adoptPtr(new HTMLToken))
     , m_tokenizer(HTMLTokenizer::create(options))
     , m_options(options)
@@ -104,6 +91,11 @@ void BackgroundHTMLParser::finish()
     pumpTokenizer();
 }
 
+void BackgroundHTMLParser::stop()
+{
+    delete this;
+}
+
 void BackgroundHTMLParser::markEndOfFile()
 {
     // FIXME: This should use InputStreamPreprocessor::endOfFileMarker
@@ -194,36 +186,6 @@ void BackgroundHTMLParser::sendTokensToMainThread()
     m_pendingTokens = adoptPtr(new CompactHTMLTokenStream);
 }
 
-void BackgroundHTMLParser::createPartial(ParserIdentifier identifier, const HTMLParserOptions& options, const WeakPtr<HTMLDocumentParser>& parser, PassOwnPtr<XSSAuditor> xssAuditor)
-{
-    ASSERT(!parserMap().backgroundParsers().get(identifier));
-    parserMap().backgroundParsers().set(identifier, BackgroundHTMLParser::create(options, parser, xssAuditor));
-}
-
-void BackgroundHTMLParser::stopPartial(ParserIdentifier identifier)
-{
-    parserMap().backgroundParsers().remove(identifier);
-}
-
-void BackgroundHTMLParser::appendPartial(ParserIdentifier identifier, const String& input)
-{
-    ASSERT(!input.impl() || input.impl()->hasOneRef() || input.isEmpty());
-    if (BackgroundHTMLParser* parser = parserMap().backgroundParsers().get(identifier))
-        parser->append(input);
-}
-
-void BackgroundHTMLParser::resumeFromPartial(ParserIdentifier identifier, const WeakPtr<HTMLDocumentParser>& parser, PassOwnPtr<HTMLToken> token, PassOwnPtr<HTMLTokenizer> tokenizer, HTMLInputCheckpoint checkpoint)
-{
-    if (BackgroundHTMLParser* backgroundParser = parserMap().backgroundParsers().get(identifier))
-        backgroundParser->resumeFrom(parser, token, tokenizer, checkpoint);
-}
-
-void BackgroundHTMLParser::finishPartial(ParserIdentifier identifier)
-{
-    if (BackgroundHTMLParser* parser = parserMap().backgroundParsers().get(identifier))
-        parser->finish();
-}
-
 }
 
 #endif // ENABLE(THREADED_HTML_PARSER)
index 6d0d8e369a3c85574a34a5c2afe95bdca505adff..6a39310b40145ddca57c0f9937a91099fc009627 100644 (file)
@@ -47,23 +47,19 @@ class XSSAuditor;
 class BackgroundHTMLParser {
     WTF_MAKE_FAST_ALLOCATED;
 public:
-    void append(const String&);
-    void resumeFrom(const WeakPtr<HTMLDocumentParser>&, PassOwnPtr<HTMLToken>, PassOwnPtr<HTMLTokenizer>, HTMLInputCheckpoint);
-    void finish();
-
-    static PassOwnPtr<BackgroundHTMLParser> create(const HTMLParserOptions& options, const WeakPtr<HTMLDocumentParser>& parser, PassOwnPtr<XSSAuditor> xssAuditor)
+    static void create(PassRefPtr<WeakReference<BackgroundHTMLParser> > reference, const HTMLParserOptions& options, const WeakPtr<HTMLDocumentParser>& parser, PassOwnPtr<XSSAuditor> xssAuditor)
     {
-        return adoptPtr(new BackgroundHTMLParser(options, parser, xssAuditor));
+        new BackgroundHTMLParser(reference, options, parser, xssAuditor);
+        // Caller must free by calling stop().
     }
 
-    static void createPartial(ParserIdentifier, const HTMLParserOptions&, const WeakPtr<HTMLDocumentParser>&, PassOwnPtr<XSSAuditor>);
-    static void stopPartial(ParserIdentifier);
-    static void appendPartial(ParserIdentifier, const String& input);
-    static void resumeFromPartial(ParserIdentifier, const WeakPtr<HTMLDocumentParser>&, PassOwnPtr<HTMLToken>, PassOwnPtr<HTMLTokenizer>, HTMLInputCheckpoint);
-    static void finishPartial(ParserIdentifier);
+    void append(const String&);
+    void resumeFrom(const WeakPtr<HTMLDocumentParser>&, PassOwnPtr<HTMLToken>, PassOwnPtr<HTMLTokenizer>, HTMLInputCheckpoint);
+    void finish();
+    void stop();
 
 private:
-    BackgroundHTMLParser(const HTMLParserOptions&, const WeakPtr<HTMLDocumentParser>&, PassOwnPtr<XSSAuditor>);
+    BackgroundHTMLParser(PassRefPtr<WeakReference<BackgroundHTMLParser> >, const HTMLParserOptions&, const WeakPtr<HTMLDocumentParser>&, PassOwnPtr<XSSAuditor>);
 
     void markEndOfFile();
     void pumpTokenizer();
@@ -72,6 +68,7 @@ private:
     void sendTokensToMainThread();
 
     bool m_inForeignContent; // FIXME: We need a stack of foreign content markers.
+    WeakPtrFactory<BackgroundHTMLParser> m_weakFactory;
     BackgroundHTMLInputStream m_input;
     HTMLSourceTracker m_sourceTracker;
     OwnPtr<HTMLToken> m_token;
@@ -82,23 +79,6 @@ private:
     OwnPtr<XSSAuditor> m_xssAuditor;
 };
 
-class ParserMap {
-public:
-    static ParserIdentifier identifierForParser(HTMLDocumentParser* parser)
-    {
-        return reinterpret_cast<ParserIdentifier>(parser);
-    }
-
-    typedef HashMap<ParserIdentifier, OwnPtr<BackgroundHTMLParser> > BackgroundParserMap;
-
-    BackgroundParserMap& backgroundParsers();
-
-private:
-    BackgroundParserMap m_backgroundParsers;
-};
-
-ParserMap& parserMap();
-
 }
 
 #endif // ENABLE(THREADED_HTML_PARSER)
index 1e7c0b88de34f213cba1b855a97445675f0bae4f..2b0caf12bd58370f14b6b96d3c2c06f1c129f3cf 100644 (file)
@@ -301,9 +301,8 @@ void HTMLDocumentParser::didFailSpeculation(PassOwnPtr<HTMLToken> token, PassOwn
     m_weakFactory.revokeAll();
     m_speculations.clear();
 
-    ParserIdentifier identifier = ParserMap::identifierForParser(this);
-    HTMLParserThread::shared()->postTask(bind(&BackgroundHTMLParser::resumeFromPartial,
-        identifier, m_weakFactory.createWeakPtr(), token, tokenizer, m_currentChunk->checkpoint));
+    HTMLParserThread::shared()->postTask(bind(&BackgroundHTMLParser::resumeFrom,
+        m_backgroundParser, m_weakFactory.createWeakPtr(), token, tokenizer, m_currentChunk->checkpoint));
 }
 
 void HTMLDocumentParser::processParsedChunkFromBackgroundParser(PassOwnPtr<ParsedChunk> chunk)
@@ -512,12 +511,14 @@ void HTMLDocumentParser::startBackgroundParser()
     ASSERT(!m_haveBackgroundParser);
     m_haveBackgroundParser = true;
 
+    RefPtr<WeakReference<BackgroundHTMLParser> > reference = WeakReference<BackgroundHTMLParser>::createUnbound();
+    m_backgroundParser = WeakPtr<BackgroundHTMLParser>(reference);
+
     WeakPtr<HTMLDocumentParser> parser = m_weakFactory.createWeakPtr();
     OwnPtr<XSSAuditor> xssAuditor = adoptPtr(new XSSAuditor);
     xssAuditor->init(document());
     ASSERT(xssAuditor->isSafeToSendToAnotherThread());
-    ParserIdentifier identifier = ParserMap::identifierForParser(this);
-    HTMLParserThread::shared()->postTask(bind(&BackgroundHTMLParser::createPartial, identifier, m_options, parser, xssAuditor.release()));
+    HTMLParserThread::shared()->postTask(bind(&BackgroundHTMLParser::create, reference.release(), m_options, parser, xssAuditor.release()));
 }
 
 void HTMLDocumentParser::stopBackgroundParser()
@@ -526,8 +527,7 @@ void HTMLDocumentParser::stopBackgroundParser()
     ASSERT(m_haveBackgroundParser);
     m_haveBackgroundParser = false;
 
-    ParserIdentifier identifier = ParserMap::identifierForParser(this);
-    HTMLParserThread::shared()->postTask(bind(&BackgroundHTMLParser::stopPartial, identifier));
+    HTMLParserThread::shared()->postTask(bind(&BackgroundHTMLParser::stop, m_backgroundParser));
     m_weakFactory.revokeAll();
 }
 
@@ -543,9 +543,8 @@ void HTMLDocumentParser::append(const SegmentedString& source)
         if (!m_haveBackgroundParser)
             startBackgroundParser();
 
-        ParserIdentifier identifier = ParserMap::identifierForParser(this);
-        const Closure& appendPartial = bind(&BackgroundHTMLParser::appendPartial, identifier, source.toString().isolatedCopy());
-        HTMLParserThread::shared()->postTask(appendPartial);
+        HTMLParserThread::shared()->postTask(bind(
+            &BackgroundHTMLParser::append, m_backgroundParser, source.toString().isolatedCopy()));
         return;
     }
 #endif
@@ -645,7 +644,7 @@ void HTMLDocumentParser::finish()
     // a background parser. In those cases, we ignore shouldUseThreading()
     // and fall through to the non-threading case.
     if (m_haveBackgroundParser) {
-        HTMLParserThread::shared()->postTask(bind(&BackgroundHTMLParser::finishPartial, ParserMap::identifierForParser(this)));
+        HTMLParserThread::shared()->postTask(bind(&BackgroundHTMLParser::finish, m_backgroundParser));
         return;
     }
     if (shouldUseThreading() && !wasCreatedByScript()) {
index 95722f1b8f9570a56f75a48082ecff84fba94476..7c4278525de92c05912ba42c448b7350f6a0f879 100644 (file)
@@ -46,6 +46,7 @@
 
 namespace WebCore {
 
+class BackgroundHTMLParser;
 class CompactHTMLToken;
 class Document;
 class DocumentFragment;
@@ -182,6 +183,7 @@ private:
     OwnPtr<ParsedChunk> m_currentChunk;
     Deque<OwnPtr<ParsedChunk> > m_speculations;
     WeakPtrFactory<HTMLDocumentParser> m_weakFactory;
+    WeakPtr<BackgroundHTMLParser> m_backgroundParser;
 #endif
 
     bool m_endWasDelayed;