6% regression in intl1 page cycler on chromium-mac
authorabarth@webkit.org <abarth@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 25 Feb 2013 20:58:27 +0000 (20:58 +0000)
committerabarth@webkit.org <abarth@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 25 Feb 2013 20:58:27 +0000 (20:58 +0000)
https://bugs.webkit.org/show_bug.cgi?id=110784

Reviewed by Eric Seidel.

This patch attempts to heal the regression by reverting all the changes
to the preload scanner up to (and including)
http://trac.webkit.org/changeset/143020/.

* html/parser/BackgroundHTMLParser.cpp:
(WebCore::BackgroundHTMLParser::resumeFrom):
(WebCore::BackgroundHTMLParser::pumpTokenizer):
(WebCore::BackgroundHTMLParser::sendTokensToMainThread):
* html/parser/CSSPreloadScanner.cpp:
(WebCore::CSSPreloadScanner::scan):
(WebCore::CSSPreloadScanner::emitRule):
* html/parser/CSSPreloadScanner.h:
(CSSPreloadScanner):
* html/parser/HTMLPreloadScanner.cpp:
(WebCore::isStartTag):
(WebCore):
(WebCore::isStartOrEndTag):
(WebCore::TokenPreloadScanner::identifierFor):
(WebCore::TokenPreloadScanner::inititatorFor):
(WebCore::TokenPreloadScanner::StartTagScanner::processAttributes):
(WebCore::TokenPreloadScanner::StartTagScanner::createPreloadRequest):
(WebCore::TokenPreloadScanner::processPossibleTemplateTag):
(WebCore::TokenPreloadScanner::processPossibleStyleTag):
(WebCore::TokenPreloadScanner::processPossibleBaseTag):
(WebCore::TokenPreloadScanner::scan):
(WebCore::HTMLPreloadScanner::scan):
* html/parser/HTMLPreloadScanner.h:
(TokenPreloadScanner):

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

Source/WebCore/ChangeLog
Source/WebCore/html/parser/BackgroundHTMLParser.cpp
Source/WebCore/html/parser/CSSPreloadScanner.cpp
Source/WebCore/html/parser/CSSPreloadScanner.h
Source/WebCore/html/parser/HTMLPreloadScanner.cpp
Source/WebCore/html/parser/HTMLPreloadScanner.h

index 8c30d41..879a70f 100644 (file)
@@ -1,3 +1,39 @@
+2013-02-25  Adam Barth  <abarth@webkit.org>
+
+        6% regression in intl1 page cycler on chromium-mac
+        https://bugs.webkit.org/show_bug.cgi?id=110784
+
+        Reviewed by Eric Seidel.
+
+        This patch attempts to heal the regression by reverting all the changes
+        to the preload scanner up to (and including)
+        http://trac.webkit.org/changeset/143020/.
+
+        * html/parser/BackgroundHTMLParser.cpp:
+        (WebCore::BackgroundHTMLParser::resumeFrom):
+        (WebCore::BackgroundHTMLParser::pumpTokenizer):
+        (WebCore::BackgroundHTMLParser::sendTokensToMainThread):
+        * html/parser/CSSPreloadScanner.cpp:
+        (WebCore::CSSPreloadScanner::scan):
+        (WebCore::CSSPreloadScanner::emitRule):
+        * html/parser/CSSPreloadScanner.h:
+        (CSSPreloadScanner):
+        * html/parser/HTMLPreloadScanner.cpp:
+        (WebCore::isStartTag):
+        (WebCore):
+        (WebCore::isStartOrEndTag):
+        (WebCore::TokenPreloadScanner::identifierFor):
+        (WebCore::TokenPreloadScanner::inititatorFor):
+        (WebCore::TokenPreloadScanner::StartTagScanner::processAttributes):
+        (WebCore::TokenPreloadScanner::StartTagScanner::createPreloadRequest):
+        (WebCore::TokenPreloadScanner::processPossibleTemplateTag):
+        (WebCore::TokenPreloadScanner::processPossibleStyleTag):
+        (WebCore::TokenPreloadScanner::processPossibleBaseTag):
+        (WebCore::TokenPreloadScanner::scan):
+        (WebCore::HTMLPreloadScanner::scan):
+        * html/parser/HTMLPreloadScanner.h:
+        (TokenPreloadScanner):
+
 2013-02-25  Mark Lam  <mark.lam@apple.com>
 
         Changed DatabaseTracker::getMaxSizeForDatabase() to return the previous
index 56e47fc..b5ed15c 100644 (file)
@@ -156,7 +156,6 @@ void BackgroundHTMLParser::resumeFrom(PassOwnPtr<Checkpoint> checkpoint)
     m_token = checkpoint->token.release();
     m_tokenizer = checkpoint->tokenizer.release();
     m_input.rewindTo(checkpoint->inputCheckpoint, checkpoint->unparsedInput);
-    m_preloadScanner->rewindTo(checkpoint->preloadScannerCheckpoint);
     pumpTokenizer();
 }
 
@@ -252,8 +251,6 @@ void BackgroundHTMLParser::pumpTokenizer()
             if (xssInfo)
                 token.setXSSInfo(xssInfo.release());
 
-            m_preloadScanner->scan(token, m_pendingPreloads);
-
             m_pendingTokens->append(token);
         }
 
@@ -280,7 +277,6 @@ void BackgroundHTMLParser::sendTokensToMainThread()
     chunk->tokens = m_pendingTokens.release();
     chunk->preloads.swap(m_pendingPreloads);
     chunk->inputCheckpoint = m_input.createCheckpoint();
-    chunk->preloadScannerCheckpoint = m_preloadScanner->createCheckpoint();
     callOnMainThread(bind(&HTMLDocumentParser::didReceiveParsedChunkFromBackgroundParser, m_parser, chunk.release()));
 
     m_pendingTokens = adoptPtr(new CompactHTMLTokenStream);
index 0c66fc1..9577079 100644 (file)
@@ -50,29 +50,20 @@ void CSSPreloadScanner::reset()
     m_ruleValue.clear();
 }
 
-template<typename Char>
-void CSSPreloadScanner::scanCommon(const Char* begin, const Char* end, PreloadRequestStream& requests)
+void CSSPreloadScanner::scan(const UChar* begin, const UChar* end, Vector<OwnPtr<PreloadRequest> >& requests)
 {
     m_requests = &requests;
-    for (const Char* it = begin; it != end && m_state != DoneParsingImportRules; ++it)
+    for (const UChar* it = begin; it != end && m_state != DoneParsingImportRules; ++it)
         tokenize(*it);
     m_requests = 0;
 }
 
-void CSSPreloadScanner::scan(const HTMLToken::DataVector& data, PreloadRequestStream& requests)
+void CSSPreloadScanner::scan(const LChar* begin, const LChar* end, Vector<OwnPtr<PreloadRequest> >& requests)
 {
-    scanCommon(data.data(), data.data() + data.size(), requests);
-}
-
-void CSSPreloadScanner::scan(const String& data, PreloadRequestStream& requests)
-{
-    if (data.is8Bit()) {
-        const LChar* begin = data.characters8();
-        scanCommon(begin, begin + data.length(), requests);
-        return;
-    }
-    const UChar* begin = data.characters16();
-    scanCommon(begin, begin + data.length(), requests);
+    m_requests = &requests;
+    for (const LChar* it = begin; it != end && m_state != DoneParsingImportRules; ++it)
+        tokenize(*it);
+    m_requests = 0;
 }
 
 inline void CSSPreloadScanner::tokenize(UChar c)
@@ -213,7 +204,8 @@ void CSSPreloadScanner::emitRule()
         String url = parseCSSStringOrURL(m_ruleValue.characters(), m_ruleValue.length());
         if (!url.isEmpty()) {
             KURL baseElementURL; // FIXME: This should be passed in from the HTMLPreloadScaner via scan()!
-            OwnPtr<PreloadRequest> request = PreloadRequest::create("css", url, baseElementURL, CachedResource::CSSStyleSheet);
+            OwnPtr<PreloadRequest> request = PreloadRequest::create(
+                cachedResourceRequestInitiators().css, url, baseElementURL, CachedResource::CSSStyleSheet);
             // FIXME: Should this be including the charset in the preload request?
             m_requests->append(request.release());
         }
