Make follow the same pattern for equalSelections() in VisibleSelection.

This CL adds equalSelectionsAlgorithm() which is called in
equalSelections() for both composed tree and DOM tree.

BUG=none

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

[hyup, 2015-07-16 05:13:19]

sh53.lee@samsung.com changed reviewers:
+ yosin@google.com

[hyup, 2015-07-16 05:17:11]

PTAL

[yosin_UTC9, 2015-07-16 06:18:15]

yosin@chromium.org changed reviewers:
+ yosin@chromium.org
- yosin@google.com

[yosin_UTC9, 2015-07-16 07:24:20]

https://codereview.chromium.org/1234513004/diff/1/Source/core/editing/Visible...
File Source/core/editing/VisibleSelection.cpp (right):

https://codereview.chromium.org/1234513004/diff/1/Source/core/editing/Visible...
Source/core/editing/VisibleSelection.cpp:1154: return a.startInComposedTree() ==
b.startInComposedTree() && a.endInComposedTree() == b.endInComposedTree() &&
a.affinity() == b.affinity() && a.isBaseFirst() == b.isBaseFirst() &&
a.isDirectional() == b.isDirectional();
Could you get rid of duplicate conditions here too?

[hyup, 2015-07-16 09:41:02]

https://codereview.chromium.org/1234513004/diff/1/Source/core/editing/Visible...
File Source/core/editing/VisibleSelection.cpp (right):

https://codereview.chromium.org/1234513004/diff/1/Source/core/editing/Visible...
Source/core/editing/VisibleSelection.cpp:1154: return a.startInComposedTree() ==
b.startInComposedTree() && a.endInComposedTree() == b.endInComposedTree() &&
a.affinity() == b.affinity() && a.isBaseFirst() == b.isBaseFirst() &&
a.isDirectional() == b.isDirectional();
On 2015/07/16 07:24:20, Yosi_UTC9 wrote:
> Could you get rid of duplicate conditions here too?

I removed conditions about affinity and isDirectional in
VisibleSelection::InDOMTree::equalSelections because of 1143 line. But I'm not
sure in here. Could you explain which conditions should I remove here?

[yosin_UTC9, 2015-07-17 01:06:07]

https://codereview.chromium.org/1234513004/diff/1/Source/core/editing/Visible...
File Source/core/editing/VisibleSelection.cpp (right):

https://codereview.chromium.org/1234513004/diff/1/Source/core/editing/Visible...
Source/core/editing/VisibleSelection.cpp:1154: return a.startInComposedTree() ==
b.startInComposedTree() && a.endInComposedTree() == b.endInComposedTree() &&
a.affinity() == b.affinity() && a.isBaseFirst() == b.isBaseFirst() &&
a.isDirectional() == b.isDirectional();
On 2015/07/16 09:41:02, hyup wrote:
> On 2015/07/16 07:24:20, Yosi_UTC9 wrote:
> > Could you get rid of duplicate conditions here too?
> 
> I removed conditions about affinity and isDirectional in
> VisibleSelection::InDOMTree::equalSelections because of 1143 line. But I'm not
> sure in here. Could you explain which conditions should I remove here?

Oops, composed tree version isn't matched DOM tree version. This is bad.
We should follow the pattern using equalSelectionsAlgorithm.
So, equalSelections should be:

template <typename Strategy>
static bool equalSelectionsAlgorithm(const VisbileSelection& selection1, const
VisbileSelection& selection2)
{
    if (selection1.affinity() != selection2.affinity() ||
selection1.isDirectional() != selection2.isDirectional())
        return false;
    if (selection1.isNone())
        return selection2.isNone();
    return Strategy::selectionStart(selection1) ==
Strategy::selectionStart(selection2) && Strategy::selectionEnd(selection1) ==
Strategy::selectionEnd(selection2) && Strategy::selectionBase(selection1) ==
Strategy::selectionBase(selection2) && Strategy::selectionExtent(selection1) ==
Strategy::selectionExtent(selection2);
}

