Changeset 140589 in webkit


Ignore:
Timestamp:
Jan 23, 2013, 2:15:54 PM (12 years ago)
Author:
abarth@webkit.org
Message:

BackgroundHTMLParser::sendTokensToMainThread should use bind
​https://bugs.webkit.org/show_bug.cgi?id=107637

Reviewed by Eric Seidel.

Source/WebCore:

This patch replaces our hand-written implementation of bind for
didReceiveTokensFromBackgroundParser with bind from Functional.h. To
use the generic version of bind, we need to switch to using WeakPtr to
hold a reference to the main thread parser in the BackgroundHTMLParser.

  • html/parser/BackgroundHTMLParser.cpp:

(WebCore::BackgroundHTMLParser::BackgroundHTMLParser):
(WebCore::BackgroundHTMLParser::sendTokensToMainThread):
(WebCore::BackgroundHTMLParser::createPartial):

  • html/parser/BackgroundHTMLParser.h:

(WebCore::BackgroundHTMLParser::create):
(BackgroundHTMLParser):
(ParserMap):

  • html/parser/HTMLDocumentParser.cpp:

(WebCore::HTMLDocumentParser::HTMLDocumentParser):
(WebCore::HTMLDocumentParser::startBackgroundParser):
(WebCore::HTMLDocumentParser::stopBackgroundParser):

  • html/parser/HTMLDocumentParser.h:

(HTMLDocumentParser):

Source/WTF:

  • wtf/Functional.h:
    • I had to re-work the approach to validating WeakPtr |this| arguments a bit. Previously you couldn't pass a WeakPtr as a non-|this| argument to a function because we would try to unwrap it into a raw pointer.
  • wtf/WeakPtr.h:

(WTF::WeakPtrFactory::revokeAll):
(WeakPtrFactory):

  • Let clients revoke all outstanding WeakPtrs without needing to destroy the WeakPtrFactory.
  • wtf/chromium/MainThreadChromium.cpp:

(WTF::callFunctionObject):
(WTF::callOnMainThread):

  • Implement callOnMainThread for Function objects. The non-Chromium implementation of MainThread.cpp already implements this feature.
