Made StyleEngine CSSFontSelectorClient instead of StyleResolver.

Made StyleEngine CSSFontSelectorClient instead of StyleResolver.

BUG=369401
TEST=All tests should pass, because of just refactoring.

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=175255

[tasak, 2014-05-29 04:28:16]

Would you review this CL?

[Kunihiko Sakamoto, 2014-05-29 05:56:52]

non-owner lgtm

https://codereview.chromium.org/304083003/diff/20001/Source/core/dom/StyleEng...
File Source/core/dom/StyleEngine.cpp (right):

https://codereview.chromium.org/304083003/diff/20001/Source/core/dom/StyleEng...
Source/core/dom/StyleEngine.cpp:465: ASSERT(m_fontSelector || !m_resolver);
Maybe remove m_fontSelector assertion here and the above?

[tasak, 2014-05-30 04:18:27]

Thank you for reviewing.

https://codereview.chromium.org/304083003/diff/20001/Source/core/dom/StyleEng...
File Source/core/dom/StyleEngine.cpp (right):

https://codereview.chromium.org/304083003/diff/20001/Source/core/dom/StyleEng...
Source/core/dom/StyleEngine.cpp:465: ASSERT(m_fontSelector || !m_resolver);
On 2014/05/29 05:56:53, Kunihiko Sakamoto wrote:
> Maybe remove m_fontSelector assertion here and the above?

Done.

[dglazkov, 2014-05-30 15:26:40]

lgtm

[tasak, 2014-06-02 03:42:43]

The CQ bit was checked by tasak@google.com

[commit-bot: I haz the power, 2014-06-02 03:42:58]

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

[commit-bot: I haz the power, 2014-06-02 04:38:23]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/9793)
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/10103)

[commit-bot: I haz the power, 2014-06-02 05:13:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-06-02 05:13:19]

Try jobs failed on following builders:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/10107)

[eseidel, 2014-06-02 05:29:01]

Do we have a diagram of how these all fit together?  My understanding was that
STyleREsolver was slowly becoming a static-only class, with StyleResolverstate
holding the state and StyleEngine being the front-end API to the rest of core/

Would be nice to see a drawing of how these fit together or a short doc
explaining more of the grand plan. :)  Maybe Dimitri has one of these tucked
away?

[tasak, 2014-06-02 07:42:34]

The CQ bit was checked by tasak@google.com

[commit-bot: I haz the power, 2014-06-02 07:42:44]

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

[tasak, 2014-06-02 08:03:17]

On 2014/06/02 05:29:01, eseidel wrote:
> Do we have a diagram of how these all fit together?  My understanding was that
> STyleREsolver was slowly becoming a static-only class, with StyleResolverstate
> holding the state and StyleEngine being the front-end API to the rest of core/
> 
> Would be nice to see a drawing of how these fit together or a short doc
> explaining more of the grand plan. :)  Maybe Dimitri has one of these tucked
> away?

I will describe a current diagram of these:

https://docs.google.com/document/d/1AHBj51N36Vazq2qqrqY1Rsp0dqjx_OhwiAEd_BnOW...

I think, this patch would help us to make StyleResolver a static-only class,
because CSSFontSelector doesn't need any instance of StyleResolver (as
CSSFontSelectorClient) after applying this patch.

I'm not sure, however I expect, StyleEngine could use appropriate
StyleInvalidationAnalysis for fontsUpdate to minimize StyleRecalc.

[commit-bot: I haz the power, 2014-06-02 08:49:49]

Change committed as 175255

[eseidel, 2014-06-02 17:27:33]

Thank you very much Takashi.

On Mon, Jun 2, 2014 at 1:49 AM,  <commit-bot@chromium.org> wrote:
> Change committed as 175255
>
> https://codereview.chromium.org/304083003/

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.