Move CompositionUnderlineVectorBuilder.* from web/ -> core/editing/.

Move CompositionUnderlineVectorBuilder.* from web/ -> core/editing/.

Move CompositionUnderlineVectorBuilder.h to core/editing/ as part of the 
removal of the web/ layer. 

Now that core has visibility into public/web/ we can remove 
CompositionUnderlineBuilder entirely and just provide a copy constructor on 
CompositionUnderling to do the conversion from WebCompositionUnderline.


BUG=708879
Review-Url: https://codereview.chromium.org/2826373002
Cr-Commit-Position: refs/heads/master@{#467620}
Committed: https://chromium.googlesource.com/chromium/src/+/8db797d21d544547fdf4436072224c01691e938c

[slangley, 2017-04-20 00:51:12]

Description was changed from

==========
git cl format


Move CompositionUnderlineVectorBuilder.h to core/editing/.


Remove CompositionUnderlineBuilder.

BUG=708879
==========

to

==========
Move CompositionUnderlineVectorBuilder.* from web/ -> core/editing/.

Move CompositionUnderlineVectorBuilder.h to core/editing/ as part of the 
removal of the web/ layer. 

Now that core has visibility into public/web/ we can remove 
CompositionUnderlineBuilder entirely and just provide a copy constructor on 
CompositionUnderling to do the conversion from WebCompositionUnderline.


BUG=708879
==========

[slangley, 2017-04-20 00:51:12]

slangley@chromium.org changed reviewers:
+ joelhockey@chromium.org

[joelhockey, 2017-04-20 01:11:43]

lgtm

[slangley, 2017-04-20 01:16:56]

slangley@chromium.org changed reviewers:
+ haraken@chromium.org

[slangley, 2017-04-20 01:16:57]

haraken@ - ptal

[haraken, 2017-04-20 07:25:50]

LGTM

(FWIW I'm in a EMEA timezone this week. Sorry for the review delay.)

https://codereview.chromium.org/2826373002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/editing/CompositionUnderline.cpp (right):

https://codereview.chromium.org/2826373002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/CompositionUnderline.cpp:25:
CompositionUnderline::CompositionUnderline(const WebCompositionUnderline& u)

Nit: u => underline

Blink prefers a fully qualified name.

https://codereview.chromium.org/2826373002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/editing/CompositionUnderlineVectorBuilder.h
(right):

https://codereview.chromium.org/2826373002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/CompositionUnderlineVectorBuilder.h:42:
// This classes are used for converting from WebVector<WebCompositionUnderline>

This class is...

(Nit: You could consider putting CompositionUnderlineBuilder and
CompositionUnderlineVectorBuilder in the same file.)

[slangley, 2017-04-21 00:07:15]

Thanks!

https://codereview.chromium.org/2826373002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/editing/CompositionUnderline.cpp (right):

https://codereview.chromium.org/2826373002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/CompositionUnderline.cpp:25:
CompositionUnderline::CompositionUnderline(const WebCompositionUnderline& u)
On 2017/04/20 at 07:25:50, haraken wrote:
> Nit: u => underline
> 
> Blink prefers a fully qualified name.

Done

https://codereview.chromium.org/2826373002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/editing/CompositionUnderlineVectorBuilder.h
(right):

https://codereview.chromium.org/2826373002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/CompositionUnderlineVectorBuilder.h:42:
// This classes are used for converting from WebVector<WebCompositionUnderline>
On 2017/04/20 at 07:25:50, haraken wrote:
> This class is...
> 
> (Nit: You could consider putting CompositionUnderlineBuilder and
CompositionUnderlineVectorBuilder in the same file.)

Done.

TBH I'm from the one class -> one file school - is there any general guidance in
blink?

[slangley, 2017-04-21 00:09:34]

The CQ bit was checked by slangley@chromium.org

[slangley, 2017-04-21 00:09:34]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, joelhockey@chromium.org
Link to the patchset: https://codereview.chromium.org/2826373002/#ps20001
(title: "Address code review comments.")

[commit-bot: I haz the power, 2017-04-21 00:09:55]

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-04-21 01:04:39]

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

[commit-bot: I haz the power, 2017-04-21 01:04:40]

Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[slangley, 2017-04-21 03:54:14]

The CQ bit was checked by slangley@chromium.org

[commit-bot: I haz the power, 2017-04-21 03:54:32]

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-04-21 05:32:14]

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

