[Editing][CodeHealth] Extract new FrameCaret class from FrameSelection

[Editing][CodeHealth] Extract new FrameCaret class from FrameSelection

Create new FrameCaret to own caret painting in FrameSelection.
This is a kind of refactoring and many of them are in FrameSelection.cpp with
 renaming for ease of reading.
I will move them to FrameCaret.cpp later.

TEST=no change in behavior

Committed: https://crrev.com/5bd604e399dd7e7ceb54ad520d74cf829221de7e
Cr-Commit-Position: refs/heads/master@{#395290}

[yoichio, 2016-05-19 08:01:19]

Description was changed from

==========
refactor fracecaret
BUG=
==========

to

==========
[Editing][CodeHealth] Extract new FrameCaret class from FrameSelection

Create new FrameCaret to own caret painting in FrameSelection.
This is a kind of refactoring and many of them are in FrameSelection.cpp with
 renaming for ease of reading.
I will move them to FrameCaret.cpp later.

TEST=no change in behavior
==========

[yoichio, 2016-05-19 08:01:19]

yoichio@chromium.org changed reviewers:
+ yosin@chromium.org

[yosin_UTC9, 2016-05-19 08:32:37]

https://codereview.chromium.org/1994883002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/FrameCaret.cpp (right):

https://codereview.chromium.org/1994883002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/FrameCaret.cpp:1: // Copyright 2016 The
Chromium Authors. All rights reserved.
Copyright text should be taken from FrameSelection.cpp
Since these codes are moved with some changes from FrameSelection.cpp

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

https://codereview.chromium.org/1994883002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/FrameCaret.h:1: // Copyright 2016 The
Chromium Authors. All rights reserved.
Copyright text should be taken from FrameSelection.h
Since these codes are moved with some changes from FrameSelection.h

https://codereview.chromium.org/1994883002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/FrameCaret.h:16: virtual ~FrameCaret() {
}
Please put int .cpp as |FrameCaret::~FrameCaret() = defualt;|
http://dev.chromium.org/developers/coding-style/cpp-dos-and-donts#TOC-Stop-in...

https://codereview.chromium.org/1994883002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/FrameSelection.cpp (left):

https://codereview.chromium.org/1994883002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/FrameSelection.cpp:1081: if
(!m_caretBase->caretIsVisible() || !isCaret())
FrameCaret should have bool shouldBlinkCaret()

https://codereview.chromium.org/1994883002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/FrameSelection.cpp (right):

https://codereview.chromium.org/1994883002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/FrameSelection.cpp:400: void
FrameCaret::nodeWillBeRemoved(Node& node)
Please add TODO to move this function to "FrameCaret.cpp"

https://codereview.chromium.org/1994883002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/FrameSelection.cpp:402: if (node ==
m_previousCaretNode) {
Please use early return style.

https://codereview.chromium.org/1994883002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/FrameSelection.cpp:719: if (!isCaret()) {
All of functionality should be in FrameCaret::absoulteCaretBounds().
FrameCaret should have PositionWithAffnity and FrameSelection set in
setSelecitonAlgorithm.

https://codereview.chromium.org/1994883002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/FrameSelection.cpp:1090:
m_frameCaret->stopCaretBlinkTimer();
It is better to have FrameCaret::updateApperance(). FrameSelection doesn't want
to control caret blinking here.

https://codereview.chromium.org/1994883002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/FrameSelection.cpp:1104: if
(m_frameCaret->getCaretVisibility() == visibility)
Checking newValue == oldValue should be in FrameCaret.
So, FrameSelection::setCaretVisibility() should be just
m_frameCaret->setCaretVisibility(newVisibility)

[yoichio, 2016-05-20 05:12:13]

https://codereview.chromium.org/1994883002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/FrameCaret.cpp (right):

https://codereview.chromium.org/1994883002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/FrameCaret.cpp:1: // Copyright 2016 The
Chromium Authors. All rights reserved.
On 2016/05/19 08:32:36, Yosi_UTC9 wrote:
> Copyright text should be taken from FrameSelection.cpp
> Since these codes are moved with some changes from FrameSelection.cpp

Done.

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

https://codereview.chromium.org/1994883002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/FrameCaret.h:1: // Copyright 2016 The
Chromium Authors. All rights reserved.
On 2016/05/19 08:32:36, Yosi_UTC9 wrote:
> Copyright text should be taken from FrameSelection.h
> Since these codes are moved with some changes from FrameSelection.h

Done.

https://codereview.chromium.org/1994883002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/FrameCaret.h:16: virtual ~FrameCaret() {
}
On 2016/05/19 08:32:36, Yosi_UTC9 wrote:
> Please put int .cpp as |FrameCaret::~FrameCaret() = defualt;|
>
http://dev.chromium.org/developers/coding-style/cpp-dos-and-donts#TOC-Stop-in...

Done.

https://codereview.chromium.org/1994883002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/FrameSelection.cpp (left):

