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

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
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=162432

[tasak, 2013-11-15 15:22:27]

https://codereview.chromium.org/73993002/diff/1/Source/core/dom/StyleSheetCol...
File Source/core/dom/StyleSheetCollection.cpp (right):

https://codereview.chromium.org/73993002/diff/1/Source/core/dom/StyleSheetCol...
Source/core/dom/StyleSheetCollection.cpp:171: }
I found that the following code is better:

if (styleSheetContentsHasFontFaceRule(addedSheets)) {
  change.styleResolverUpdateType = ResetStyleResolverAndFontSelector;
  return;
}
// We can use StyleInvalidationAnalysis.
if (cssStyleSheetHasFontFaceRule(newStyleSheets))
  change.styleResolverUpdateType = ResetStyleResolverAndFontSelector;

[tasak, 2013-11-18 08:48:53]

Would you review this CL?

[ojan, 2013-11-21 00:57:47]

lgtm

https://codereview.chromium.org/73993002/diff/80001/Source/core/dom/StyleShee...
File Source/core/dom/StyleSheetCollection.cpp (right):

https://codereview.chromium.org/73993002/diff/80001/Source/core/dom/StyleShee...
Source/core/dom/StyleSheetCollection.cpp:170: // we can avoid appending all
stylesheetcontents in reset case.
I'm hoping that once this is done we can get rid of the Reset codepath entirely
and just do Reconstruct instead in those cases (we'd still keep Additive).
Recreating the StyleResolver should be very inexpensive if we're not recreating
the RuleSets.

[commit-bot: I haz the power, 2013-11-21 01:04:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tasak@google.com/73993002/80001

[commit-bot: I haz the power, 2013-11-21 01:33:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tasak@google.com/73993002/80001

[commit-bot: I haz the power, 2013-11-21 03:02:54]

Change committed as 162432

[ojan, 2013-11-22 01:08:54]

https://codereview.chromium.org/73993002/diff/80001/Source/core/dom/StyleShee...
File Source/core/dom/StyleSheetCollection.cpp (right):

https://codereview.chromium.org/73993002/diff/80001/Source/core/dom/StyleShee...
Source/core/dom/StyleSheetCollection.cpp:174: change.styleResolverUpdateType =
Reset;
Hmmm...not lgtm actually. Now that I look more closely at the patch, this line
will always override the work from the new code above.

[tasak, 2013-11-22 05:16:24]

https://codereview.chromium.org/73993002/diff/80001/Source/core/dom/StyleShee...
File Source/core/dom/StyleSheetCollection.cpp (right):

https://codereview.chromium.org/73993002/diff/80001/Source/core/dom/StyleShee...
Source/core/dom/StyleSheetCollection.cpp:170: // we can avoid appending all
stylesheetcontents in reset case.
On 2013/11/21 00:57:48, ojan wrote:
> I'm hoping that once this is done we can get rid of the Reset codepath
entirely
> and just do Reconstruct instead in those cases (we'd still keep Additive).

Yeah, I think, this patch is a temporary one.

> Recreating the StyleResolver should be very inexpensive if we're not
recreating
> the RuleSets.

I see.

https://codereview.chromium.org/73993002/diff/80001/Source/core/dom/StyleShee...
Source/core/dom/StyleSheetCollection.cpp:174: change.styleResolverUpdateType =
Reset;
On 2013/11/22 01:08:54, ojan wrote:
> Hmmm...not lgtm actually. Now that I look more closely at the patch, this line
> will always override the work from the new code above.

Yeah, It's my fault.

I'm now trying to land the fixing patch for this.