Commit 94d7db17 authored by yoav@yoav.ws's avatar yoav@yoav.ws

Fix MQEvaluator::mediaType() related perf regressions.

When we stopped storing the mediaType as a string on m_medium on StyleResolver, 
we started calling FrameView::mediaType() every time that the mediaType as required.
In the related bug's test case, mediaType() was being called a lot, 
in order to compare it to "print" to see which default style sheet should be used.
Since mediaType() calls also call InspectorInstrumentation::applyEmulatedMedia(), 
they have a certain overhead, that impacted the benchmark's results.

I've changed it so that the mediaType() call and the comparison to "print"
would be done once, and the result cached.

BUG=394275

Review URL: https://codereview.chromium.org/405953002

git-svn-id: svn://svn.chromium.org/blink/trunk@179991 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent 2ea1103e
...@@ -111,15 +111,6 @@ bool MediaQueryEvaluator::mediaTypeMatch(const String& mediaTypeToMatch) const ...@@ -111,15 +111,6 @@ bool MediaQueryEvaluator::mediaTypeMatch(const String& mediaTypeToMatch) const
|| equalIgnoringCase(mediaTypeToMatch, mediaType()); || equalIgnoringCase(mediaTypeToMatch, mediaType());
} }
bool MediaQueryEvaluator::mediaTypeMatchSpecific(const char* mediaTypeToMatch) const
{
// Like mediaTypeMatch, but without the special cases for "" and "all".
ASSERT(mediaTypeToMatch);
ASSERT(mediaTypeToMatch[0] != '\0');
ASSERT(!equalIgnoringCase(mediaTypeToMatch, MediaTypeNames::all));
return equalIgnoringCase(mediaTypeToMatch, mediaType());
}
static bool applyRestrictor(MediaQuery::Restrictor r, bool value) static bool applyRestrictor(MediaQuery::Restrictor r, bool value)
{ {
return r == MediaQuery::Not ? !value : value; return r == MediaQuery::Not ? !value : value;
......
...@@ -68,15 +68,14 @@ public: ...@@ -68,15 +68,14 @@ public:
MediaQueryEvaluator(const char* acceptedMediaType, bool mediaFeatureResult = false); MediaQueryEvaluator(const char* acceptedMediaType, bool mediaFeatureResult = false);
// Creates evaluator which evaluates full media queries // Creates evaluator which evaluates full media queries
MediaQueryEvaluator(LocalFrame*); explicit MediaQueryEvaluator(LocalFrame*);
// Creates evaluator which evaluates in a thread-safe manner a subset of media values // Creates evaluator which evaluates in a thread-safe manner a subset of media values
MediaQueryEvaluator(const MediaValues&); explicit MediaQueryEvaluator(const MediaValues&);
~MediaQueryEvaluator(); ~MediaQueryEvaluator();
bool mediaTypeMatch(const String& mediaTypeToMatch) const; bool mediaTypeMatch(const String& mediaTypeToMatch) const;
bool mediaTypeMatchSpecific(const char* mediaTypeToMatch) const;
// Evaluates a list of media queries // Evaluates a list of media queries
bool eval(const MediaQuerySet*, MediaQueryResultList* = 0) const; bool eval(const MediaQuerySet*, MediaQueryResultList* = 0) const;
......
...@@ -31,6 +31,7 @@ ...@@ -31,6 +31,7 @@
#include "core/CSSPropertyNames.h" #include "core/CSSPropertyNames.h"
#include "core/HTMLNames.h" #include "core/HTMLNames.h"
#include "core/MediaTypeNames.h"
#include "core/StylePropertyShorthand.h" #include "core/StylePropertyShorthand.h"
#include "core/animation/ActiveAnimations.h" #include "core/animation/ActiveAnimations.h"
#include "core/animation/Animation.h" #include "core/animation/Animation.h"
...@@ -132,16 +133,19 @@ StyleResolver::StyleResolver(Document& document) ...@@ -132,16 +133,19 @@ StyleResolver::StyleResolver(Document& document)
: m_document(document) : m_document(document)
, m_viewportStyleResolver(ViewportStyleResolver::create(&document)) , m_viewportStyleResolver(ViewportStyleResolver::create(&document))
, m_needCollectFeatures(false) , m_needCollectFeatures(false)
, m_printMediaType(false)
, m_styleResourceLoader(document.fetcher()) , m_styleResourceLoader(document.fetcher())
, m_styleSharingDepth(0) , m_styleSharingDepth(0)
, m_styleResolverStatsSequence(0) , m_styleResolverStatsSequence(0)
, m_accessCount(0) , m_accessCount(0)
{ {
FrameView* view = document.view(); FrameView* view = document.view();
if (view) if (view) {
m_medium = adoptPtr(new MediaQueryEvaluator(&view->frame())); m_medium = adoptPtr(new MediaQueryEvaluator(&view->frame()));
else m_printMediaType = equalIgnoringCase(view->mediaType(), MediaTypeNames::print);
} else {
m_medium = adoptPtr(new MediaQueryEvaluator("all")); m_medium = adoptPtr(new MediaQueryEvaluator("all"));
}
initWatchedSelectorRules(CSSSelectorWatch::from(document).watchedCallbackSelectors()); initWatchedSelectorRules(CSSSelectorWatch::from(document).watchedCallbackSelectors());
...@@ -456,8 +460,7 @@ void StyleResolver::matchUARules(ElementRuleCollector& collector) ...@@ -456,8 +460,7 @@ void StyleResolver::matchUARules(ElementRuleCollector& collector)
collector.setMatchingUARules(true); collector.setMatchingUARules(true);
CSSDefaultStyleSheets& defaultStyleSheets = CSSDefaultStyleSheets::instance(); CSSDefaultStyleSheets& defaultStyleSheets = CSSDefaultStyleSheets::instance();
RuleSet* userAgentStyleSheet = m_medium->mediaTypeMatchSpecific("print") RuleSet* userAgentStyleSheet = m_printMediaType ? defaultStyleSheets.defaultPrintStyle() : defaultStyleSheets.defaultStyle();
? defaultStyleSheets.defaultPrintStyle() : defaultStyleSheets.defaultStyle();
matchUARules(collector, userAgentStyleSheet); matchUARules(collector, userAgentStyleSheet);
// In quirks mode, we match rules from the quirks user agent sheet. // In quirks mode, we match rules from the quirks user agent sheet.
......
...@@ -305,6 +305,7 @@ private: ...@@ -305,6 +305,7 @@ private:
TreeBoundaryCrossingRules m_treeBoundaryCrossingRules; TreeBoundaryCrossingRules m_treeBoundaryCrossingRules;
bool m_needCollectFeatures; bool m_needCollectFeatures;
bool m_printMediaType;
StyleResourceLoader m_styleResourceLoader; StyleResourceLoader m_styleResourceLoader;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment