Add EditingMarkerList in preparation for DocumentMarkerController refactor

Add EditingMarkerList

Spliting off from https://codereview.chromium.org/2773883003 (add
CompositionMarkerList) since that CL is getting fairly large. This CL depends on
https://codereview.chromium.org/2773343003 (Add DocumentMarkerList)

The only new behavior here (vs. what's already in DocumentMarkerList) is that EditingMarkerList keeps track of if it's currently sorted or not based on what order markers were inserted, so I've added tests for that in EditingMarkerListTest.cpp.

BUG=707867

[rlanday, 2017-03-27 21:20:32]

The CQ bit was checked by rlanday@chromium.org to run a CQ dry run

[rlanday, 2017-03-27 21:20:54]

rlanday@chromium.org changed reviewers:
+ xiaochengh@chromium.org, yosin@chromium.org

[rlanday, 2017-03-27 21:20:55]

rlanday@chromium.org changed required reviewers:
+ yosin@chromium.org

[commit-bot: I haz the power, 2017-03-27 21:21:03]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[rlanday, 2017-03-27 21:27:11]

Description was changed from

==========
Add EditingMarkerList

Spliting off from https://codereview.chromium.org/2773883003 (add
CompositionMarkerList) since that CL is getting fairly large
==========

to

==========
Add EditingMarkerList

Spliting off from https://codereview.chromium.org/2773883003 (add
CompositionMarkerList) since that CL is getting fairly large. This CL depends on
https://codereview.chromium.org/2773343003 (Add DocumentMarkerList)

The only new behavior here (vs. what's already in DocumentMarkerList) is that
EditingMarkerList keeps track of if it's currently sorted or not based on what
order markers were inserted, so I've added tests for that in
EditingMarkerListTest.cpp.
==========

[Xiaocheng, 2017-03-27 22:46:22]

lgtm with nits.

https://codereview.chromium.org/2772423002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/markers/EditingMarkerList.h (right):

https://codereview.chromium.org/2772423002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/markers/EditingMarkerList.h:14: class
CORE_EXPORT EditingMarkerList : public DocumentMarkerList {
It's better to also document its behavior in the code, not just in the patch
description.

https://codereview.chromium.org/2772423002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/markers/EditingMarkerList.h:19: bool
isEditingMarkerList() const final;
Please move all overridden functions to private.

[rlanday, 2017-03-28 01:05:07]

https://codereview.chromium.org/2772423002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/markers/EditingMarkerList.h (right):

https://codereview.chromium.org/2772423002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/markers/EditingMarkerList.h:19: bool
isEditingMarkerList() const final;
On 2017/03/27 at 22:46:22, Xiaocheng wrote:
> Please move all overridden functions to private.

As we discussed offline, I'm going to leave these as-is since we sometimes need
to access DocumentMarkerList through a pointer to a derived class (to call
methods that only exist on the derived class)

[rlanday, 2017-03-28 01:12:02]

The CQ bit was checked by rlanday@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-28 01:12:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-28 04:28:28]

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

[commit-bot: I haz the power, 2017-03-28 04:28:29]

Dry run: This issue passed the CQ dry run.

[rlanday, 2017-03-29 01:04:13]

The CQ bit was checked by rlanday@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-29 01:06:11]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[yosin_UTC9, 2017-03-29 01:55:57]

On 2017/03/28 at 01:05:07, rlanday wrote:
>
https://codereview.chromium.org/2772423002/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/editing/markers/EditingMarkerList.h
(right):
> 
>
https://codereview.chromium.org/2772423002/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/editing/markers/EditingMarkerList.h:19: bool
isEditingMarkerList() const final;
> On 2017/03/27 at 22:46:22, Xiaocheng wrote:
> > Please move all overridden functions to private.
> 
> As we discussed offline, I'm going to leave these as-is since we sometimes
need to access DocumentMarkerList through a pointer to a derived class (to call
methods that only exist on the derived class)

(I discussed tkent@ about this topic)

Google C++ code style doesn't mention about private-vs-public about
implementation of interface.
Then, the rule in inferred from existing code.

Please move interface implementation member functions to private section as
existing code.
We want to align existing code not to make reader feel strange, e.g. reviewers
say "please move them private section".

For me, I switched to implementation-in-private since working from Chromium, I
was implementation-in-public before Chromium.

Please ask chromium-dev@/blink-dev@ about this to find out why chromium/blink
use implementation-in-private.
Thanks!

[commit-bot: I haz the power, 2017-03-29 02:58:59]

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

[commit-bot: I haz the power, 2017-03-29 02:59:00]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[yosin_UTC9, 2017-03-29 03:14:52]

