Refactoring: Remove StyledMarkupAccumulator::RangeFullySelectsNodes

Refactoring: Remove StyledMarkupAccumulator::RangeFullySelectsNodes

This CL aims to remove an enum StyledMarkupAccumulator::
RangeFullySelectsNodes, which is used like a boolean value.

This value is used to create an inline style for appendElement, and this
CL exposes a function to create an inline style (createInlineStyle) and
has StyledMarkupSerializer call this.

There was only one case when the value DoesNotFullySelectNode was used,
and this CL moves the logic for this case to the caller of
createInlineStyle.

I plan to remove another boolean parameter |addDisplayInline| later.

This patch is result of collaboration work with yosin@chromium.org
for selection of web components. We've already prepared the new
node-traversal usages at crrev.com/1134473004.

BUG=n/a
TEST=n/a; no behavior change

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

[hajimehoshi, 2015-06-08 10:32:22]

hajimehoshi@chromium.org changed reviewers:
+ tkent@chromium.org, yosin@chromium.org

[hajimehoshi, 2015-06-08 10:32:30]

PTAL

[yosin_UTC9, 2015-06-09 01:18:11]

It is better to do this in two patches:
1. Move StyledMarkupAccumulator::wrapWithNode() to StyledMarkupSerailizer() for
removing convertBlocksToInlines()
2. Remove RangeFullySelectsNodes

https://codereview.chromium.org/1163673005/diff/1/Source/core/editing/StyledM...
File Source/core/editing/StyledMarkupAccumulator.h (right):

https://codereview.chromium.org/1163673005/diff/1/Source/core/editing/StyledM...
Source/core/editing/StyledMarkupAccumulator.h:47:
StyledMarkupAccumulator(EAbsoluteURLs, const TextOffset& start, const
TextOffset& end, const PassRefPtrWillBeRawPtr<Document>,
EAnnotateForInterchange, Node*);
Do you also remove convertBlocksToInlines parameter? It seems description
doesn't mention it.

https://codereview.chromium.org/1163673005/diff/1/Source/core/editing/StyledM...
Source/core/editing/StyledMarkupAccumulator.h:55: void appendElement(Element&,
bool, PassRefPtrWillBeRawPtr<EditingStyle>);
Could you add parameter name for bool parameter?

https://codereview.chromium.org/1163673005/diff/1/Source/core/editing/StyledM...
Source/core/editing/StyledMarkupAccumulator.h:56: void
appendElement(StringBuilder&, Element&, bool,
PassRefPtrWillBeRawPtr<EditingStyle>);
Could you add parameter name for bool parameter?

https://codereview.chromium.org/1163673005/diff/1/Source/core/editing/StyledM...
Source/core/editing/StyledMarkupAccumulator.h:68:
RefPtrWillBeMember<EditingStyle> createInlineStyle(Element&, bool);
Could you add parameter name for second bool parameter?

https://codereview.chromium.org/1163673005/diff/1/Source/core/editing/StyledM...
File Source/core/editing/StyledMarkupSerializer.cpp (right):

https://codereview.chromium.org/1163673005/diff/1/Source/core/editing/StyledM...
Source/core/editing/StyledMarkupSerializer.cpp:310:
accumulator.appendText(toText(node));
I understand that StyledMarkupAccumlator still works only for DOM tree. So,
we'll change this part to check Strategy:parent(*node).

https://codereview.chromium.org/1163673005/diff/1/Source/core/editing/StyledM...
Source/core/editing/StyledMarkupSerializer.cpp:333: if (node.isElementNode())
nit: early return is better.

[hajimehoshi, 2015-06-10 10:36:08]

Patchset #5 (id:80001) has been deleted

[hajimehoshi, 2015-06-10 10:45:09]

Committed some parts of this (crrev.com/1177623005, crrev.com/1175913002) and
rebased. PTAL

https://codereview.chromium.org/1163673005/diff/1/Source/core/editing/StyledM...
File Source/core/editing/StyledMarkupAccumulator.h (right):

https://codereview.chromium.org/1163673005/diff/1/Source/core/editing/StyledM...
Source/core/editing/StyledMarkupAccumulator.h:55: void appendElement(Element&,
bool, PassRefPtrWillBeRawPtr<EditingStyle>);
On 2015/06/09 01:18:11, Yosi_UTC9 wrote:
> Could you add parameter name for bool parameter?

Done.

https://codereview.chromium.org/1163673005/diff/1/Source/core/editing/StyledM...
Source/core/editing/StyledMarkupAccumulator.h:56: void
appendElement(StringBuilder&, Element&, bool,
PassRefPtrWillBeRawPtr<EditingStyle>);
On 2015/06/09 01:18:11, Yosi_UTC9 wrote:
> Could you add parameter name for bool parameter?

Done.

https://codereview.chromium.org/1163673005/diff/1/Source/core/editing/StyledM...
Source/core/editing/StyledMarkupAccumulator.h:68:
RefPtrWillBeMember<EditingStyle> createInlineStyle(Element&, bool);
On 2015/06/09 01:18:11, Yosi_UTC9 wrote:
> Could you add parameter name for second bool parameter?

Done.

[yosin_UTC9, 2015-06-11 01:16:11]

https://codereview.chromium.org/1163673005/diff/120001/Source/core/editing/St...
File Source/core/editing/StyledMarkupAccumulator.cpp (right):

https://codereview.chromium.org/1163673005/diff/120001/Source/core/editing/St...
Source/core/editing/StyledMarkupAccumulator.cpp:138: void
StyledMarkupAccumulator::appendElement(Element& element, bool addDisplayInline,
PassRefPtrWillBeRawPtr<EditingStyle> style)
It seems we don't need to add |addDisplayInline| parameter here. There is only
one call sites of |appendElement()| having |false|.

[hajimehoshi, 2015-06-11 01:46:24]

https://codereview.chromium.org/1163673005/diff/120001/Source/core/editing/St...
File Source/core/editing/StyledMarkupAccumulator.cpp (right):

https://codereview.chromium.org/1163673005/diff/120001/Source/core/editing/St...
Source/core/editing/StyledMarkupAccumulator.cpp:138: void
StyledMarkupAccumulator::appendElement(Element& element, bool addDisplayInline,
PassRefPtrWillBeRawPtr<EditingStyle> style)
On 2015/06/11 01:16:11, Yosi_UTC9 wrote:
> It seems we don't need to add |addDisplayInline| parameter here. There is only
> one call sites of |appendElement()| having |false|.

Done.

[yosin_UTC9, 2015-06-11 01:58:35]

lgtm

https://codereview.chromium.org/1163673005/diff/140001/Source/core/editing/St...
File Source/core/editing/StyledMarkupSerializer.h (left):

https://codereview.chromium.org/1163673005/diff/140001/Source/core/editing/St...
Source/core/editing/StyledMarkupSerializer.h:54: 
no need to remove this blank line at this time.

[yosin_UTC9, 2015-06-11 01:58:41]

The CQ bit was checked by yosin@chromium.org

[commit-bot: I haz the power, 2015-06-11 01:58:57]

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

[commit-bot: I haz the power, 2015-06-11 02:46:10]

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

[commit-bot: I haz the power, 2015-06-11 02:46:11]

Try jobs failed on following builders:
  linux_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/6...)

[hajimehoshi, 2015-06-11 03:19:54]

The CQ bit was checked by hajimehoshi@chromium.org

[commit-bot: I haz the power, 2015-06-11 03:19:59]

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

[commit-bot: I haz the power, 2015-06-11 05:24:12]

Committed patchset #7 (id:140001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=196911