New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 4 users

Issue metadata

Status: Verified
Owner: ----
Closed: Apr 2014
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment
link

Issue 305885: Blink always recalcs all style (and repaints) when removing stylesheets

Reported by eseidel@chromium.org, Oct 10 2013 Project Member

Issue description

Blink always recalcs *all* style (and repaints) when appending/removing stylesheets

Chrome Version       : 30.0.1599.88

http://jsfiddle.net/GL4aE/

We only recalc the elements affected by the stylesheet.  In the case of the above example, nothing is affected and nothing should be recalculated.

We should be especially careful to avoid @rules which don't affect anything at all! :)  (like @keyframes)
 

Comment 1 by baker@google.com, Oct 10 2013

Labels: Hotlist-GoogleApps Hotlist-Jank

Comment 2 by eseidel@chromium.org, Oct 15 2013

Summary: Blink always recalcs all style (and repaints) when removing stylesheets (was: Blink always recalcs *all* style (and repaints) when appending/removing stylesheets)

Comment 3 by eseidel@chromium.org, Oct 15 2013

I've spun off the @keyframes issue as  issue 307725 .

Comment 4 by bugdroid1@chromium.org, Oct 16 2013

Project Member
The following revision refers to this bug:
    http://src.chromium.org/viewvc/blink?view=rev&rev=159727

------------------------------------------------------------------------
r159727 | eseidel@chromium.org | 2013-10-16T08:32:24.565197Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/Document.cpp?r1=159727&r2=159726&pathrev=159727
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/Document.h?r1=159727&r2=159726&pathrev=159727
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/StyleSheetCollection.cpp?r1=159727&r2=159726&pathrev=159727

Rename WebCore::StyleResolverUpdateType to RecalcStyleTime

This makes it better match what it actually controls (recalcStyle)
and avoids confusion with StyleSheetCollection::StyleResolverUpdateType
which has a completely different meaning.

BUG= 305885 

Review URL: https://codereview.chromium.org/26155004
------------------------------------------------------------------------

Comment 5 by tasak@google.com, Oct 17 2013

Cc: tasak@chromium.org

Comment 6 by bugdroid1@chromium.org, Oct 18 2013

Project Member
The following revision refers to this bug:
    http://src.chromium.org/viewvc/blink?view=rev&rev=159957

------------------------------------------------------------------------
r159957 | tasak@google.com | 2013-10-18T18:13:28.684158Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/Document.h?r1=159957&r2=159956&pathrev=159957
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/html/HTMLStyleElement.cpp?r1=159957&r2=159956&pathrev=159957
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/StyleSheetCollection.cpp?r1=159957&r2=159956&pathrev=159957
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/StyleEngine.cpp?r1=159957&r2=159956&pathrev=159957
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/resolver/ScopedStyleResolver.cpp?r1=159957&r2=159956&pathrev=159957
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/StyleElement.cpp?r1=159957&r2=159956&pathrev=159957
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSStyleSheet.cpp?r1=159957&r2=159956&pathrev=159957
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/StyleEngine.h?r1=159957&r2=159956&pathrev=159957
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/resolver/StyleResolver.cpp?r1=159957&r2=159956&pathrev=159957
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/Document.cpp?r1=159957&r2=159956&pathrev=159957
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/StyleSheetScopingNodeList.cpp?r1=159957&r2=159956&pathrev=159957
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/resolver/ScopedStyleResolver.h?r1=159957&r2=159956&pathrev=159957
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSStyleSheet.h?r1=159957&r2=159956&pathrev=159957

Avoid always style recalc when removing stylesheets.

Replaced FullStyleUpdate with AnalyzedStyleUpdate when:
- CSSStyleSheet::didMutate() is invoked from clearOwnerNode().
- StyleElement::removedFromDocument is invoked.

Modified StyleSheetCollection::analyzeStyleSheetChange when stylesheet contents are just removed:
- use Reset for StyleResolverUpdateType.
- use StyleInvalidationAnalysis for style recalc.


BUG= 305885 
TEST=All existing tests should pass after applying this patch.

Review URL: https://codereview.chromium.org/27537009
------------------------------------------------------------------------

Comment 9 by eseidel@chromium.org, Oct 22 2013

I think this is a progression.  It may not be related to this bug... but I'm rather new to this perf-sheriffing thing and figuring out the tools.

Comment 10 by tasak@google.com, Oct 22 2013

Before r159957, out/Release/content_shell --no-sandbox --dump-render-tree third_party/WebKit/PerformanceTests/CSS/StyleSheetInsert.html says:

Running 5 times
Ignoring warm-up run (10.637999977916479 ms)
0.8819999638944864 ms
0.9280000813305378 ms
0.8819999638944864 ms
0.863000052049756 ms
0.8600000292062759 ms

After r159957,
Running 5 times
Ignoring warm-up run (10.77900012023747 ms)
10.54599997587502 ms
10.511999949812889 ms
10.57699997909367 ms
10.52100001834333 ms
10.556000052019954 ms

Si warn-up run looks almost the same.
I'm not sure, but I guess, the test seems not to run 5 times before r159957. 
I will investigate.

Comment 11 by tasak@google.com, Oct 22 2013

I think, 0.88ms < 1ms is too fast.

Comment 12 by tasak@google.com, Oct 23 2013