On 2017/03/29 at 01:55:57, yosin_UTC9 wrote:
> On 2017/03/28 at 01:05:07, rlanday wrote:
> >
https://codereview.chromium.org/2772423002/diff/20001/third_party/WebKit/Sour...
> > File third_party/WebKit/Source/core/editing/markers/EditingMarkerList.h
(right):
> > 
> >
https://codereview.chromium.org/2772423002/diff/20001/third_party/WebKit/Sour...
> > third_party/WebKit/Source/core/editing/markers/EditingMarkerList.h:19: bool
isEditingMarkerList() const final;
> > On 2017/03/27 at 22:46:22, Xiaocheng wrote:
> > > Please move all overridden functions to private.
> > 
> > As we discussed offline, I'm going to leave these as-is since we sometimes
need to access DocumentMarkerList through a pointer to a derived class (to call
methods that only exist on the derived class)
> 
> (I discussed tkent@ about this topic)
> 
> Google C++ code style doesn't mention about private-vs-public about
implementation of interface.
> Then, the rule in inferred from existing code.
> 
> Please move interface implementation member functions to private section as
existing code.
> We want to align existing code not to make reader feel strange, e.g. reviewers
say "please move them private section".
> 
> For me, I switched to implementation-in-private since working from Chromium, I
was implementation-in-public before Chromium.
> 
> Please ask chromium-dev@/blink-dev@ about this to find out why chromium/blink
use implementation-in-private.
> Thanks!

I discussed with xiaochengh@ offline. I agree to go as-is.

Let's wait chromium-dev@ discussion how people thinks.

[1]
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/XdsXTHy9lis
Overriding public virtual methods as private in C++?

[yosin_UTC9, 2017-03-29 07:43:15]

lgtm

Since reply [1] suggest impl-in-private or not is case-by-case.




On 2017/03/29 at 03:14:52, yosin_UTC9 wrote:
> On 2017/03/29 at 01:55:57, yosin_UTC9 wrote:
> > On 2017/03/28 at 01:05:07, rlanday wrote:
> > >
https://codereview.chromium.org/2772423002/diff/20001/third_party/WebKit/Sour...
> > > File third_party/WebKit/Source/core/editing/markers/EditingMarkerList.h
(right):
> > > 
> > >
https://codereview.chromium.org/2772423002/diff/20001/third_party/WebKit/Sour...
> > > third_party/WebKit/Source/core/editing/markers/EditingMarkerList.h:19:
bool isEditingMarkerList() const final;
> > > On 2017/03/27 at 22:46:22, Xiaocheng wrote:
> > > > Please move all overridden functions to private.
> > > 
> > > As we discussed offline, I'm going to leave these as-is since we sometimes
need to access DocumentMarkerList through a pointer to a derived class (to call
methods that only exist on the derived class)
> > 
> > (I discussed tkent@ about this topic)
> > 
> > Google C++ code style doesn't mention about private-vs-public about
implementation of interface.
> > Then, the rule in inferred from existing code.
> > 
> > Please move interface implementation member functions to private section as
existing code.
> > We want to align existing code not to make reader feel strange, e.g.
reviewers say "please move them private section".
> > 
> > For me, I switched to implementation-in-private since working from Chromium,
I was implementation-in-public before Chromium.
> > 
> > Please ask chromium-dev@/blink-dev@ about this to find out why
chromium/blink use implementation-in-private.
> > Thanks!
> 
> I discussed with xiaochengh@ offline. I agree to go as-is.
> 
> Let's wait chromium-dev@ discussion how people thinks.
> 
> [1]
https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/XdsXTHy9lis
Overriding public virtual methods as private in C++?

[rlanday, 2017-03-30 00:29:54]

The CQ bit was checked by rlanday@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-30 00:31:09]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-30 02:36:22]

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

[commit-bot: I haz the power, 2017-03-30 02:36:23]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_TIMED_OUT, build hasn't
started yet, builder probably lacks capacity)

[rlanday, 2017-03-30 03:03:19]

The CQ bit was checked by rlanday@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-30 03:04:14]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[rlanday, 2017-03-30 03:15:04]

The CQ bit was checked by rlanday@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-30 03:15:48]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-30 05:03:10]

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

[commit-bot: I haz the power, 2017-03-30 05:03:11]

Dry run: This issue passed the CQ dry run.

[rlanday, 2017-03-30 22:55:51]

The CQ bit was checked by rlanday@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-30 22:56:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-31 01:16:45]

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

[commit-bot: I haz the power, 2017-03-31 01:16:46]

Dry run: This issue passed the CQ dry run.

[rlanday, 2017-03-31 17:30:12]

The CQ bit was checked by rlanday@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-31 17:30:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-31 19:11:51]

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

[commit-bot: I haz the power, 2017-03-31 19:11:53]

Dry run: This issue passed the CQ dry run.

[rlanday, 2017-04-03 18:55:40]

Description was changed from

==========
Add EditingMarkerList

Spliting off from https://codereview.chromium.org/2773883003 (add
CompositionMarkerList) since that CL is getting fairly large. This CL depends on
https://codereview.chromium.org/2773343003 (Add DocumentMarkerList)

The only new behavior here (vs. what's already in DocumentMarkerList) is that
EditingMarkerList keeps track of if it's currently sorted or not based on what
order markers were inserted, so I've added tests for that in
EditingMarkerListTest.cpp.
==========

to

==========
Add EditingMarkerList

Spliting off from https://codereview.chromium.org/2773883003 (add
CompositionMarkerList) since that CL is getting fairly large. This CL depends on
https://codereview.chromium.org/2773343003 (Add DocumentMarkerList)

The only new behavior here (vs. what's already in DocumentMarkerList) is that
EditingMarkerList keeps track of if it's currently sorted or not based on what
order markers were inserted, so I've added tests for that in
EditingMarkerListTest.cpp.

BUG=707867
==========