bool VisbileSelection::InDOMTree::equalSelections(const VisbileSelection&
selection1, const VisbileSelection& selection2)
{
    return equalSelectionsAlgorithm<InDOMTree>(selection1, selection2);
}

bool VisbileSelection::InComposedTree::equalSelections(const VisbileSelection&
selection1, const VisbileSelection& selection2)
{
    return equalSelectionsAlgorithm<InComposedTree>(selection1, selection2);
}

[hyup, 2015-07-19 16:24:01]

On 2015/07/17 01:06:07, Yosi_UTC9 wrote:
>
https://codereview.chromium.org/1234513004/diff/1/Source/core/editing/Visible...
> File Source/core/editing/VisibleSelection.cpp (right):
> 
>
https://codereview.chromium.org/1234513004/diff/1/Source/core/editing/Visible...
> Source/core/editing/VisibleSelection.cpp:1154: return a.startInComposedTree()
==
> b.startInComposedTree() && a.endInComposedTree() == b.endInComposedTree() &&
> a.affinity() == b.affinity() && a.isBaseFirst() == b.isBaseFirst() &&
> a.isDirectional() == b.isDirectional();
> On 2015/07/16 09:41:02, hyup wrote:
> > On 2015/07/16 07:24:20, Yosi_UTC9 wrote:
> > > Could you get rid of duplicate conditions here too?
> > 
> > I removed conditions about affinity and isDirectional in
> > VisibleSelection::InDOMTree::equalSelections because of 1143 line. But I'm
not
> > sure in here. Could you explain which conditions should I remove here?
> 
> Oops, composed tree version isn't matched DOM tree version. This is bad.
> We should follow the pattern using equalSelectionsAlgorithm.
> So, equalSelections should be:
> 
> template <typename Strategy>
> static bool equalSelectionsAlgorithm(const VisbileSelection& selection1, const
> VisbileSelection& selection2)
> {
>     if (selection1.affinity() != selection2.affinity() ||
> selection1.isDirectional() != selection2.isDirectional())
>         return false;
>     if (selection1.isNone())
>         return selection2.isNone();
>     return Strategy::selectionStart(selection1) ==
> Strategy::selectionStart(selection2) && Strategy::selectionEnd(selection1) ==
> Strategy::selectionEnd(selection2) && Strategy::selectionBase(selection1) ==
> Strategy::selectionBase(selection2) && Strategy::selectionExtent(selection1)
==
> Strategy::selectionExtent(selection2);
> }
> 
> bool VisbileSelection::InDOMTree::equalSelections(const VisbileSelection&
> selection1, const VisbileSelection& selection2)
> {
>     return equalSelectionsAlgorithm<InDOMTree>(selection1, selection2);
> }
> 
> bool VisbileSelection::InComposedTree::equalSelections(const VisbileSelection&
> selection1, const VisbileSelection& selection2)
> {
>     return equalSelectionsAlgorithm<InComposedTree>(selection1, selection2);
> }

Thank you for your review. I uploaded patch. PTAL

[yosin_UTC9, 2015-07-21 01:10:07]

lgtm

Thanks!

[yosin_UTC9, 2015-07-21 01:10:09]

The CQ bit was checked by yosin@chromium.org

[commit-bot: I haz the power, 2015-07-21 01:10:15]

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

[commit-bot: I haz the power, 2015-07-21 01:10:19]

The author sh53.lee@samsung.com has not signed Google Contributor License
Agreement. Please visit https://cla.developers.google.com to sign and manage
CLA.

[commit-bot: I haz the power, 2015-07-21 01:31:04]

The author sh53.lee@samsung.com has not signed Google Contributor License
Agreement. Please visit https://cla.developers.google.com to sign and manage
CLA.

[commit-bot: I haz the power, 2015-07-21 02:01:24]

The author sh53.lee@samsung.com has not signed Google Contributor License
Agreement. Please visit https://cla.developers.google.com to sign and manage
CLA.

[commit-bot: I haz the power, 2015-07-21 02:19:38]

Committed patchset #2 (id:20001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=199195