Title: [142305] trunk/Source
Revision
142305
Author
aba...@webkit.org
Date
2013-02-08 11:30:54 -0800 (Fri, 08 Feb 2013)

Log Message

Use WeakPtrs to communicate between the HTMLDocumentParser and the BackgroundHTMLParser
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):

Modified Paths

Diff

Modified: trunk/Source/WTF/ChangeLog (142304 => 142305)


--- trunk/Source/WTF/ChangeLog	2013-02-08 19:09:13 UTC (rev 142304)
+++ trunk/Source/WTF/ChangeLog	2013-02-08 19:30:54 UTC (rev 142305)
@@ -1,3 +1,25 @@
+2013-02-08  Adam Barth  <aba...@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  <mrobin...@igalia.com>
 
         [GTK] Add an experimental gyp build

Modified: trunk/Source/WTF/wtf/WeakPtr.h (142304 => 142305)


--- trunk/Source/WTF/wtf/WeakPtr.h	2013-02-08 19:09:13 UTC (rev 142304)
+++ trunk/Source/WTF/wtf/WeakPtr.h	2013-02-08 19:30:54 UTC (rev 142305)
@@ -34,14 +34,13 @@
 
 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 @@
         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 @@
 #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 @@
     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 @@
         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

Modified: trunk/Source/WebCore/ChangeLog (142304 => 142305)


--- trunk/Source/WebCore/ChangeLog	2013-02-08 19:09:13 UTC (rev 142304)
+++ trunk/Source/WebCore/ChangeLog	2013-02-08 19:30:54 UTC (rev 142305)
@@ -1,3 +1,32 @@
+2013-02-08  Adam Barth  <aba...@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_f...@apple.com>
 
         VS2010 WebCore TestSupport project.

Modified: trunk/Source/WebCore/html/parser/BackgroundHTMLParser.cpp (142304 => 142305)


--- trunk/Source/WebCore/html/parser/BackgroundHTMLParser.cpp	2013-02-08 19:09:13 UTC (rev 142304)
+++ trunk/Source/WebCore/html/parser/BackgroundHTMLParser.cpp	2013-02-08 19:30:54 UTC (rev 142305)
@@ -58,22 +58,9 @@
 // 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 @@
     pumpTokenizer();
 }
 
+void BackgroundHTMLParser::stop()
+{
+    delete this;
+}
+
 void BackgroundHTMLParser::markEndOfFile()
 {
     // FIXME: This should use InputStreamPreprocessor::endOfFileMarker
@@ -194,36 +186,6 @@
     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)

Modified: trunk/Source/WebCore/html/parser/BackgroundHTMLParser.h (142304 => 142305)


--- trunk/Source/WebCore/html/parser/BackgroundHTMLParser.h	2013-02-08 19:09:13 UTC (rev 142304)
+++ trunk/Source/WebCore/html/parser/BackgroundHTMLParser.h	2013-02-08 19:30:54 UTC (rev 142305)
@@ -47,23 +47,19 @@
 class BackgroundHTMLParser {
     WTF_MAKE_FAST_ALLOCATED;
 public:
+    static void create(PassRefPtr<WeakReference<BackgroundHTMLParser> > reference, const HTMLParserOptions& options, const WeakPtr<HTMLDocumentParser>& parser, PassOwnPtr<XSSAuditor> xssAuditor)
+    {
+        new BackgroundHTMLParser(reference, options, parser, xssAuditor);
+        // Caller must free by calling stop().
+    }
+
     void append(const String&);
     void resumeFrom(const WeakPtr<HTMLDocumentParser>&, PassOwnPtr<HTMLToken>, PassOwnPtr<HTMLTokenizer>, HTMLInputCheckpoint);
     void finish();
+    void stop();
 
-    static PassOwnPtr<BackgroundHTMLParser> create(const HTMLParserOptions& options, const WeakPtr<HTMLDocumentParser>& parser, PassOwnPtr<XSSAuditor> xssAuditor)
-    {
-        return adoptPtr(new BackgroundHTMLParser(options, parser, xssAuditor));
-    }
-
-    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);
-
 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 @@
     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 @@
     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)

Modified: trunk/Source/WebCore/html/parser/HTMLDocumentParser.cpp (142304 => 142305)


--- trunk/Source/WebCore/html/parser/HTMLDocumentParser.cpp	2013-02-08 19:09:13 UTC (rev 142304)
+++ trunk/Source/WebCore/html/parser/HTMLDocumentParser.cpp	2013-02-08 19:30:54 UTC (rev 142305)
@@ -301,9 +301,8 @@
     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 @@
     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 @@
     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 @@
         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 @@
     // 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()) {

Modified: trunk/Source/WebCore/html/parser/HTMLDocumentParser.h (142304 => 142305)


--- trunk/Source/WebCore/html/parser/HTMLDocumentParser.h	2013-02-08 19:09:13 UTC (rev 142304)
+++ trunk/Source/WebCore/html/parser/HTMLDocumentParser.h	2013-02-08 19:30:54 UTC (rev 142305)
@@ -46,6 +46,7 @@
 
 namespace WebCore {
 
+class BackgroundHTMLParser;
 class CompactHTMLToken;
 class Document;
 class DocumentFragment;
@@ -182,6 +183,7 @@
     OwnPtr<ParsedChunk> m_currentChunk;
     Deque<OwnPtr<ParsedChunk> > m_speculations;
     WeakPtrFactory<HTMLDocumentParser> m_weakFactory;
+    WeakPtr<BackgroundHTMLParser> m_backgroundParser;
 #endif
 
     bool m_endWasDelayed;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to