I found the reason why CSS_StyleSheetInsert is affected by r159957.

CSS/StyleSheetInsert.html removes 50 style elements from document in 2nd, 3rd, 4th, 5th and 6th run,
because the test executes "testDoc.body.innerHTML = ''" in setup().

I'm not sure whether this is a rare case in the real Web or not, but at least we should have another test for removing style elements.

So is it ok to change the test in the following way?

diff --git a/PerformanceTests/CSS/StyleSheetInsert.html b/PerformanceTests/CSS/StyleSheetInsert.html
index df4f544..067a7fe 100644
--- a/PerformanceTests/CSS/StyleSheetInsert.html
+++ b/PerformanceTests/CSS/StyleSheetInsert.html
@@ -28,7 +28,9 @@ PerfTestRunner.measureTime({run:function() {
         styleElem.innerText = ".bar {color:green}";
         testDoc.body.insertBefore(styleElem, testDoc.body.firstChild);
     }
-    return PerfTestRunner.now() - start;
+    testDoc.body.innerHTML = '';
+    var result = PerfTestRunner.now() - start;
+    return result;
 }});
 </script>
 </html>

After applying the above change, the test returns the same result independent of r159957.

This also explains inserting style elements is very fast before r159957, i.e.
- In setup(), old code destroys StyleResolver.
- All following style insertion doesn't update StyleResolver because there are no StyleResolver.
- After inserting 50 style elements, StyleResolver is created by scheduled style recalc.

Comment 13 by tasak@google.com, Oct 23 2013

> CSS/StyleSheetInsert.html removes 50 style elements from document in 2nd, 3rd, 4th, 5th and 6th run,

I mean, 50 style elements are removed all at once.

If removing one by one (including style recalc and layout), r159957 improves the performance better, I think.

Comment 14 by tasak@google.com, Oct 23 2013

 crbug.com/310210  in chromium: 11% regression in chromium-rel-win8-dual/blink_perf/Parser_css-parser-yui at 229523:229542 is also caused by r159957.

I think, destroying StyleResolver makes all style-related benchmarks, i.e. inserting style elements, updating style's textContents and so on, faster.

Comment 15 by rsch...@chromium.org, Oct 23 2013

Cc: tonyg@chromium.org
+tonyg to comment on the test

Comment 16 by rsch...@chromium.org, Nov 5 2013

Ping on this.

Comment 17 by tasak@google.com, Nov 5 2013

I'm still working on https://codereview.chromium.org/42543007/ to fix performance regression.

Comment 18 by eseidel@chromium.org, Nov 7 2013

Cc: baker@google.com
I'm still seeing these when testing baker@'s app on TOT:

Invalidated: 4006
 [0x000000bc1413] base::debug::StackTrace::StackTrace()
 [0x00000182bdf8] WebCore::Node::setNeedsStyleRecalc()
 [0x000001b518c9] WebCore::StyleInvalidationAnalysis::invalidateStyle()
 [0x000001867593] WebCore::StyleSheetCollection::analyzeStyleSheetChange()
 [0x000001884910] WebCore::DocumentStyleSheetCollection::updateActiveStyleSheets()
 [0x000001864147] WebCore::StyleEngine::updateActiveStyleSheets()
 [0x0000017f3eef] WebCore::Document::styleResolverChanged()
 [0x000001b3d750] WebCore::CSSStyleSheet::clearOwnerNode()
 [0x00000390630d] WebCore::StyleElement::removedFromDocument()
 [0x0000017e4ae9] WebCore::ContainerNode::removeChild()
 [0x00000182b052] WebCore::Node::removeChild()
 [0x00000238ab48] WebCore::V8Node::removeChildMethodCustom()
 [0x00000230393f] WebCore::NodeV8Internal::removeChildMethodCallbackForMainWorld()
 [0x120c9a21229c] <unknown>

Comment 19 by eseidel@chromium.org, Nov 7 2013

I'm using https://codereview.chromium.org/61833006 to generate that printout, btw.  That's in a Release build so it's likely missing a couple stack frames.

Comment 20 by nduca@chromium.org, Nov 13 2013

Labels: SlowLayoutOrRecalcStyle

Comment 21 by bugdroid1@chromium.org, Nov 15 2013

Project Member
The following revision refers to this bug:
    http://src.chromium.org/viewvc/blink?view=rev&rev=162094

------------------------------------------------------------------------
r162094 | tasak@google.com | 2013-11-15T09:42:46.890336Z

Changed paths:
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/remove-style-after-insert-font-face-expected.html?r1=162094&r2=162093&pathrev=162094
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/ShadowTreeStyleSheetCollection.cpp?r1=162094&r2=162093&pathrev=162094
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/font-face-remove-expected.html?r1=162094&r2=162093&pathrev=162094
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/remove-style-after-insert-font-face.html?r1=162094&r2=162093&pathrev=162094
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/resolver/StyleResolver.cpp?r1=162094&r2=162093&pathrev=162094
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/font-face-remove.html?r1=162094&r2=162093&pathrev=162094
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/StyleSheetContents.cpp?r1=162094&r2=162093&pathrev=162094
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/resolver/StyleResolver.h?r1=162094&r2=162093&pathrev=162094
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSParser-in.cpp?r1=162094&r2=162093&pathrev=162094
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/remove-style-after-remove-font-face-expected.html?r1=162094&r2=162093&pathrev=162094
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/DocumentStyleSheetCollection.cpp?r1=162094&r2=162093&pathrev=162094
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/remove-style-after-remove-font-face.html?r1=162094&r2=162093&pathrev=162094
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/StyleSheetContents.h?r1=162094&r2=162093&pathrev=162094
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/StyleSheetCollection.cpp?r1=162094&r2=162093&pathrev=162094
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/StyleSheetCollection.h?r1=162094&r2=162093&pathrev=162094

fontSelector should be reset when removing stylesheets which has @font-face rule.

r159957 changed to avoid destroying StyleResolver when removing stylesheets.
However if the removed stylesheets has any @font-face rule, we need to reset not only rulesets,
but fontselector.

BUG= 305885 
TEST=fast/css/font-face-remove.html,fast/css/remove-style-after-insert-font-face.html,fast/css/remove-style-after-remove-font-face.html

Review URL: https://codereview.chromium.org/66483002
------------------------------------------------------------------------

Comment 22 by eseidel@chromium.org, Nov 15 2013

I was wrong in comment 18/19, btw.  That stack is showing that your logic clearly *is* working.  Before your change the whole document would have been invalidated outside of StyleInvalidationAnalysis::invalidateStyle().

Comment 23 by bugdroid1@chromium.org, Nov 18 2013

Project Member
The following revision refers to this bug:
    http://src.chromium.org/viewvc/blink?view=rev&rev=162180

------------------------------------------------------------------------
r162180 | tasak@google.com | 2013-11-18T06:42:09.767604Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/SelectorQuery.cpp?r1=162180&r2=162179&pathrev=162180
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/StyleEngine.cpp?r1=162180&r2=162179&pathrev=162180
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/Element.cpp?r1=162180&r2=162179&pathrev=162180
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/FontFaceSet.cpp?r1=162180&r2=162179&pathrev=162180
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/Node.h?r1=162180&r2=162179&pathrev=162180
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/StyleSheetScopingNodeList.cpp?r1=162180&r2=162179&pathrev=162180
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/ElementRuleCollector.cpp?r1=162180&r2=162179&pathrev=162180
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/RenderRegion.cpp?r1=162180&r2=162179&pathrev=162180
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/animation/css/CSSAnimations.cpp?r1=162180&r2=162179&pathrev=162180
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/SelectorChecker.h?r1=162180&r2=162179&pathrev=162180
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/StyleSheetScopingNodeList.h?r1=162180&r2=162179&pathrev=162180
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/inspector/InspectorCSSAgent.cpp?r1=162180&r2=162179&pathrev=162180

[Refactoring] Remove include "core/inspector/InspectorInstrumentation.h" from SelectorChecker.h

To make it possible to add include "StyleResolver.h" to StyleEngine.h,
We need to remove the above include from SelectorChecker.h.

BUG= 305885 
TEST=No layout tests, just refactoring.

Review URL: https://codereview.chromium.org/70063005
------------------------------------------------------------------------

Comment 24 by bugdroid1@chromium.org, Nov 18 2013

Project Member
The following revision refers to this bug:
    http://src.chromium.org/viewvc/blink?view=rev&rev=162187

------------------------------------------------------------------------
r162187 | tasak@google.com | 2013-11-18T10:43:48.591875Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/Element.cpp?r1=162187&r2=162186&pathrev=162187
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/resolver/ScopedStyleResolver.cpp?r1=162187&r2=162186&pathrev=162187
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/ShadowTreeStyleSheetCollection.cpp?r1=162187&r2=162186&pathrev=162187
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/StyleEngine.h?r1=162187&r2=162186&pathrev=162187
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/RuleSet.cpp?r1=162187&r2=162186&pathrev=162187
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/resolver/StyleResolver.cpp?r1=162187&r2=162186&pathrev=162187
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/Document.cpp?r1=162187&r2=162186&pathrev=162187
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/resolver/ScopedStyleResolver.h?r1=162187&r2=162186&pathrev=162187
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/RuleSet.h?r1=162187&r2=162186&pathrev=162187
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/resolver/StyleResolver.h?r1=162187&r2=162186&pathrev=162187
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/loader/resource-request-callbacks-expected.txt?r1=162187&r2=162186&pathrev=162187
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/DocumentStyleSheetCollection.cpp?r1=162187&r2=162186&pathrev=162187
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/StyleRule.h?r1=162187&r2=162186&pathrev=162187
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/StyleEngine.cpp?r1=162187&r2=162186&pathrev=162187

StyleResolver should update RuleSets lazily.

This is a fixing patch for preformance regression caused by r159957.
Before applying r159957, removing a style element (or modifying a style element's textcontent)
destroys StyleResolver, e.g. style.textContent = '';

After applying r159957, we don't need to destroy StyleResolver.
The r159957 makes us to avoid FullStyleRecalc and full RuleSet re-creation (document,
and shadow trees).

But the patch caused Parser/css-parser-yui.html's performance regression.
Because firstly the performance test destroys StyleResolver and skip StyleResolver::appendAuthorStyleSheets.
(If we have no StyleResolver, we don't need to update RuleSet, StyleInvalidationAnalysis, and so on)

So we should update RuleSets lazily. This solution improves Parser/css-parser-yui.html's performance again.

BUG= 305885 , 308796 
TEST=all existing tests should pass.

Review URL: https://codereview.chromium.org/42543007
------------------------------------------------------------------------

Comment 26 by bugdroid1@chromium.org, Nov 19 2013

Project Member
The following revision refers to this bug:
    http://src.chromium.org/viewvc/blink?view=rev&rev=162247

------------------------------------------------------------------------
r162247 | zmo@chromium.org | 2013-11-19T00:00:17.227051Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/ElementRuleCollector.cpp?r1=162247&r2=162246&pathrev=162247
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/RenderRegion.cpp?r1=162247&r2=162246&pathrev=162247
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/animation/css/CSSAnimations.cpp?r1=162247&r2=162246&pathrev=162247
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/SelectorChecker.h?r1=162247&r2=162246&pathrev=162247
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/StyleSheetScopingNodeList.h?r1=162247&r2=162246&pathrev=162247
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/inspector/InspectorCSSAgent.cpp?r1=162247&r2=162246&pathrev=162247
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/SelectorQuery.cpp?r1=162247&r2=162246&pathrev=162247
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/StyleEngine.cpp?r1=162247&r2=162246&pathrev=162247
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/Element.cpp?r1=162247&r2=162246&pathrev=162247
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/FontFaceSet.cpp?r1=162247&r2=162246&pathrev=162247
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/Node.h?r1=162247&r2=162246&pathrev=162247
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/StyleSheetScopingNodeList.cpp?r1=162247&r2=162246&pathrev=162247

Revert 162180 "[Refactoring] Remove include "core/inspector/Insp..."

One of the two CLs that might cause fast/frames/seamless/seamless-nested-crash.html to timeout in debug.  Can't reproduce locally, so attempting reverting to confirm.  Will reland if wrong.

> [Refactoring] Remove include "core/inspector/InspectorInstrumentation.h" from SelectorChecker.h
> 
> To make it possible to add include "StyleResolver.h" to StyleEngine.h,
> We need to remove the above include from SelectorChecker.h.
> 
> BUG= 305885 
> TEST=No layout tests, just refactoring.
> 
> Review URL: https://codereview.chromium.org/70063005

TBR=tasak@google.com

Review URL: https://codereview.chromium.org/73623006
------------------------------------------------------------------------

Comment 27 by bugdroid1@chromium.org, Nov 19 2013

Project Member
The following revision refers to this bug:
    http://src.chromium.org/viewvc/blink?view=rev&rev=162248

------------------------------------------------------------------------
r162248 | zmo@chromium.org | 2013-11-19T00:17:27.063409Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/animation/css/CSSAnimations.cpp?r1=162248&r2=162247&pathrev=162248
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/SelectorChecker.h?r1=162248&r2=162247&pathrev=162248
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/StyleSheetScopingNodeList.h?r1=162248&r2=162247&pathrev=162248
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/inspector/InspectorCSSAgent.cpp?r1=162248&r2=162247&pathrev=162248
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/SelectorQuery.cpp?r1=162248&r2=162247&pathrev=162248
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/StyleEngine.cpp?r1=162248&r2=162247&pathrev=162248
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/Element.cpp?r1=162248&r2=162247&pathrev=162248
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/FontFaceSet.cpp?r1=162248&r2=162247&pathrev=162248
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/Node.h?r1=162248&r2=162247&pathrev=162248
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/StyleSheetScopingNodeList.cpp?r1=162248&r2=162247&pathrev=162248
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/ElementRuleCollector.cpp?r1=162248&r2=162247&pathrev=162248
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/RenderRegion.cpp?r1=162248&r2=162247&pathrev=162248

Revert 162247 "Revert 162180 "[Refactoring] Remove include "core..."

Reland as reverting breaking all builders.

> Revert 162180 "[Refactoring] Remove include "core/inspector/Insp..."
> 
> One of the two CLs that might cause fast/frames/seamless/seamless-nested-crash.html to timeout in debug.  Can't reproduce locally, so attempting reverting to confirm.  Will reland if wrong.
> 
> > [Refactoring] Remove include "core/inspector/InspectorInstrumentation.h" from SelectorChecker.h
> > 
> > To make it possible to add include "StyleResolver.h" to StyleEngine.h,
> > We need to remove the above include from SelectorChecker.h.
> > 
> > BUG= 305885 
> > TEST=No layout tests, just refactoring.
> > 
> > Review URL: https://codereview.chromium.org/70063005
> 
> TBR=tasak@google.com
> 
> Review URL: https://codereview.chromium.org/73623006

TBR=zmo@chromium.org

Review URL: https://codereview.chromium.org/75463008
------------------------------------------------------------------------

Comment 28 by bugdroid1@chromium.org, Nov 21 2013

Project Member
The following revision refers to this bug:
    http://src.chromium.org/viewvc/blink?view=rev&rev=162432

------------------------------------------------------------------------
r162432 | tasak@google.com | 2013-11-21T03:02:52.788873Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/StyleSheetCollection.cpp?r1=162432&r2=162431&pathrev=162432

Reset fontselector() when removing stylesheets and left stylesheets still have @font-face rules.

If not reset, CSSSegmentedFontFaceCache has mulitple fontFaces for the same @font-face rule.
This causes fast/css/font-face-html-as-svg flaky (Timeout in mac-blink-rel).

BUG= 305885 , 309370 

Review URL: https://codereview.chromium.org/73993002
------------------------------------------------------------------------

Comment 29 by bugdroid1@chromium.org, Nov 22 2013

Project Member
The following revision refers to this bug:
    http://src.chromium.org/viewvc/blink?view=rev&rev=162527

------------------------------------------------------------------------
r162527 | tasak@google.com | 2013-11-22T08:57:00.746236Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/LayoutTests/TestExpectations?r1=162527&r2=162526&pathrev=162527
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/StyleSheetCollection.cpp?r1=162527&r2=162526&pathrev=162527

Should not override styleResolverUpdateType to Reset when some existing stylesheet still has @font-face rule.

BUG= 305885 ,  309370 
TEST=fast/css/font-face-html-as-svg.html (Mac)

Review URL: https://codereview.chromium.org/82643002
------------------------------------------------------------------------

Comment 30 by tasak@google.com, Nov 22 2013

I chatted with ksakamoto@.

He said, now CSSFontSegmentCache checks StyleRuleFontFace , and if a given rule is duplicate one, CSSFontSegementCache skips the rule.

So I'm now trying to remove ResetStyleResolverAndFontSelector and to use removeFontFace instead.

I created a patch: https://codereview.chromium.org/82583005/

However the patch is still WIP. I need to fix fast/css/font-face-remove.html's regression.

Comment 31 by bugdroid1@chromium.org, Nov 25 2013

Project Member
The following revision refers to this bug:
    http://src.chromium.org/viewvc/blink?view=rev&rev=162598

------------------------------------------------------------------------
r162598 | ksakamoto@chromium.org | 2013-11-25T06:11:56.803598Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSSegmentedFontFaceCache.cpp?r1=162598&r2=162597&pathrev=162598

Increment CSSSegmentedFontFaceCache version when removing FontFaceRule

This prevents old font data (before the style change) from being used.

BUG= 305885 
TEST=no test, this code path is not used yet

Review URL: https://codereview.chromium.org/85053002
------------------------------------------------------------------------

Comment 32 by dglazkov@chromium.org, Dec 3 2013

Cc: e...@chromium.org eseidel@chromium.org
 Issue 267622  has been merged into this issue.

Comment 33 by bugdroid1@chromium.org, Dec 7 2013

Project Member
The following revision refers to this bug:
    http://src.chromium.org/viewvc/blink?view=rev&rev=163369

------------------------------------------------------------------------
r163369 | tasak@google.com | 2013-12-07T03:05:26.186859Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/FontFaceSet.cpp?r1=163369&r2=163368&pathrev=163369
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/StyleEngine.h?r1=163369&r2=163368&pathrev=163369
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/resolver/StyleResolver.cpp?r1=163369&r2=163368&pathrev=163369
   M http://src.chromium.org/viewvc/blink/trunk/Source/web/AutofillPopupMenuClient.cpp?r1=163369&r2=163368&pathrev=163369
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/Document.cpp?r1=163369&r2=163368&pathrev=163369
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/RenderMenuList.cpp?r1=163369&r2=163368&pathrev=163369
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/svg/RenderSVGInlineText.cpp?r1=163369&r2=163368&pathrev=163369
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/resolver/StyleResolver.h?r1=163369&r2=163368&pathrev=163369
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/rendering/RenderListBox.cpp?r1=163369&r2=163368&pathrev=163369
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/Document.h?r1=163369&r2=163368&pathrev=163369
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/DocumentStyleSheetCollection.cpp?r1=163369&r2=163368&pathrev=163369
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/html/canvas/CanvasRenderingContext2D.cpp?r1=163369&r2=163368&pathrev=163369
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/StyleEngine.cpp?r1=163369&r2=163368&pathrev=163369

Moving fontSelector from StyleResolver to StyleEngine.

To avoid dispatching fontLoaded event multiple times for the same font (caused by clearResolver), we should change fontSelector's owner.

BUG= 236298 , 305885 

Review URL: https://codereview.chromium.org/87503003
------------------------------------------------------------------------

Comment 34 by rsch...@chromium.org, Jan 6 2014

What's the latest status here? Are more changes needed?

Comment 35 by bugdroid1@chromium.org, Jan 8 2014

Project Member
The following revision refers to this bug:
    http://src.chromium.org/viewvc/blink?view=rev&rev=164691

------------------------------------------------------------------------
r164691 | tasak@google.com | 2014-01-08T17:42:49.542913Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/StyleEngine.h?r1=164691&r2=164690&pathrev=164691
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/testing/InternalSettings.cpp?r1=164691&r2=164690&pathrev=164691
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/DocumentStyleSheetCollection.cpp?r1=164691&r2=164690&pathrev=164691
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/StyleSheetCollection.cpp?r1=164691&r2=164690&pathrev=164691
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/page/Page.cpp?r1=164691&r2=164690&pathrev=164691
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/StyleSheetCollection.h?r1=164691&r2=164690&pathrev=164691
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/frame/Settings.cpp?r1=164691&r2=164690&pathrev=164691
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/frame/SettingsDelegate.h?r1=164691&r2=164690&pathrev=164691
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/cssom-modify-font-face-rule-expected.html?r1=164691&r2=164690&pathrev=164691
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/cssom-modify-font-face-rule.html?r1=164691&r2=164690&pathrev=164691
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/Document.cpp?r1=164691&r2=164690&pathrev=164691
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/StyleSheetContents.cpp?r1=164691&r2=164690&pathrev=164691
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/frame/Settings.h?r1=164691&r2=164690&pathrev=164691
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/Document.h?r1=164691&r2=164690&pathrev=164691
   M http://src.chromium.org/viewvc/blink/trunk/Source/platform/fonts/FontDescription.h?r1=164691&r2=164690&pathrev=164691
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/StyleSheetContents.h?r1=164691&r2=164690&pathrev=164691
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/font-face-insertBefore-expected.html?r1=164691&r2=164690&pathrev=164691
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/StyleEngine.cpp?r1=164691&r2=164690&pathrev=164691
   M http://src.chromium.org/viewvc/blink/trunk/Source/web/WebSettingsImpl.cpp?r1=164691&r2=164690&pathrev=164691
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/font-face-insertBefore.html?r1=164691&r2=164690&pathrev=164691

Use removeFontFace to avoid resetting fontSelector.

BUG= 305885 
TEST=fast/css/font-face-remove.html, fast/css/font-face-html-as-svg.html

Review URL: https://codereview.chromium.org/82583005
------------------------------------------------------------------------

Comment 36 by bugdroid1@chromium.org, Jan 9 2014

Project Member
The following revision refers to this bug:
    http://src.chromium.org/viewvc/blink?view=rev&rev=164783

------------------------------------------------------------------------
r164783 | jochen@chromium.org | 2014-01-09T15:42:19.283693Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/DocumentStyleSheetCollection.cpp?r1=164783&r2=164782&pathrev=164783
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/StyleSheetCollection.cpp?r1=164783&r2=164782&pathrev=164783
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/page/Page.cpp?r1=164783&r2=164782&pathrev=164783
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/StyleSheetCollection.h?r1=164783&r2=164782&pathrev=164783
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/frame/Settings.cpp?r1=164783&r2=164782&pathrev=164783
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/frame/SettingsDelegate.h?r1=164783&r2=164782&pathrev=164783
   D http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/cssom-modify-font-face-rule-expected.html?r1=164783&r2=164782&pathrev=164783
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/Document.cpp?r1=164783&r2=164782&pathrev=164783
   D http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/cssom-modify-font-face-rule.html?r1=164783&r2=164782&pathrev=164783
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/StyleSheetContents.cpp?r1=164783&r2=164782&pathrev=164783
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/frame/Settings.h?r1=164783&r2=164782&pathrev=164783
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/Document.h?r1=164783&r2=164782&pathrev=164783
   M http://src.chromium.org/viewvc/blink/trunk/Source/platform/fonts/FontDescription.h?r1=164783&r2=164782&pathrev=164783
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/StyleSheetContents.h?r1=164783&r2=164782&pathrev=164783
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/StyleEngine.cpp?r1=164783&r2=164782&pathrev=164783
   D http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/font-face-insertBefore-expected.html?r1=164783&r2=164782&pathrev=164783
   M http://src.chromium.org/viewvc/blink/trunk/Source/web/WebSettingsImpl.cpp?r1=164783&r2=164782&pathrev=164783
   D http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/font-face-insertBefore.html?r1=164783&r2=164782&pathrev=164783
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/testing/InternalSettings.cpp?r1=164783&r2=164782&pathrev=164783
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/StyleEngine.h?r1=164783&r2=164782&pathrev=164783

Revert of https://codereview.chromium.org/82583005/
Reason for revert: This broke PrintWebViewHelperTest.PrintLayoutTest on Mac

TBR=ksakamoto@chromium.org,esprehn@chromium.org,ojan@chromium.org,dglazkov@chromium.org,tasak@google.com
NOTREECHECKS=true
NOTRY=true
BUG= 305885 

Review URL: https://codereview.chromium.org/131403008
------------------------------------------------------------------------

Comment 37 by bugdroid1@chromium.org, Jan 18 2014

Project Member
The following revision refers to this bug:
    http://src.chromium.org/viewvc/blink?view=rev&rev=165348

------------------------------------------------------------------------
r165348 | dglazkov@chromium.org | 2014-01-18T00:02:35.603501Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSFontSelector.cpp?r1=165348&r2=165347&pathrev=165348
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/FontFaceSet.cpp?r1=165348&r2=165347&pathrev=165348
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/resolver/StyleResolver.cpp?r1=165348&r2=165347&pathrev=165348
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSFontSelector.h?r1=165348&r2=165347&pathrev=165348

Remove indirection plumbing in CSSFontSelector.

Now that fontFaceCache is a direct accessor, we no longer
need wrappers around adding/removing items to/from it.

R=ksakamoto@chromium.org,eae
BUG= 305885 

Review URL: https://codereview.chromium.org/141553003
------------------------------------------------------------------------

Comment 38 by bugdroid1@chromium.org, Feb 5 2014

Project Member
------------------------------------------------------------------------
r249043 | tasak@google.com | 2014-02-05T17:50:41.189802Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/public/test/render_view_test.cc?r1=249043&r2=249042&pathrev=249043
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/public/test/render_view_test.h?r1=249043&r2=249042&pathrev=249043

Added ScopedNSAutoreleasePool to RenderViewTest.

If Blink checks font metrics during some browser_test (e.g. PrintLayoutTest), Blink might allocates CFString by using Mac allocator, and the CFString might have a reference to StringImpl allocated by FastMalloc.

So we should invoke Mac decallocator before destroying Blink instance. If we don't, browser_tests will crash.

BUG= 305885 ,  336756 

Review URL: https://codereview.chromium.org/137023014
------------------------------------------------------------------------

Comment 39 by bugdroid1@chromium.org, Feb 7 2014

Project Member
The following revision refers to this bug:
    http://src.chromium.org/viewvc/blink?view=rev&rev=166681

------------------------------------------------------------------------
r166681 | tasak@google.com | 2014-02-07T05:43:32.656576Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSFontSelector.h?r1=166681&r2=166680&pathrev=166681
   M http://src.chromium.org/viewvc/blink/trunk/Source/platform/fonts/GenericFontFamilySettings.h?r1=166681&r2=166680&pathrev=166681
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/DocumentStyleSheetCollection.cpp?r1=166681&r2=166680&pathrev=166681
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/page/Page.cpp?r1=166681&r2=166680&pathrev=166681
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/frame/SettingsDelegate.h?r1=166681&r2=166680&pathrev=166681
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/cssom-modify-font-face-rule-expected.html?r1=166681&r2=166680&pathrev=166681
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/cssom-modify-font-face-rule.html?r1=166681&r2=166680&pathrev=166681
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/TreeScopeStyleSheetCollection.cpp?r1=166681&r2=166680&pathrev=166681
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/StyleSheetContents.cpp?r1=166681&r2=166680&pathrev=166681
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/frame/Settings.h?r1=166681&r2=166680&pathrev=166681
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/FontFaceCache.cpp?r1=166681&r2=166680&pathrev=166681
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/TreeScopeStyleSheetCollection.h?r1=166681&r2=166680&pathrev=166681
   M http://src.chromium.org/viewvc/blink/trunk/Source/platform/fonts/FontDescription.h?r1=166681&r2=166680&pathrev=166681
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/StyleSheetContents.h?r1=166681&r2=166680&pathrev=166681
   M http://src.chromium.org/viewvc/blink/trunk/Source/web/WebSettingsImpl.cpp?r1=166681&r2=166680&pathrev=166681
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/font-face-insertBefore-expected.html?r1=166681&r2=166680&pathrev=166681
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/StyleEngine.cpp?r1=166681&r2=166680&pathrev=166681
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/font-face-insertBefore.html?r1=166681&r2=166680&pathrev=166681
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/FontFaceCache.h?r1=166681&r2=166680&pathrev=166681
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSFontSelector.cpp?r1=166681&r2=166680&pathrev=166681
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/StyleEngine.h?r1=166681&r2=166680&pathrev=166681
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/testing/InternalSettings.cpp?r1=166681&r2=166680&pathrev=166681
   M http://src.chromium.org/viewvc/blink/trunk/Source/platform/fonts/GenericFontFamilySettings.cpp?r1=166681&r2=166680&pathrev=166681

Use removeFontFace to avoid resetting fontSelector.

BUG= 305885 , 336756 
TEST=fast/css/font-face-remove.html, fast/css/font-face-html-as-svg.html

Review URL: https://codereview.chromium.org/82583005
------------------------------------------------------------------------

Comment 40 by bugdroid1@chromium.org, Feb 7 2014

Project Member
The following revision refers to this bug:
    http://src.chromium.org/viewvc/blink?view=rev&rev=166732

------------------------------------------------------------------------
r166732 | abarth@chromium.org | 2014-02-07T19:39:21.701942Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSFontSelector.h?r1=166732&r2=166731&pathrev=166732
   M http://src.chromium.org/viewvc/blink/trunk/Source/platform/fonts/GenericFontFamilySettings.h?r1=166732&r2=166731&pathrev=166732
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/DocumentStyleSheetCollection.cpp?r1=166732&r2=166731&pathrev=166732
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/page/Page.cpp?r1=166732&r2=166731&pathrev=166732
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/frame/SettingsDelegate.h?r1=166732&r2=166731&pathrev=166732
   D http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/cssom-modify-font-face-rule-expected.html?r1=166732&r2=166731&pathrev=166732
   D http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/cssom-modify-font-face-rule.html?r1=166732&r2=166731&pathrev=166732
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/TreeScopeStyleSheetCollection.cpp?r1=166732&r2=166731&pathrev=166732
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/frame/Settings.h?r1=166732&r2=166731&pathrev=166732
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/StyleSheetContents.cpp?r1=166732&r2=166731&pathrev=166732
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/FontFaceCache.cpp?r1=166732&r2=166731&pathrev=166732
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/TreeScopeStyleSheetCollection.h?r1=166732&r2=166731&pathrev=166732
   M http://src.chromium.org/viewvc/blink/trunk/Source/platform/fonts/FontDescription.h?r1=166732&r2=166731&pathrev=166732
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/StyleSheetContents.h?r1=166732&r2=166731&pathrev=166732
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/StyleEngine.cpp?r1=166732&r2=166731&pathrev=166732
   M http://src.chromium.org/viewvc/blink/trunk/Source/web/WebSettingsImpl.cpp?r1=166732&r2=166731&pathrev=166732
   D http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/font-face-insertBefore-expected.html?r1=166732&r2=166731&pathrev=166732
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/FontFaceCache.h?r1=166732&r2=166731&pathrev=166732
   D http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/font-face-insertBefore.html?r1=166732&r2=166731&pathrev=166732
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSFontSelector.cpp?r1=166732&r2=166731&pathrev=166732
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/testing/InternalSettings.cpp?r1=166732&r2=166731&pathrev=166732
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/StyleEngine.h?r1=166732&r2=166731&pathrev=166732
   M http://src.chromium.org/viewvc/blink/trunk/Source/platform/fonts/GenericFontFamilySettings.cpp?r1=166732&r2=166731&pathrev=166732

Revert of Use removeFontFace to avoid resetting fontSelector. (https://codereview.chromium.org/82583005/)

Reason for revert:
This CL appears to have broken

org.chromium.android_webview.test.AwSettingsTest#testLayoutAlgorithmWithTwoViews

in androidwebview_instrumentation_tests.

http://build.chromium.org/p/chromium.webkit/builders/Android%20Tests%20%28dbg%29/builds/17264

Original issue's description:
> Use removeFontFace to avoid resetting fontSelector.
> 
> BUG= 305885 , 336756 
> TEST=fast/css/font-face-remove.html, fast/css/font-face-html-as-svg.html
> 
> Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=166681

TBR=ksakamoto@chromium.org,esprehn@chromium.org,ojan@chromium.org,dglazkov@chromium.org,tasak@google.com
NOTREECHECKS=true
NOTRY=true
BUG= 305885 , 336756 

Review URL: https://codereview.chromium.org/157853002
------------------------------------------------------------------------

Comment 41 by bugdroid1@chromium.org, Feb 18 2014

Project Member
The following revision refers to this bug:
    http://src.chromium.org/viewvc/blink?view=rev&rev=167257

------------------------------------------------------------------------
r167257 | tasak@google.com | 2014-02-17T03:50:24.617426Z

Changed paths:
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/frame/SettingsDelegate.h?r1=167257&r2=167256&pathrev=167257
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/frame/Settings.cpp?r1=167257&r2=167256&pathrev=167257
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/cssom-modify-font-face-rule-expected.html?r1=167257&r2=167256&pathrev=167257
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/cssom-modify-font-face-rule.html?r1=167257&r2=167256&pathrev=167257
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/page/Page.h?r1=167257&r2=167256&pathrev=167257
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/TreeScopeStyleSheetCollection.cpp?r1=167257&r2=167256&pathrev=167257
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/StyleSheetContents.cpp?r1=167257&r2=167256&pathrev=167257
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/frame/Settings.h?r1=167257&r2=167256&pathrev=167257
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/FontFaceCache.cpp?r1=167257&r2=167256&pathrev=167257
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/TreeScopeStyleSheetCollection.h?r1=167257&r2=167256&pathrev=167257
   M http://src.chromium.org/viewvc/blink/trunk/Source/platform/fonts/FontDescription.h?r1=167257&r2=167256&pathrev=167257
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/StyleSheetContents.h?r1=167257&r2=167256&pathrev=167257
   M http://src.chromium.org/viewvc/blink/trunk/Source/web/WebSettingsImpl.cpp?r1=167257&r2=167256&pathrev=167257
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/font-face-insertBefore-expected.html?r1=167257&r2=167256&pathrev=167257
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/StyleEngine.cpp?r1=167257&r2=167256&pathrev=167257
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/FontFaceCache.h?r1=167257&r2=167256&pathrev=167257
   A http://src.chromium.org/viewvc/blink/trunk/LayoutTests/fast/css/font-face-insertBefore.html?r1=167257&r2=167256&pathrev=167257
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSFontSelector.cpp?r1=167257&r2=167256&pathrev=167257
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/testing/InternalSettings.cpp?r1=167257&r2=167256&pathrev=167257
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/StyleEngine.h?r1=167257&r2=167256&pathrev=167257
   M http://src.chromium.org/viewvc/blink/trunk/Source/platform/fonts/GenericFontFamilySettings.cpp?r1=167257&r2=167256&pathrev=167257
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/css/CSSFontSelector.h?r1=167257&r2=167256&pathrev=167257
   M http://src.chromium.org/viewvc/blink/trunk/Source/platform/fonts/GenericFontFamilySettings.h?r1=167257&r2=167256&pathrev=167257
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/dom/DocumentStyleSheetCollection.cpp?r1=167257&r2=167256&pathrev=167257
   M http://src.chromium.org/viewvc/blink/trunk/Source/core/page/Page.cpp?r1=167257&r2=167256&pathrev=167257

Use removeFontFace to avoid resetting fontSelector.

BUG= 305885 , 336756 
TEST=fast/css/font-face-remove.html, fast/css/font-face-html-as-svg.html

Review URL: https://codereview.chromium.org/82583005
------------------------------------------------------------------------

Comment 42 by rsch...@chromium.org, Mar 24 2014

Are we declaring this resolved?

Comment 43 by rsch...@chromium.org, Apr 29 2014

Status: Fixed
Declaring this resolved. Reopen if I'm wrong.

Comment 44 by krisr@chromium.org, May 14 2014

Status: Verified

Comment 45 by laforge@google.com, Jan 9 2015

Labels: -Cr-Blink-Rendering Cr-Blink-Layout
Migrate from Cr-Blink-Rendering to Cr-Blink-Layout

Sign in to add a comment