index 4b69bc3..22f6fa8 100644 (file)
@@ -28,7 +28,6 @@
 #define CSSPreloadScanner_h
 
 #include "HTMLResourcePreloader.h"
-#include "HTMLToken.h"
 #include <wtf/text/StringBuilder.h>
 
 namespace WebCore {
@@ -41,8 +40,8 @@ public:
 
     void reset();
 
-    void scan(const HTMLToken::DataVector&, PreloadRequestStream&);
-    void scan(const String&, PreloadRequestStream&);
+    void scan(const UChar* begin, const UChar* end, Vector<OwnPtr<PreloadRequest> >&);
+    void scan(const LChar* begin, const LChar* end, Vector<OwnPtr<PreloadRequest> >&);
 
 private:
     enum State {
@@ -58,9 +57,6 @@ private:
         DoneParsingImportRules,
     };
 
-    template<typename Char>
-    void scanCommon(const Char* begin, const Char* end, PreloadRequestStream&);
-
     inline void tokenize(UChar);
     void emitRule();
 
@@ -69,7 +65,7 @@ private:
     StringBuilder m_ruleValue;
 
     // Only non-zero during scan()
-    PreloadRequestStream* m_requests;
+    Vector<OwnPtr<PreloadRequest> >* m_requests;
 };
 
 }