Location:
trunk/Source
Files:
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WTF/ChangeLog

    r140579 r140589  
     12013-01-23  Adam Barth  <abarth@webkit.org>
     2
     3        BackgroundHTMLParser::sendTokensToMainThread should use bind
     4        https://bugs.webkit.org/show_bug.cgi?id=107637
     5
     6        Reviewed by Eric Seidel.
     7
     8        * wtf/Functional.h:
     9            - I had to re-work the approach to validating WeakPtr |this|
     10              arguments a bit. Previously you couldn't pass a WeakPtr as a
     11              non-|this| argument to a function because we would try to unwrap
     12              it into a raw pointer.
     13        * wtf/WeakPtr.h:
     14        (WTF::WeakPtrFactory::revokeAll):
     15        (WeakPtrFactory):
     16            - Let clients revoke all outstanding WeakPtrs without needing to
     17              destroy the WeakPtrFactory.
     18        * wtf/chromium/MainThreadChromium.cpp:
     19        (WTF::callFunctionObject):
     20        (WTF::callOnMainThread):
     21            - Implement callOnMainThread for Function objects. The non-Chromium
     22              implementation of MainThread.cpp already implements this feature.
     23
    1242013-01-23  Justin Schuh  <jschuh@chromium.org>
    225

  • trunk/Source/WTF/wtf/Functional.h

    r140040 r140589  
    8484    typedef R ResultType;
    8585    static const bool shouldRefFirstParameter = false;
    86     static const bool shouldValidateFirstParameter = false;
    8786
    8887    explicit FunctionWrapper(R (*function)())
    … …  
    105104    typedef R ResultType;
    106105    static const bool shouldRefFirstParameter = false;
    107     static const bool shouldValidateFirstParameter = false;
    108106
    109107    explicit FunctionWrapper(R (*function)(P1))
    … …  
    126124    typedef R ResultType;
    127125    static const bool shouldRefFirstParameter = false;
    128     static const bool shouldValidateFirstParameter = false;
    129126
    130127    explicit FunctionWrapper(R (*function)(P1, P2))
    … …  
    147144    typedef R ResultType;
    148145    static const bool shouldRefFirstParameter = false;
    149     static const bool shouldValidateFirstParameter = false;
    150146
    151147    explicit FunctionWrapper(R (*function)(P1, P2, P3))
    … …  
    168164    typedef R ResultType;
    169165    static const bool shouldRefFirstParameter = HasRefAndDeref<C>::value;
    170     static const bool shouldValidateFirstParameter = true;
    171166
    172167    explicit FunctionWrapper(R (C::*function)())
    … …  
    178173    {
    179174        return (c->*m_function)();
     175    }
     176
     177    R operator()(const WeakPtr<C>& c)
     178    {
     179        C* obj = c.get();
     180        if (!obj)
     181            return R();
     182        return (obj->*m_function)();
    180183    }
    181184
    … …  
    189192    typedef R ResultType;
    190193    static const bool shouldRefFirstParameter = HasRefAndDeref<C>::value;
    191     static const bool shouldValidateFirstParameter = true;
    192194
    193195    explicit FunctionWrapper(R (C::*function)(P1))
    … …  
    199201    {
    200202        return (c->*m_function)(p1);
     203    }
     204
     205    R operator()(const WeakPtr<C>& c, P1 p1)
     206    {
     207        C* obj = c.get();
     208        if (!obj)
     209            return R();
     210        return (obj->*m_function)(p1);
    201211    }
    202212
    … …  
    210220    typedef R ResultType;
    211221    static const bool shouldRefFirstParameter = HasRefAndDeref<C>::value;
    212     static const bool shouldValidateFirstParameter = true;
    213222
    214223    explicit FunctionWrapper(R (C::*function)(P1, P2))
    … …  
    220229    {
    221230        return (c->*m_function)(p1, p2);
     231    }
     232
     233    R operator()(const WeakPtr<C>& c, P1 p1, P2 p2)
     234    {
     235        C* obj = c.get();
     236        if (!obj)
     237            return R();
     238        return (obj->*m_function)(p1, p2);
    222239    }
    223240
    … …  
    231248    typedef R ResultType;
    232249    static const bool shouldRefFirstParameter = HasRefAndDeref<C>::value;
    233     static const bool shouldValidateFirstParameter = true;
    234250
    235251    explicit FunctionWrapper(R (C::*function)(P1, P2, P3))
    … …  
    241257    {
    242258        return (c->*m_function)(p1, p2, p3);
     259    }
     260
     261    R operator()(const WeakPtr<C>& c, P1 p1, P2 p2, P3 p3)
     262    {
     263        C* obj = c.get();
     264        if (!obj)
     265            return R();
     266        return (obj->*m_function)(p1, p2, p3);
    243267    }
    244268
    … …  
    252276    typedef R ResultType;
    253277    static const bool shouldRefFirstParameter = HasRefAndDeref<C>::value;
    254     static const bool shouldValidateFirstParameter = true;
    255278
    256279    explicit FunctionWrapper(R (C::*function)(P1, P2, P3, P4))
    … …  
    262285    {
    263286        return (c->*m_function)(p1, p2, p3, p4);
     287    }
     288
     289    R operator()(const WeakPtr<C>& c, P1 p1, P2 p2, P3 p3, P4 p4)
     290    {
     291        C* obj = c.get();
     292        if (!obj)
     293            return R();
     294        return (obj->*m_function)(p1, p2, p3, p4);
    264295    }
    265296
    … …  
    273304    typedef R ResultType;
    274305    static const bool shouldRefFirstParameter = HasRefAndDeref<C>::value;
    275     static const bool shouldValidateFirstParameter = true;
    276306
    277307    explicit FunctionWrapper(R (C::*function)(P1, P2, P3, P4, P5))
    … …  
    283313    {
    284314        return (c->*m_function)(p1, p2, p3, p4, p5);
     315    }
     316
     317    R operator()(const WeakPtr<C>& c, P1 p1, P2 p2, P3 p3, P4 p4, P5 p5)
     318    {
     319        C* obj = c.get();
     320        if (!obj)
     321            return R();
     322        return (obj->*m_function)(p1, p2, p3, p4, p5);
    285323    }
    286324
    … …  
    295333    typedef R ResultType;
    296334    static const bool shouldRefFirstParameter = false;
    297     static const bool shouldValidateFirstParameter = false;
    298335
    299336    explicit FunctionWrapper(R (^block)())
    … …  
    336373
    337374    static StorageType wrap(const T& value) { return value; }
    338     static bool validate(const StorageType&) { return true; }
    339375    static const T& unwrap(const StorageType& value) { return value; }
    340376};
    … …  
    344380
    345381    static StorageType wrap(PassRefPtr<T> value) { return value; }
    346     static bool validate(const StorageType&) { return true; }
    347382    static T* unwrap(const StorageType& value) { return value.get(); }
    348383};
    … …  
    352387
    353388    static StorageType wrap(RefPtr<T> value) { return value.release(); }
    354     static bool validate(const StorageType&) { return true; }
    355389    static T* unwrap(const StorageType& value) { return value.get(); }
    356390};
    357 
    358 template<typename T> struct ParamStorageTraits<WeakPtr<T> > {
    359     typedef WeakPtr<T> StorageType;
    360 
    361     static StorageType wrap(WeakPtr<T> value) { return value; }
    362     static bool validate(const StorageType& value) { return value.get(); }
    363     static T* unwrap(const StorageType& value) { return value.get(); }
    364 };
    365 
    366391
    367392template<typename> class RetainPtr;
    … …  
    371396
    372397    static StorageType wrap(const RetainPtr<T>& value) { return value; }
    373     static bool validate(const StorageType&) { return true; }
    374398    static typename RetainPtr<T>::PtrType unwrap(const StorageType& value) { return value.get(); }
    375399};
    … …  
    426450    virtual typename FunctionWrapper::ResultType operator()()
    427451    {
    428         if (FunctionWrapper::shouldValidateFirstParameter && !ParamStorageTraits<P1>::validate(m_p1))
    429             return typename FunctionWrapper::ResultType();
    430452        return m_functionWrapper(ParamStorageTraits<P1>::unwrap(m_p1));
    431453    }
    … …  
    454476    virtual typename FunctionWrapper::ResultType operator()()
    455477    {
    456         if (FunctionWrapper::shouldValidateFirstParameter && !ParamStorageTraits<P1>::validate(m_p1))
    457             return typename FunctionWrapper::ResultType();
    458478        return m_functionWrapper(ParamStorageTraits<P1>::unwrap(m_p1), ParamStorageTraits<P2>::unwrap(m_p2));
    459479    }
    … …  
    484504    virtual typename FunctionWrapper::ResultType operator()()
    485505    {
    486         if (FunctionWrapper::shouldValidateFirstParameter && !ParamStorageTraits<P1>::validate(m_p1))
    487             return typename FunctionWrapper::ResultType();
    488506        return m_functionWrapper(ParamStorageTraits<P1>::unwrap(m_p1), ParamStorageTraits<P2>::unwrap(m_p2), ParamStorageTraits<P3>::unwrap(m_p3));
    489507    }
    … …  
    516534    virtual typename FunctionWrapper::ResultType operator()()
    517535    {
    518         if (FunctionWrapper::shouldValidateFirstParameter && !ParamStorageTraits<P1>::validate(m_p1))
    519             return typename FunctionWrapper::ResultType();
    520536        return m_functionWrapper(ParamStorageTraits<P1>::unwrap(m_p1), ParamStorageTraits<P2>::unwrap(m_p2), ParamStorageTraits<P3>::unwrap(m_p3), ParamStorageTraits<P4>::unwrap(m_p4));
    521537    }
    … …  
    550566    virtual typename FunctionWrapper::ResultType operator()()
    551567    {
    552         if (FunctionWrapper::shouldValidateFirstParameter && !ParamStorageTraits<P1>::validate(m_p1))
    553             return typename FunctionWrapper::ResultType();
    554568        return m_functionWrapper(ParamStorageTraits<P1>::unwrap(m_p1), ParamStorageTraits<P2>::unwrap(m_p2), ParamStorageTraits<P3>::unwrap(m_p3), ParamStorageTraits<P4>::unwrap(m_p4), ParamStorageTraits<P5>::unwrap(m_p5));
    555569    }
    … …  
    586600    virtual typename FunctionWrapper::ResultType operator()()
    587601    {
    588         if (FunctionWrapper::shouldValidateFirstParameter && !ParamStorageTraits<P1>::validate(m_p1))
    589             return typename FunctionWrapper::ResultType();
    590602        return m_functionWrapper(ParamStorageTraits<P1>::unwrap(m_p1), ParamStorageTraits<P2>::unwrap(m_p2), ParamStorageTraits<P3>::unwrap(m_p3), ParamStorageTraits<P4>::unwrap(m_p4), ParamStorageTraits<P5>::unwrap(m_p5), ParamStorageTraits<P6>::unwrap(m_p6));
    591603    }

  • trunk/Source/WTF/wtf/WeakPtr.h

    r139780 r140589  
    9797    WeakPtr<T> createWeakPtr() { return WeakPtr<T>(m_ref); }
    9898
     99    void revokeAll()
     100    {
     101        T* ptr = m_ref->get();
     102        m_ref->clear();
     103        // We create a new WeakReference so that future calls to createWeakPtr() create nonzero WeakPtrs.
     104        m_ref = Internal::WeakReference<T>::create(ptr);
     105    }
     106
    99107private:
    100108    RefPtr<Internal::WeakReference<T> > m_ref;

  • trunk/Source/WTF/wtf/chromium/MainThreadChromium.cpp

    r111778 r140589  
    3535#include "ChromiumThreading.h"
    3636#include "Threading.h"
     37#include <wtf/Functional.h>
    3738
    3839namespace WTF {
    … …  
    5556}
    5657
     58static void callFunctionObject(void* context)
     59{
     60    Function<void()>* function = static_cast<Function<void()>*>(context);
     61    (*function)();
     62    delete function;
     63}
     64
     65void callOnMainThread(const Function<void()>& function)
     66{
     67    callOnMainThread(callFunctionObject, new Function<void()>(function));
     68}
     69
    5770void callOnMainThreadAndWait(MainThreadFunction*, void*)
    5871{

  • trunk/Source/WebCore/ChangeLog

    r140588 r140589  
     12013-01-23  Adam Barth  <abarth@webkit.org>
     2
     3        BackgroundHTMLParser::sendTokensToMainThread should use bind
     4        https://bugs.webkit.org/show_bug.cgi?id=107637
     5
     6        Reviewed by Eric Seidel.
     7
     8        This patch replaces our hand-written implementation of bind for
     9        didReceiveTokensFromBackgroundParser with bind from Functional.h. To
     10        use the generic version of bind, we need to switch to using WeakPtr to
     11        hold a reference to the main thread parser in the BackgroundHTMLParser.
     12
     13        * html/parser/BackgroundHTMLParser.cpp:
     14        (WebCore::BackgroundHTMLParser::BackgroundHTMLParser):
     15        (WebCore::BackgroundHTMLParser::sendTokensToMainThread):
     16        (WebCore::BackgroundHTMLParser::createPartial):
     17        * html/parser/BackgroundHTMLParser.h:
     18        (WebCore::BackgroundHTMLParser::create):
     19        (BackgroundHTMLParser):
     20        (ParserMap):
     21        * html/parser/HTMLDocumentParser.cpp:
     22        (WebCore::HTMLDocumentParser::HTMLDocumentParser):
     23        (WebCore::HTMLDocumentParser::startBackgroundParser):
     24        (WebCore::HTMLDocumentParser::stopBackgroundParser):
     25        * html/parser/HTMLDocumentParser.h:
     26        (HTMLDocumentParser):
     27
    1282013-01-23  Roger Fong  <roger_fong@apple.com>
    229

  • trunk/Source/WebCore/html/parser/BackgroundHTMLParser.cpp

    r140478 r140589  
    6969}
    7070
    71 typedef const void* ParserIdentifier;
    72 class HTMLDocumentParser;
    73 
    7471ParserMap& parserMap()
    7572{
    … …  
    8683}
    8784
    88 ParserMap::MainThreadParserMap& ParserMap::mainThreadParsers()
    89 {
    90     ASSERT(isMainThread());
    91     return m_mainThreadParsers;
    92 }
    93 
    94 BackgroundHTMLParser::BackgroundHTMLParser(const HTMLParserOptions& options, ParserIdentifier identifier)
     85BackgroundHTMLParser::BackgroundHTMLParser(const HTMLParserOptions& options, WeakPtr<HTMLDocumentParser> parser)
    9586    : m_inForeignContent(false)
    9687    , m_tokenizer(HTMLTokenizer::create(options))
    9788    , m_options(options)
    98     , m_parserIdentifer(identifier)
     89    , m_parser(parser)
    9990    , m_pendingTokens(adoptPtr(new CompactHTMLTokenStream))
    10091{
    … …  
    174165}
    175166
    176 class TokenDelivery {
    177     WTF_MAKE_NONCOPYABLE(TokenDelivery);
    178 public:
    179     TokenDelivery()
    180         : identifier(0)
    181     {
    182     }
    183 
    184     ParserIdentifier identifier;
    185     OwnPtr<CompactHTMLTokenStream> tokens;
    186     static void execute(void* context)
    187     {
    188         TokenDelivery* delivery = static_cast<TokenDelivery*>(context);
    189         HTMLDocumentParser* parser = parserMap().mainThreadParsers().get(delivery->identifier);
    190         if (parser)
    191             parser->didReceiveTokensFromBackgroundParser(delivery->tokens.release());
    192         // FIXME: Ideally we wouldn't need to call delete manually. Instead
    193         // we would like an API where the message queue owns the tasks and
    194         // takes care of deleting them.
    195         delete delivery;
    196     }
    197 };
    198 
    199167void BackgroundHTMLParser::sendTokensToMainThread()
    200168{
    … …  
    206174#endif
    207175
    208     TokenDelivery* delivery = new TokenDelivery;
    209     delivery->identifier = m_parserIdentifer;
    210     delivery->tokens = m_pendingTokens.release();
    211     callOnMainThread(TokenDelivery::execute, delivery);
     176    callOnMainThread(bind(&HTMLDocumentParser::didReceiveTokensFromBackgroundParser, m_parser, m_pendingTokens.release()));
    212177
    213178    m_pendingTokens = adoptPtr(new CompactHTMLTokenStream);
    214179}
    215180
    216 void BackgroundHTMLParser::createPartial(ParserIdentifier identifier, HTMLParserOptions options)
     181void BackgroundHTMLParser::createPartial(ParserIdentifier identifier, HTMLParserOptions options, WeakPtr<HTMLDocumentParser> parser)
    217182{
    218183    ASSERT(!parserMap().backgroundParsers().get(identifier));
    219     parserMap().backgroundParsers().set(identifier, BackgroundHTMLParser::create(options, identifier));
     184    parserMap().backgroundParsers().set(identifier, BackgroundHTMLParser::create(options, parser));
    220185}
    221186

  • trunk/Source/WebCore/html/parser/BackgroundHTMLParser.h

    r140478 r140589  
    3333#include "HTMLToken.h"
    3434#include "HTMLTokenizer.h"
     35#include <wtf/WeakPtr.h>
    3536
    3637namespace WebCore {
    … …  
    4546    void finish();
    4647
    47     static PassOwnPtr<BackgroundHTMLParser> create(const HTMLParserOptions& options, ParserIdentifier identifier)
     48    static PassOwnPtr<BackgroundHTMLParser> create(const HTMLParserOptions& options, WeakPtr<HTMLDocumentParser> parser)
    4849    {
    49         return adoptPtr(new BackgroundHTMLParser(options, identifier));
     50        return adoptPtr(new BackgroundHTMLParser(options, parser));
    5051    }
    5152
    52     static void createPartial(ParserIdentifier, HTMLParserOptions);
     53    static void createPartial(ParserIdentifier, HTMLParserOptions, WeakPtr<HTMLDocumentParser>);
    5354    static void stopPartial(ParserIdentifier);
    5455    static void appendPartial(ParserIdentifier, const String& input);
    … …  
    5657
    5758private:
    58     explicit BackgroundHTMLParser(const HTMLParserOptions&, ParserIdentifier);
     59    BackgroundHTMLParser(const HTMLParserOptions&, WeakPtr<HTMLDocumentParser>);
    5960
    6061    void markEndOfFile();
    … …  
    6970    OwnPtr<HTMLTokenizer> m_tokenizer;
    7071    HTMLParserOptions m_options;
    71     ParserIdentifier m_parserIdentifer;
     72    WeakPtr<HTMLDocumentParser> m_parser;
    7273    OwnPtr<CompactHTMLTokenStream> m_pendingTokens;
    7374};
    … …  
    8182
    8283    typedef HashMap<ParserIdentifier, OwnPtr<BackgroundHTMLParser> > BackgroundParserMap;
    83     typedef HashMap<ParserIdentifier, HTMLDocumentParser*> MainThreadParserMap;
    8484
    8585    BackgroundParserMap& backgroundParsers();
    86     MainThreadParserMap& mainThreadParsers();
    8786
    8887private:
    8988    BackgroundParserMap m_backgroundParsers;
    90     MainThreadParserMap m_mainThreadParsers;
    9189};
    9290

  • trunk/Source/WebCore/html/parser/HTMLDocumentParser.cpp

    r140559 r140589  
    8383    , m_parserScheduler(HTMLParserScheduler::create(this))
    8484    , m_xssAuditor(this)
     85#if ENABLE(THREADED_HTML_PARSER)
     86    , m_weakFactory(this)
     87#endif
    8588    , m_endWasDelayed(false)
    8689    , m_haveBackgroundParser(false)
    … …  
    97100    , m_treeBuilder(HTMLTreeBuilder::create(this, fragment, contextElement, scriptingPermission, m_options))
    98101    , m_xssAuditor(this)
     102#if ENABLE(THREADED_HTML_PARSER)
     103    , m_weakFactory(this)
     104#endif
    99105    , m_endWasDelayed(false)
    100106    , m_haveBackgroundParser(false)
    … …  
    457463
    458464    ParserIdentifier identifier = ParserMap::identifierForParser(this);
    459     parserMap().mainThreadParsers().set(identifier, this);
    460 
    461     HTMLParserThread::shared()->postTask(bind(&BackgroundHTMLParser::createPartial, identifier, m_options));
     465    WeakPtr<HTMLDocumentParser> parser = m_weakFactory.createWeakPtr();
     466    HTMLParserThread::shared()->postTask(bind(&BackgroundHTMLParser::createPartial, identifier, m_options, parser));
    462467}
    463468
    … …  
    470475    ParserIdentifier identifier = ParserMap::identifierForParser(this);
    471476    HTMLParserThread::shared()->postTask(bind(&BackgroundHTMLParser::stopPartial, identifier));
    472 
    473     parserMap().mainThreadParsers().set(identifier, 0);
    474     // We will not recieve any messages from the parser after this point.
     477    m_weakFactory.revokeAll();
    475478}
    476479

  • trunk/Source/WebCore/html/parser/HTMLDocumentParser.h

    r140478 r140589  
    4040#include <wtf/Deque.h>
    4141#include <wtf/OwnPtr.h>
     42#include <wtf/WeakPtr.h>
    4243#include <wtf/text/TextPosition.h>
    4344
    … …  
    172173#if ENABLE(THREADED_HTML_PARSER)
    173174    Deque<OwnPtr<CompactHTMLTokenStream> > m_pendingTokens;
     175    WeakPtrFactory<HTMLDocumentParser> m_weakFactory;
    174176#endif
    175177
Note: See TracChangeset for help on using the changeset viewer.