Attachment 341225 Details for Bug 185961 – Patch

[patch] Patch

bug-185961-20180524144106.patch (text/plain), 8.02 KB, created by Chris Dumez on 2018-05-24 14:41:06 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2018-05-24 14:41:06 PDT

Size: 8.02 KB

patch

obsolete

>Subversion Revision: 232157
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index a2b43d66ac8d88b7c8ae63784ce27470b2933180..cdd20d746437d45aa5a4490ee0b62cc435df45ce 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,35 @@
>+2018-05-24  Chris Dumez  <cdumez@apple.com>
>+
>+        Avoid constructing the string "all" repeatedly in MediaQueryParser
>+        https://bugs.webkit.org/show_bug.cgi?id=185961
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Avoid constructing the string "all" repeatedly in MediaQueryParser by:
>+        - Having MediaQueryData use an std::optional<String> instead of a String
>+        - Updating MediaQueryParser to use a static to store the default media type
>+          and use it when the MediaQueryData's media type is std::nullopt.
>+
>+        Also do some cleanup.
>+
>+        * css/parser/MediaQueryParser.cpp:
>+        (WebCore::MediaQueryParser::commitMediaQuery):
>+        (WebCore::MediaQueryParser::MediaQueryData::MediaQueryData):
>+        (WebCore::MediaQueryParser::MediaQueryData::clear):
>+        (WebCore::MediaQueryParser::MediaQueryData::addExpression):
>+        (WebCore::MediaQueryParser::MediaQueryData::lastExpressionValid):
>+        (WebCore::MediaQueryParser::MediaQueryData::removeLastExpression):
>+        * css/parser/MediaQueryParser.h:
>+        (WebCore::MediaQueryParser::MediaQueryData::setMediaType):
>+        (WebCore::MediaQueryParser::MediaQueryData::restrictor const):
>+        (WebCore::MediaQueryParser::MediaQueryData::expressions):
>+        (WebCore::MediaQueryParser::MediaQueryData::mediaType const):
>+        (WebCore::MediaQueryParser::MediaQueryData::currentMediaQueryChanged const):
>+        (WebCore::MediaQueryParser::MediaQueryData::restrictor):
>+        (WebCore::MediaQueryParser::MediaQueryData::setRestrictor):
>+        (WebCore::MediaQueryParser::MediaQueryData::setMediaFeature):
>+        (WebCore::MediaQueryParser::MediaQueryData::setMediaQueryParserContext):
>+
> 2018-05-24  Chris Dumez  <cdumez@apple.com>
> 
>         [iOS WK2] Layout Test imported/w3c/web-platform-tests/service-workers/service-worker/update-after-navigation-fetch-event.https.html is a flaky failure
>diff --git a/Source/WebCore/css/parser/MediaQueryParser.cpp b/Source/WebCore/css/parser/MediaQueryParser.cpp
>index 1f140a6002a404328d4ceb849fbe984dd6ab01b9..778a9be661a679415e550f102f9deac750e595ac 100644
>--- a/Source/WebCore/css/parser/MediaQueryParser.cpp
>+++ b/Source/WebCore/css/parser/MediaQueryParser.cpp
>@@ -140,7 +140,8 @@ void MediaQueryParser::commitMediaQuery()
> {
>     // FIXME-NEWPARSER: Convoluted and awful, but we can't change the MediaQuerySet yet because of the
>     // old parser.
>-    MediaQuery mediaQuery = MediaQuery(m_mediaQueryData.restrictor(), m_mediaQueryData.mediaType(), WTFMove(m_mediaQueryData.expressions()));
>+    static const NeverDestroyed<String> defaultMediaType = ASCIILiteral { "all" };
>+    MediaQuery mediaQuery { m_mediaQueryData.restrictor(), m_mediaQueryData.mediaType().value_or(defaultMediaType), WTFMove(m_mediaQueryData.expressions()) };
>     m_mediaQueryData.clear();
>     m_querySet->addMediaQuery(WTFMove(mediaQuery));
> }
>@@ -274,43 +275,32 @@ RefPtr<MediaQuerySet> MediaQueryParser::parseInternal(CSSParserTokenRange range)
>     return m_querySet;
> }
> 
>-MediaQueryData::MediaQueryData(MediaQueryParserContext context)
>-    : m_restrictor(MediaQuery::None)
>-    , m_mediaType("all")
>-    , m_mediaTypeSet(false)
>-    , m_context(context)
>+MediaQueryParser::MediaQueryData::MediaQueryData(MediaQueryParserContext context)
>+    : m_context(context)
> {
> }
> 
>-void MediaQueryData::clear()
>+void MediaQueryParser::MediaQueryData::clear()
> {
>     m_restrictor = MediaQuery::None;
>-    m_mediaType = "all";
>-    m_mediaTypeSet = false;
>+    m_mediaType = std::nullopt;
>     m_mediaFeature = String();
>     m_expressions.clear();
> }
> 
>-void MediaQueryData::addExpression(CSSParserTokenRange& range)
>+void MediaQueryParser::MediaQueryData::addExpression(CSSParserTokenRange& range)
> {
>-    MediaQueryExpression expression = MediaQueryExpression(m_mediaFeature, range, m_context);
>-    m_expressions.append(WTFMove(expression));
>+    m_expressions.append(MediaQueryExpression { m_mediaFeature, range, m_context });
> }
> 
>-bool MediaQueryData::lastExpressionValid()
>+bool MediaQueryParser::MediaQueryData::lastExpressionValid()
> {
>     return m_expressions.last().isValid();
> }
> 
>-void MediaQueryData::removeLastExpression()
>+void MediaQueryParser::MediaQueryData::removeLastExpression()
> {
>     m_expressions.removeLast();
> }
> 
>-void MediaQueryData::setMediaType(const String& mediaType)
>-{
>-    m_mediaType = mediaType;
>-    m_mediaTypeSet = true;
>-}
>-
> } // namespace WebCore
>diff --git a/Source/WebCore/css/parser/MediaQueryParser.h b/Source/WebCore/css/parser/MediaQueryParser.h
>index 18369e9dcf9d10478d439d5b4641fc8324ea5794..90f0439b72f641415aff8e00f3586ca1e9182c88 100644
>--- a/Source/WebCore/css/parser/MediaQueryParser.h
>+++ b/Source/WebCore/css/parser/MediaQueryParser.h
>@@ -35,6 +35,7 @@
> #include "MediaQueryBlockWatcher.h"
> #include "MediaQueryExpression.h"
> #include "MediaQueryParserContext.h"
>+#include <wtf/Optional.h>
> #include <wtf/text/WTFString.h>
> 
> namespace WebCore {
>@@ -42,41 +43,6 @@ namespace WebCore {
> class MediaQuerySet;
> struct CSSParserContext;
> 
>-class MediaQueryData {
>-    WTF_MAKE_NONCOPYABLE(MediaQueryData);
>-public:
>-    MediaQueryData(MediaQueryParserContext context);
>-    void clear();
>-    void addExpression(CSSParserTokenRange&);
>-    bool lastExpressionValid();
>-    void removeLastExpression();
>-    void setMediaType(const String&);
>-    
>-    MediaQuery::Restrictor restrictor() const { return m_restrictor; };
>-    Vector<MediaQueryExpression>& expressions() { return m_expressions; }
>-    String mediaType() const { return m_mediaType; }
>-    
>-    inline bool currentMediaQueryChanged() const
>-    {
>-        return (m_restrictor != MediaQuery::None || m_mediaTypeSet || m_expressions.size() > 0);
>-    }
>-    inline MediaQuery::Restrictor restrictor() { return m_restrictor; }
>-
>-    inline void setRestrictor(MediaQuery::Restrictor restrictor) { m_restrictor = restrictor; }
>-
>-    inline void setMediaFeature(const String& str) { m_mediaFeature = str; }
>-    
>-    inline void setMediaQueryParserContext(MediaQueryParserContext context) { m_context = context; }
>-
>-private:
>-    MediaQuery::Restrictor m_restrictor;
>-    String m_mediaType;
>-    Vector<MediaQueryExpression> m_expressions;
>-    String m_mediaFeature;
>-    bool m_mediaTypeSet;
>-    MediaQueryParserContext m_context;
>-};
>-
> class MediaQueryParser {
>     WTF_MAKE_NONCOPYABLE(MediaQueryParser);
> public:
>@@ -117,6 +83,40 @@ private:
>     
>     void commitMediaQuery();
> 
>+    class MediaQueryData {
>+        WTF_MAKE_NONCOPYABLE(MediaQueryData);
>+    public:
>+        explicit MediaQueryData(MediaQueryParserContext);
>+        void clear();
>+        void addExpression(CSSParserTokenRange&);
>+        bool lastExpressionValid();
>+        void removeLastExpression();
>+        void setMediaType(const String& mediaType) { m_mediaType = mediaType; }
>+
>+        MediaQuery::Restrictor restrictor() const { return m_restrictor; }
>+        Vector<MediaQueryExpression>& expressions() { return m_expressions; }
>+        const std::optional<String>& mediaType() const { return m_mediaType; }
>+
>+        bool currentMediaQueryChanged() const
>+        {
>+            return (m_restrictor != MediaQuery::None || m_mediaType || !m_expressions.isEmpty());
>+        }
>+        MediaQuery::Restrictor restrictor() { return m_restrictor; }
>+
>+        void setRestrictor(MediaQuery::Restrictor restrictor) { m_restrictor = restrictor; }
>+
>+        void setMediaFeature(const String& str) { m_mediaFeature = str; }
>+
>+        void setMediaQueryParserContext(MediaQueryParserContext context) { m_context = context; }
>+
>+    private:
>+        MediaQuery::Restrictor m_restrictor { MediaQuery::None };
>+        std::optional<String> m_mediaType;
>+        Vector<MediaQueryExpression> m_expressions;
>+        String m_mediaFeature;
>+        MediaQueryParserContext m_context;
>+    };
>+
>     State m_state;
>     ParserType m_parserType;
>     MediaQueryData m_mediaQueryData;

Actions: View | Formatted Diff | Diff

Attachments on bug 185961: 341225