Use RenderStyle::isCollapsibleWhiteSpace when renderer is available.

Use RenderStyle::isCollapsibleWhiteSpace when renderer is available.

This patch replaces isCollapsibleWhitespace() with RenderStyle::isCollapsibleWhiteSpace
in places where renderer is available. The latter provides more accurate decision so we
should always use it when possible. Some test results require rebaseline but none of
them failed the test really and the new behavior is correct.

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

[pdr., 2014-03-31 18:23:39]

+cc esprehn.

[c.shu, 2014-04-01 00:20:00]

Would you review the patch? thanks.

[esprehn, 2014-04-01 00:25:54]

https://codereview.chromium.org/214523008/diff/20001/Source/core/dom/Position...
File Source/core/dom/Position.cpp (right):

https://codereview.chromium.org/214523008/diff/20001/Source/core/dom/Position...
Source/core/dom/Position.cpp:1066: if (considerNonCollapsibleWhitespace ?
(isSpaceOrNewline(c) || c == noBreakSpace) :
prev.deprecatedNode()->renderer()->style()->isCollapsibleWhiteSpace(c))
How do you know the renderer() is available here?

https://codereview.chromium.org/214523008/diff/20001/Source/core/dom/Position...
Source/core/dom/Position.cpp:1085: if (considerNonCollapsibleWhitespace ?
(isSpaceOrNewline(c) || c == noBreakSpace) :
deprecatedNode()->renderer()->style()->isCollapsibleWhiteSpace(c))
ditto

[c.shu, 2014-04-01 13:36:57]

On 2014/04/01 00:25:54, esprehn wrote:
>
https://codereview.chromium.org/214523008/diff/20001/Source/core/dom/Position...
> File Source/core/dom/Position.cpp (right):
> 
>
https://codereview.chromium.org/214523008/diff/20001/Source/core/dom/Position...
> Source/core/dom/Position.cpp:1066: if (considerNonCollapsibleWhitespace ?
> (isSpaceOrNewline(c) || c == noBreakSpace) :
> prev.deprecatedNode()->renderer()->style()->isCollapsibleWhiteSpace(c))
> How do you know the renderer() is available here?
> 
>
https://codereview.chromium.org/214523008/diff/20001/Source/core/dom/Position...
> Source/core/dom/Position.cpp:1085: if (considerNonCollapsibleWhitespace ?
> (isSpaceOrNewline(c) || c == noBreakSpace) :
> deprecatedNode()->renderer()->style()->isCollapsibleWhiteSpace(c))
> ditto

I agree we won't be able to find hard proof that renderer has been created at
this moment. I was trying to see if it caused any crash by running layout tests
and it seemed Ok. From the trybot results, the win_blink_rel failed due to
timeout, but I am able to see the crash log. I will take this part out of the
patch to make the patch safe.

[eseidel, 2014-04-02 20:02:23]

Can you please explain the test cahnges?  I don't understand them from first
look.

[c.shu, 2014-04-02 20:12:28]

On 2014/04/02 20:02:23, eseidel wrote:
> Can you please explain the test cahnges?  I don't understand them from first
> look.

My patch actually changed the behavior. The replaced function
isCollapisbleWhitespace only checks if a char is ' ' or '\n' but the new one
will make the decision based on renderStyle. Thus, in certain cases, e.g., ' '
may not be collapsible and '\t' could be collapsible. As a result, the expected
results of some layout tests have been changed. However, I looked the intention
of these tests and they are all "passing". It's just the expected texts didn't
match and as I have explained, the new results are the correct one. So it's a
fix rather than regression.

[eseidel, 2014-04-02 20:13:23]

So all the tests colapse exactly one extra space?  Why don't they collapse more?

[c.shu, 2014-04-02 20:38:15]

On 2014/04/02 20:13:23, eseidel wrote:
> So all the tests colapse exactly one extra space?  Why don't they collapse
more?

In these cases, the existing code makes the spaces collapse into a tab and a
space and with the new code, they collapse into a tab. They were not multiple
spaces. I checked by opening the files by xxd.

[c.shu, 2014-04-02 20:43:00]

On 2014/04/02 20:38:15, c.shu wrote:
> On 2014/04/02 20:13:23, eseidel wrote:
> > So all the tests colapse exactly one extra space?  Why don't they collapse
> more?
> 
> In these cases, the existing code makes the spaces collapse into a tab and a
> space and with the new code, they collapse into a tab. They were not multiple
> spaces. I checked by opening the files by xxd.

Btw, the places I touched are not where texts are rendered but where rendered
texts are converted to text strings. This is why the expected results from
'dumpAsText' are affected. The real text rendering on webpages is correct before
and after the patch.

[eseidel, 2014-04-02 20:46:01]

lgtm

[commit-bot: I haz the power, 2014-04-02 20:46:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/c.shu@samsung.com/214523008/40001

[commit-bot: I haz the power, 2014-04-02 21:54:56]

Change committed as 170706