Need to update first-line when font finish loading.

Need to update first-line when font finish loading.

Another regression from baseLayoutStyle implementation where operator==
is not affected by the loading/loaded state of web fonts. To get a re-
layout and re-rendering when a web font loading state changes, we need to
check for the loading state. In this case, there was a missing check for
::first-line pseudo style.

Also, moved the code which marks for re-layout with accompanying FIXME
into LayoutObject.

BUG=501564, 504864
TEST=fast/text/ellipsis-platform-font-change.html

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

[rune, 2015-06-29 20:54:17]

rune@opera.com changed reviewers:
+ h.joshi@samsung.com

[rune, 2015-06-29 20:56:57]

h.joshi@: can you take a look the image failures in linux/win/mac_blink_rel and
see if it just looks like they should be rebaselined or if I've introduced some
bug?

[h.joshi, 2015-06-30 04:02:25]

@rune: checked new images for "ellipsis-platform-font-change.html" in 
linux/win/mac_blink_rel bots. 
Now correct images are generated and required to be rebaselined.

[rune, 2015-06-30 07:58:01]

On 2015/06/30 04:02:25, h.joshi wrote:
> @rune: checked new images for "ellipsis-platform-font-change.html" in 
> linux/win/mac_blink_rel bots. 
> Now correct images are generated and required to be rebaselined.

Thanks.

[rune, 2015-06-30 08:09:13]

rune@opera.com changed reviewers:
+ mstensho@opera.com

[mstensho (USE GERRIT), 2015-06-30 08:25:22]

lgtm

https://codereview.chromium.org/1209253006/diff/40001/Source/core/dom/Element...
File Source/core/dom/Element.cpp (right):

https://codereview.chromium.org/1209253006/diff/40001/Source/core/dom/Element...
Source/core/dom/Element.cpp:1551: if (*oldPseudoStyle != *newPseudoStyle ||
oldPseudoStyle->font().loadingCustomFonts() !=
newPseudoStyle->font().loadingCustomFonts()) {
So I assume that letting differing loadingCustomFonts() states count as style
difference would be bad?

https://codereview.chromium.org/1209253006/diff/40001/Source/core/layout/Layo...
File Source/core/layout/LayoutObject.h (right):

https://codereview.chromium.org/1209253006/diff/40001/Source/core/layout/Layo...
Source/core/layout/LayoutObject.h:739: void firstLineStyleDidChange(const
ComputedStyle& oldStyle, const ComputedStyle& newStyle);
Could you try to match the declaration point with the definition point?

In LayoutObject.cpp the definition of firstLineStyleDidChange() lies between
styleDidChange() and propagateStyleToAnonymousChildren(). Maybe move it right
after the definition of LayoutObject::setPseudoStyle() instead?

Then again, the ordering is already a mess, so feel free to ignore this issue.
:)

[rune, 2015-06-30 09:39:40]

https://codereview.chromium.org/1209253006/diff/40001/Source/core/dom/Element...
File Source/core/dom/Element.cpp (right):

https://codereview.chromium.org/1209253006/diff/40001/Source/core/dom/Element...
Source/core/dom/Element.cpp:1551: if (*oldPseudoStyle != *newPseudoStyle ||
oldPseudoStyle->font().loadingCustomFonts() !=
newPseudoStyle->font().loadingCustomFonts()) {
On 2015/06/30 08:25:22, mstensho wrote:
> So I assume that letting differing loadingCustomFonts() states count as style
> difference would be bad?

This was changed when introducing the baseLayoutStyle optimization. I discovered
that the loading state would change ComputedStyle equality under our feet. That
is, a computed style set with LayoutObject::setStyle would be mutable in the
sense of equality, and baseRenderStyle equality check would fail. See for
instance: https://codereview.chromium.org/754743004

https://codereview.chromium.org/1209253006/diff/40001/Source/core/layout/Layo...
File Source/core/layout/LayoutObject.h (right):

https://codereview.chromium.org/1209253006/diff/40001/Source/core/layout/Layo...
Source/core/layout/LayoutObject.h:739: void firstLineStyleDidChange(const
ComputedStyle& oldStyle, const ComputedStyle& newStyle);
On 2015/06/30 08:25:22, mstensho wrote:
> Could you try to match the declaration point with the definition point?
> 
> In LayoutObject.cpp the definition of firstLineStyleDidChange() lies between
> styleDidChange() and propagateStyleToAnonymousChildren(). Maybe move it right
> after the definition of LayoutObject::setPseudoStyle() instead?
> 
> Then again, the ordering is already a mess, so feel free to ignore this issue.
> :)

Done.

[rune, 2015-06-30 09:40:44]

The CQ bit was checked by rune@opera.com

[rune, 2015-06-30 09:40:45]

The patchset sent to the CQ was uploaded after l-g-t-m from mstensho@opera.com
Link to the patchset: https://codereview.chromium.org/1209253006/#ps60001
(title: "Review issue fix and virtual suite rebaseline.")

[commit-bot: I haz the power, 2015-06-30 09:41:11]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1209253006/60001

[commit-bot: I haz the power, 2015-06-30 10:47:38]

Committed patchset #4 (id:60001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=198048