index a57c5d3..848564d 100644 (file)
@@ -43,9 +43,18 @@ namespace WebCore {
 
 using namespace HTMLNames;
 
-TokenPreloadScanner::TagId TokenPreloadScanner::tagIdFor(const HTMLToken::DataVector& data)
+static bool isStartTag(HTMLToken::Type type)
+{
+    return type == HTMLToken::StartTag;
+}
+
+static bool isStartOrEndTag(HTMLToken::Type type)
+{
+    return type == HTMLToken::EndTag || isStartTag(type);
+}
+
+TokenPreloadScanner::TagId TokenPreloadScanner::identifierFor(const AtomicString& tagName)
 {
-    AtomicString tagName(data);
     if (tagName == imgTag)
         return ImgTagId;
     if (tagName == inputTag)
@@ -63,26 +72,7 @@ TokenPreloadScanner::TagId TokenPreloadScanner::tagIdFor(const HTMLToken::DataVe
     return UnknownTagId;
 }
 
-TokenPreloadScanner::TagId TokenPreloadScanner::tagIdFor(const String& tagName)
-{
-    if (threadSafeMatch(tagName, imgTag))
-        return ImgTagId;
-    if (threadSafeMatch(tagName, inputTag))
-        return InputTagId;
-    if (threadSafeMatch(tagName, linkTag))
-        return LinkTagId;
-    if (threadSafeMatch(tagName, scriptTag))
-        return ScriptTagId;
-    if (threadSafeMatch(tagName, styleTag))
-        return StyleTagId;
-    if (threadSafeMatch(tagName, baseTag))
-        return BaseTagId;
-    if (threadSafeMatch(tagName, templateTag))
-        return TemplateTagId;
-    return UnknownTagId;
-}
-
-String TokenPreloadScanner::initiatorFor(TagId tagId)
+String TokenPreloadScanner::inititatorFor(TagId tagId)
 {
     switch (tagId) {
     case ImgTagId:
@@ -117,8 +107,10 @@ public:
     void processAttributes(const HTMLToken::AttributeList& attributes)
     {
         ASSERT(isMainThread());
+
         if (m_tagId >= UnknownTagId)
             return;
+
         for (HTMLToken::AttributeList::const_iterator iter = attributes.begin(); iter != attributes.end(); ++iter) {
             AtomicString attributeName(iter->name);
             String attributeValue = StringImpl::create8BitIfPossible(iter->value);
@@ -126,22 +118,12 @@ public:
         }
     }
 
-#if ENABLE(THREADED_HTML_PARSER)
-    void processAttributes(const Vector<CompactHTMLToken::Attribute>& attributes)
-    {
-        if (m_tagId >= UnknownTagId)
-            return;
-        for (Vector<CompactHTMLToken::Attribute>::const_iterator iter = attributes.begin(); iter != attributes.end(); ++iter)
-            processAttribute(iter->name, iter->value);
-    }
-#endif
-
     PassOwnPtr<PreloadRequest> createPreloadRequest(const KURL& predictedBaseURL)
     {
         if (!shouldPreload())
             return nullptr;
 
-        OwnPtr<PreloadRequest> request = PreloadRequest::create(initiatorFor(m_tagId), m_urlToLoad, predictedBaseURL, resourceType());
+        OwnPtr<PreloadRequest> request = PreloadRequest::create(inititatorFor(m_tagId), m_urlToLoad, predictedBaseURL, resourceType());
         request->setCrossOriginModeAllowsCookies(crossOriginModeAllowsCookies());
         request->setCharset(charset());
         return request.release();
@@ -262,111 +244,88 @@ TokenPreloadScanner::~TokenPreloadScanner()
 {
 }
 
-TokenPreloadScannerCheckpoint TokenPreloadScanner::createCheckpoint()
-{
-    TokenPreloadScannerCheckpoint checkpoint = m_checkpoints.size();
-    m_checkpoints.append(Checkpoint(m_predictedBaseElementURL, m_inStyle
 #if ENABLE(TEMPLATE_ELEMENT)
-                                    , m_templateCount
-#endif
-                                    ));
-    return checkpoint;
-}
-
-void TokenPreloadScanner::rewindTo(TokenPreloadScannerCheckpoint checkpointIndex)
+bool TokenPreloadScanner::processPossibleTemplateTag(TagId tagId, HTMLToken::Type type)
 {
-    ASSERT(checkpointIndex < m_checkpoints.size()); // If this ASSERT fires, checkpointIndex is invalid.
-    const Checkpoint& checkpoint = m_checkpoints[checkpointIndex];
-    m_predictedBaseElementURL = checkpoint.predictedBaseElementURL;
-    m_inStyle = checkpoint.inStyle;
-#if ENABLE(TEMPLATE_ELEMENT)
-    m_templateCount = checkpoint.templateCount;
-#endif
-    m_cssScanner.reset();
-    m_checkpoints.clear();
+    if (isStartOrEndTag(type) && tagId == TemplateTagId) {
+        if (isStartTag(type))
+            m_templateCount++;
+        else
+            m_templateCount--;
+        return true; // Twas our token.
+    }
+    // If we're in a template we "consume" all tokens.
+    return m_templateCount > 0;
 }
+#endif
 
-void TokenPreloadScanner::scan(const HTMLToken& token, Vector<OwnPtr<PreloadRequest> >& requests)
+bool TokenPreloadScanner::processPossibleStyleTag(TagId tagId, HTMLToken::Type type)
 {
-    scanCommon(token, requests);
+    ASSERT(isStartOrEndTag(type));
+    if (tagId != StyleTagId)
+        return false;
+
+    m_inStyle = isStartTag(type);
+
+    if (!m_inStyle)
+        m_cssScanner.reset();
+
+    return true;
 }
 
-#if ENABLE(THREADED_HTML_PARSER)
-void TokenPreloadScanner::scan(const CompactHTMLToken& token, Vector<OwnPtr<PreloadRequest> >& requests)
+bool TokenPreloadScanner::processPossibleBaseTag(TagId tagId, const HTMLToken& token)
 {
-    scanCommon(token, requests);
+    ASSERT(isStartTag(token.type()));
+    if (tagId != BaseTagId)
+        return false;
+
+    // The first <base> element is the one that wins.
+    if (!m_predictedBaseElementURL.isEmpty())
+        return true;
+
+    for (HTMLToken::AttributeList::const_iterator iter = token.attributes().begin(); iter != token.attributes().end(); ++iter) {
+        AtomicString attributeName(iter->name);
+        if (attributeName == hrefAttr) {
+            String hrefValue = StringImpl::create8BitIfPossible(iter->value);
+            m_predictedBaseElementURL = KURL(m_documentURL, stripLeadingAndTrailingHTMLSpaces(hrefValue));
+            break;
+        }
+    }
+    return true;
 }
-#endif
 
-template<typename Token>
-void TokenPreloadScanner::scanCommon(const Token& token, Vector<OwnPtr<PreloadRequest> >& requests)
+void TokenPreloadScanner::scan(const HTMLToken& token, Vector<OwnPtr<PreloadRequest> >& requests)
 {
-    switch (token.type()) {
-    case HTMLToken::Character: {
-        if (!m_inStyle)
+    // <style> is the only place we search for urls in non-start/end-tag tokens.
+    if (m_inStyle) {
+        if (token.type() != HTMLToken::Character)
             return;
-        m_cssScanner.scan(token.data(), requests);
-        return;
+        const HTMLToken::DataVector& characters = token.characters();
+        return m_cssScanner.scan(characters.begin(), characters.end(), requests);
     }
-    case HTMLToken::EndTag: {
-        TagId tagId = tagIdFor(token.data());
-#if ENABLE(TEMPLATE_ELEMENT)
-        if (tagId == TemplateTagId) {
-            if (m_templateCount)
-                --m_templateCount;
-            return;
-        }
-#endif
-        if (tagId == StyleTagId) {
-            if (m_inStyle)
-                m_cssScanner.reset();
-            m_inStyle = false;
-        }
+
+    if (!isStartOrEndTag(token.type()))
         return;
-    }
-    case HTMLToken::StartTag: {
-#if ENABLE(TEMPLATE_ELEMENT)
-        if (m_templateCount)
-            return;
-#endif
-        TagId tagId = tagIdFor(token.data());
+
+    AtomicString tagName(token.name());
+    TagId tagId = identifierFor(tagName);
+
 #if ENABLE(TEMPLATE_ELEMENT)
-        if (tagId == TemplateTagId) {
-            ++m_templateCount;
-            return;
-        }
+    if (processPossibleTemplateTag(tagId, token.type()))
+        return;
 #endif
-        if (tagId == StyleTagId) {
-            m_inStyle = true;
-            return;
-        }
-        if (tagId == BaseTagId) {
-            // The first <base> element is the one that wins.
-            if (!m_predictedBaseElementURL.isEmpty())
-                return;
-            updatePredictedBaseURL(token);
-            return;
-        }
-
-        StartTagScanner scanner(tagId);
-        scanner.processAttributes(token.attributes());
-        OwnPtr<PreloadRequest> request = scanner.createPreloadRequest(m_predictedBaseElementURL);
-        if (request)
-            requests.append(request.release());
+    if (processPossibleStyleTag(tagId, token.type()))
         return;
-    }
-    default: {
+    if (!isStartTag(token.type()))
+        return;
+    if (processPossibleBaseTag(tagId, token))
         return;
-    }
-    }
-}
 
-template<typename Token>
-void TokenPreloadScanner::updatePredictedBaseURL(const Token& token)
-{
-    ASSERT(m_predictedBaseElementURL.isEmpty());
-    if (const typename Token::Attribute* hrefAttribute = token.getAttributeItem(hrefAttr))
-        m_predictedBaseElementURL = KURL(m_documentURL, stripLeadingAndTrailingHTMLSpaces(hrefAttribute->value)).copy();
+    StartTagScanner scanner(tagId);
+    scanner.processAttributes(token.attributes());
+    OwnPtr<PreloadRequest> request =  scanner.createPreloadRequest(m_predictedBaseElementURL);
+    if (request)
+        requests.append(request.release());
 }
 
 HTMLPreloadScanner::HTMLPreloadScanner(const HTMLParserOptions& options, const KURL& documentURL)
@@ -392,16 +351,15 @@ void HTMLPreloadScanner::scan(HTMLResourcePreloader* preloader, const KURL& star
     if (!startingBaseElementURL.isEmpty())
         m_scanner.setPredictedBaseElementURL(startingBaseElementURL);
 
-    PreloadRequestStream requests;
-
+    Vector<OwnPtr<PreloadRequest> > requests;
     while (m_tokenizer->nextToken(m_source, m_token)) {
-        if (m_token.type() == HTMLToken::StartTag)
+        if (isStartTag(m_token.type()))
             m_tokenizer->updateStateFor(AtomicString(m_token.name()));
         m_scanner.scan(m_token, requests);
         m_token.clear();
     }
-
-    preloader->takeAndPreload(requests);
+    for (size_t i = 0; i < requests.size(); i++)
+        preloader->preload(requests[i].release());
 }
 
 }
index 47a5444..df6470a 100644 (file)
 #define HTMLPreloadScanner_h
 
 #include "CSSPreloadScanner.h"
-#include "CompactHTMLToken.h"
 #include "HTMLToken.h"
 #include "SegmentedString.h"
-#include <wtf/Vector.h>
 
 namespace WebCore {
 
@@ -47,24 +45,10 @@ public:
     explicit TokenPreloadScanner(const KURL& documentURL);
     ~TokenPreloadScanner();
 
-    void scan(const HTMLToken&, PreloadRequestStream& requests);
-#if ENABLE(THREADED_HTML_PARSER)
-    void scan(const CompactHTMLToken&, PreloadRequestStream& requests);
-#endif
+    void scan(const HTMLToken&, Vector<OwnPtr<PreloadRequest> >& requests);
 
     void setPredictedBaseElementURL(const KURL& url) { m_predictedBaseElementURL = url; }
 
-    // A TokenPreloadScannerCheckpoint is valid until the next call to rewindTo,
-    // at which point all outstanding checkpoints are invalidated.
-    TokenPreloadScannerCheckpoint createCheckpoint();
-    void rewindTo(TokenPreloadScannerCheckpoint);
-
-    bool isSafeToSendToAnotherThread()
-    {
-        return m_documentURL.isSafeToSendToAnotherThread()
-            && m_predictedBaseElementURL.isSafeToSendToAnotherThread();
-    }
-
 private:
     enum TagId {
         // These tags are scanned by the StartTagScanner.
@@ -82,48 +66,24 @@ private:
 
     class StartTagScanner;
 
-    template<typename Token>
-    inline void scanCommon(const Token&, PreloadRequestStream& requests);
-
-    static TagId tagIdFor(const HTMLToken::DataVector&);
-    static TagId tagIdFor(const String&);
-
-    static String initiatorFor(TagId);
-
-    template<typename Token>
-    void updatePredictedBaseURL(const Token&);
+    static TagId identifierFor(const AtomicString& tagName);
+    static String inititatorFor(TagId);
 
-    struct Checkpoint {
-        Checkpoint(const KURL& predictedBaseElementURL, bool inStyle
 #if ENABLE(TEMPLATE_ELEMENT)
-            , size_t templateCount
+    bool processPossibleTemplateTag(TagId, HTMLToken::Type);
 #endif
-            )
-            : predictedBaseElementURL(predictedBaseElementURL)
-            , inStyle(inStyle)
-#if ENABLE(TEMPLATE_ELEMENT)
-            , templateCount(templateCount)
-#endif
-        {
-        }
 
-        KURL predictedBaseElementURL;
-        bool inStyle;
-#if ENABLE(TEMPLATE_ELEMENT)
-        size_t templateCount;
-#endif
-    };
+    bool processPossibleStyleTag(TagId, HTMLToken::Type);
+    bool processPossibleBaseTag(TagId, const HTMLToken&);
 
     CSSPreloadScanner m_cssScanner;
-    const KURL m_documentURL;
+    KURL m_documentURL;
     KURL m_predictedBaseElementURL;
     bool m_inStyle;
 
 #if ENABLE(TEMPLATE_ELEMENT)
     size_t m_templateCount;
 #endif
-
-    Vector<Checkpoint> m_checkpoints;
 };
 
 class HTMLPreloadScanner {