[commit-bot: I haz the power, 2017-04-21 05:32:15]

Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[haraken, 2017-04-21 06:18:54]

https://codereview.chromium.org/2826373002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/editing/CompositionUnderlineVectorBuilder.h
(right):

https://codereview.chromium.org/2826373002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/CompositionUnderlineVectorBuilder.h:42:
// This classes are used for converting from WebVector<WebCompositionUnderline>
On 2017/04/21 00:07:15, slangley wrote:
> On 2017/04/20 at 07:25:50, haraken wrote:
> > This class is...
> > 
> > (Nit: You could consider putting CompositionUnderlineBuilder and
> CompositionUnderlineVectorBuilder in the same file.)
> 
> Done.
> 
> TBH I'm from the one class -> one file school - is there any general guidance
in
> blink?

I don't know. We normally create one file per class but put closely-related
classes into the same file.

[slangley, 2017-04-27 00:48:00]

haraken@ - ptal

The original LGTM'd patch would not compile on windows, as it wanted
CompisitionUnderline to have a default ctor(). So I've changed
CompositionUnderlineVectorBuilder to be static and have a Build method rather
then derive from WTF::Vector. Logically there should be no difference.

https://codereview.chromium.org/2826373002/diff/40001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/core/editing/CompositionUnderlineVectorBuilder.cpp
(right):

https://codereview.chromium.org/2826373002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/CompositionUnderlineVectorBuilder.cpp:37:
Vector<CompositionUnderline> result;
Note: Can't call ctor(size) here as no default ctor for CompositionUnderline.

[haraken, 2017-04-27 01:02:00]

LGTM

[slangley, 2017-04-27 01:05:29]

The CQ bit was checked by slangley@chromium.org

[slangley, 2017-04-27 01:05:30]

The patchset sent to the CQ was uploaded after l-g-t-m from
joelhockey@chromium.org
Link to the patchset: https://codereview.chromium.org/2826373002/#ps40001
(title: "Change CompositionUnderlineVectorBuilder to have an explicit method to
build a Vector, rather than …")

[commit-bot: I haz the power, 2017-04-27 01:06:23]

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-04-27 03:01:10]

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

[commit-bot: I haz the power, 2017-04-27 03:01:11]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[slangley, 2017-04-27 06:55:57]

The CQ bit was checked by slangley@chromium.org

[commit-bot: I haz the power, 2017-04-27 06:56:11]

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-04-27 07:44:07]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1493276157192540,
"parent_rev": "c613e1f102863081348db126d8db74fc11e389fb", "commit_rev":
"8db797d21d544547fdf4436072224c01691e938c"}

[commit-bot: I haz the power, 2017-04-27 07:44:17]

Description was changed from

==========
Move CompositionUnderlineVectorBuilder.* from web/ -> core/editing/.

Move CompositionUnderlineVectorBuilder.h to core/editing/ as part of the 
removal of the web/ layer. 

Now that core has visibility into public/web/ we can remove 
CompositionUnderlineBuilder entirely and just provide a copy constructor on 
CompositionUnderling to do the conversion from WebCompositionUnderline.


BUG=708879
==========

to

==========
Move CompositionUnderlineVectorBuilder.* from web/ -> core/editing/.

Move CompositionUnderlineVectorBuilder.h to core/editing/ as part of the 
removal of the web/ layer. 

Now that core has visibility into public/web/ we can remove 
CompositionUnderlineBuilder entirely and just provide a copy constructor on 
CompositionUnderling to do the conversion from WebCompositionUnderline.


BUG=708879

Review-Url: https://codereview.chromium.org/2826373002
Cr-Commit-Position: refs/heads/master@{#467620}
Committed:
https://chromium.googlesource.com/chromium/src/+/8db797d21d544547fdf443607222...
==========

[commit-bot: I haz the power, 2017-04-27 07:44:18]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/8db797d21d544547fdf443607222...