https://codereview.chromium.org/1994883002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/FrameSelection.cpp:1081: if
(!m_caretBase->caretIsVisible() || !isCaret())
On 2016/05/19 08:32:36, Yosi_UTC9 wrote:
> FrameCaret should have bool shouldBlinkCaret()

Ditto.

https://codereview.chromium.org/1994883002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/FrameSelection.cpp (right):

https://codereview.chromium.org/1994883002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/FrameSelection.cpp:400: void
FrameCaret::nodeWillBeRemoved(Node& node)
On 2016/05/19 08:32:36, Yosi_UTC9 wrote:
> Please add TODO to move this function to "FrameCaret.cpp"

Done.

https://codereview.chromium.org/1994883002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/FrameSelection.cpp:402: if (node ==
m_previousCaretNode) {
On 2016/05/19 08:32:36, Yosi_UTC9 wrote:
> Please use early return style.

Done.

https://codereview.chromium.org/1994883002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/FrameSelection.cpp:719: if (!isCaret()) {
On 2016/05/19 08:32:36, Yosi_UTC9 wrote:
> All of functionality should be in FrameCaret::absoulteCaretBounds().
> FrameCaret should have PositionWithAffnity and FrameSelection set in
> setSelecitonAlgorithm.

I agree, but this function use selection() deeply and of course FrameCaret
 should not have 'selection' object.
I guess change is not so primitive thus let me change this in another CL.

https://codereview.chromium.org/1994883002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/FrameSelection.cpp:1090:
m_frameCaret->stopCaretBlinkTimer();
On 2016/05/19 08:32:36, Yosi_UTC9 wrote:
> It is better to have FrameCaret::updateApperance(). FrameSelection doesn't
want
> to control caret blinking here.

Ditto to FrameSelection::absoluteCaretBounds change.
I will do this later.

https://codereview.chromium.org/1994883002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/FrameSelection.cpp:1104: if
(m_frameCaret->getCaretVisibility() == visibility)
On 2016/05/19 08:32:36, Yosi_UTC9 wrote:
> Checking newValue == oldValue should be in FrameCaret.
> So, FrameSelection::setCaretVisibility() should be just
> m_frameCaret->setCaretVisibility(newVisibility)

Ditto to appdateAppearance.

[yosin_UTC9, 2016-05-20 07:18:05]

https://codereview.chromium.org/1994883002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/CaretBase.h (right):

https://codereview.chromium.org/1994883002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/CaretBase.h:49: virtual ~CaretBase() { }
nit: virtual dtor should not be inline.
It should be CaretBase::~CaretBase() = default in .cpp file

https://codereview.chromium.org/1994883002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/CaretBase.h:72:
DEFINE_INLINE_VIRTUAL_TRACE() { }
nit: Should be in .cpp

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

https://codereview.chromium.org/1994883002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/FrameSelection.cpp:718:
DCHECK_NE(m_frame->document()->lifecycle().state(),
DocumentLifecycle::InPaintInvalidation);
Please add TODO for moving absoulteCaretBounds() to CaretBase when CaretBase has
m_position.

https://codereview.chromium.org/1994883002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/FrameSelection.cpp:1071: if
(!m_caretBlinkTimer.isActive()) {
nit: Please use early return style

[yoichio, 2016-05-20 07:50:00]

https://codereview.chromium.org/1994883002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/CaretBase.h (right):

https://codereview.chromium.org/1994883002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/CaretBase.h:49: virtual ~CaretBase() { }
On 2016/05/20 07:18:05, Yosi_UTC9 wrote:
> nit: virtual dtor should not be inline.
> It should be CaretBase::~CaretBase() = default in .cpp file

Done.

https://codereview.chromium.org/1994883002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/CaretBase.h:72:
DEFINE_INLINE_VIRTUAL_TRACE() { }
On 2016/05/20 07:18:05, Yosi_UTC9 wrote:
> nit: Should be in .cpp

Done.

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

https://codereview.chromium.org/1994883002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/FrameSelection.cpp:718:
DCHECK_NE(m_frame->document()->lifecycle().state(),
DocumentLifecycle::InPaintInvalidation);
On 2016/05/20 07:18:05, Yosi_UTC9 wrote:
> Please add TODO for moving absoulteCaretBounds() to CaretBase when CaretBase
has
> m_position.

Done.

https://codereview.chromium.org/1994883002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/FrameSelection.cpp:1071: if
(!m_caretBlinkTimer.isActive()) {
On 2016/05/20 07:18:05, Yosi_UTC9 wrote:
> nit: Please use early return style

Done.

[yosin_UTC9, 2016-05-20 07:53:12]

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

[yosin_UTC9, 2016-05-20 07:53:13]

lgtm w/ nits

+tken@ for core.gypi changes

https://codereview.chromium.org/1994883002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/core.gypi (right):

https://codereview.chromium.org/1994883002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/core.gypi:1471: 'editing/FrameCaret.cpp',
Please add "editing/FrameCaret.h" too.

https://codereview.chromium.org/1994883002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/FrameSelection.cpp (right):

https://codereview.chromium.org/1994883002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/FrameSelection.cpp:1416: if (!m_frame)
Since |m_frame| can't be null, we don't need to check.

[tkent, 2016-05-20 08:18:06]

lgtm

https://codereview.chromium.org/1994883002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/FrameCaret.h (right):

https://codereview.chromium.org/1994883002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/FrameCaret.h:34: class FrameCaret :
public CaretBase {
If you have no plan to make subclasses, please add 'final'.

https://codereview.chromium.org/1994883002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/FrameCaret.h:37: virtual ~FrameCaret();
Remove 'virtual', and add 'override'.

[yoichio, 2016-05-20 08:58:09]

https://codereview.chromium.org/1994883002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/core.gypi (right):

https://codereview.chromium.org/1994883002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/core.gypi:1471: 'editing/FrameCaret.cpp',
On 2016/05/20 07:53:13, Yosi_UTC9 wrote:
> Please add "editing/FrameCaret.h" too.

Done.

https://codereview.chromium.org/1994883002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/FrameCaret.h (right):

https://codereview.chromium.org/1994883002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/FrameCaret.h:34: class FrameCaret :
public CaretBase {
On 2016/05/20 08:18:06, tkent wrote:
> If you have no plan to make subclasses, please add 'final'.

Done.

https://codereview.chromium.org/1994883002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/FrameCaret.h:37: virtual ~FrameCaret();
On 2016/05/20 08:18:06, tkent wrote:
> Remove 'virtual', and add 'override'.

Done.

https://codereview.chromium.org/1994883002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/FrameSelection.cpp (right):

https://codereview.chromium.org/1994883002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/FrameSelection.cpp:1416: if (!m_frame)
On 2016/05/20 07:53:13, Yosi_UTC9 wrote:
> Since |m_frame| can't be null, we don't need to check.

Done.

[yoichio, 2016-05-20 08:58:13]

The CQ bit was checked by yoichio@chromium.org

[yoichio, 2016-05-20 08:58:13]

The patchset sent to the CQ was uploaded after l-g-t-m from tkent@chromium.org,
yosin@chromium.org
Link to the patchset: https://codereview.chromium.org/1994883002/#ps80001
(title: "update")

[commit-bot: I haz the power, 2016-05-20 08:58:23]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1994883002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1994883002/80001

[commit-bot: I haz the power, 2016-05-20 09:00:09]

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

[commit-bot: I haz the power, 2016-05-20 09:00:10]

Try jobs failed on following builders:
  ios-device on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-gn/bui...)
  ios-simulator on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-gn/...)

[yoichio, 2016-05-23 04:18:52]

The CQ bit was checked by yoichio@chromium.org

[yoichio, 2016-05-23 04:18:52]

The patchset sent to the CQ was uploaded after l-g-t-m from tkent@chromium.org,
yosin@chromium.org
Link to the patchset: https://codereview.chromium.org/1994883002/#ps100001
(title: "rebase")

[commit-bot: I haz the power, 2016-05-23 04:18:59]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1994883002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1994883002/100001

[commit-bot: I haz the power, 2016-05-23 06:47:37]

Description was changed from

==========
[Editing][CodeHealth] Extract new FrameCaret class from FrameSelection

Create new FrameCaret to own caret painting in FrameSelection.
This is a kind of refactoring and many of them are in FrameSelection.cpp with
 renaming for ease of reading.
I will move them to FrameCaret.cpp later.

TEST=no change in behavior
==========

to

==========
[Editing][CodeHealth] Extract new FrameCaret class from FrameSelection

Create new FrameCaret to own caret painting in FrameSelection.
This is a kind of refactoring and many of them are in FrameSelection.cpp with
 renaming for ease of reading.
I will move them to FrameCaret.cpp later.

TEST=no change in behavior
==========

[commit-bot: I haz the power, 2016-05-23 06:47:38]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-05-23 06:49:06]

Description was changed from

==========
[Editing][CodeHealth] Extract new FrameCaret class from FrameSelection

Create new FrameCaret to own caret painting in FrameSelection.
This is a kind of refactoring and many of them are in FrameSelection.cpp with
 renaming for ease of reading.
I will move them to FrameCaret.cpp later.

TEST=no change in behavior
==========

to

==========
[Editing][CodeHealth] Extract new FrameCaret class from FrameSelection

Create new FrameCaret to own caret painting in FrameSelection.
This is a kind of refactoring and many of them are in FrameSelection.cpp with
 renaming for ease of reading.
I will move them to FrameCaret.cpp later.

TEST=no change in behavior

Committed: https://crrev.com/5bd604e399dd7e7ceb54ad520d74cf829221de7e
Cr-Commit-Position: refs/heads/master@{#395290}
==========

[commit-bot: I haz the power, 2016-05-23 06:49:07]

Patchset 6 (id:??) landed as
https://crrev.com/5bd604e399dd7e7ceb54ad520d74cf829221de7e
Cr-Commit-Position: refs/heads/master@{#395290}