Editing: Fix caret blinking after a typing.

Editing: Fix caret blinking after a typing.

FrameCaret::updateAppearance() is called in updateStyleAndLayoutIfNeeded(). So
if shouldStopBlinkingDueToTypingCommand() is true, we repeatedly stop and start
the caret timer.

Because FrameSelection::didSetSelectionDeprecated() calls stopCaretBlinkTimer(),
we always restart the caret timer after every selection change. Checking
shouldStopBlinkingDueToTypingCommand() is unnecessary.

The core part of the CL is just to remove
shouldStopBlinkingDueToTypingCommand(), however this CL adds a lot of code to
make the code testable.
- Add FrameCaret::recreateCaretBlinkTimerForTesting(RefPtr<WebTaskRunner>)
- Add FrameSelection::frameCaretForTesting()
- Add support of delayed task to FakeWebTaskRunner

BUG=692900
Review-Url: https://codereview.chromium.org/2723003002
Cr-Commit-Position: refs/heads/master@{#453858}
Committed: https://chromium.googlesource.com/chromium/src/+/045a3dd143fabf6787774090a255dc0eda649c59

[tkent, 2017-03-01 02:05:29]

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

[tkent, 2017-03-01 02:05:58]

Description was changed from

==========
Editing: Do not restart a caret timer on every updateStyleAndLayoutIfNeeded.

FrameCaret::updateAppearance() is called in updateStyleAndLayoutIfNeeded(). So
if shouldStopBlinkingDueToTypingCommand() is true, we repeatedly stop and start
the caret timer.

Because FrameSelection::didSetSelectionDeprecated() calls stopCaretBlinkTimer(),
we always restart the caret timer after every selection change. checking
shouldStopBlinkingDueToTypingCommand() is unnecessary.

The core part of the CL is just to remove
shouldStopBlinkingDueToTypingCommand(), however this CL adds a lot of code to
make the code testable.
- Add FrameCaret::recreateCaretBlinkTimerForTesting(RefPtr<WebTaskRunner>)
- Add support of delayed task to FakeWebTaskRunner

BUG=692900
==========

to

==========
Editing: Do not restart a caret timer on every updateStyleAndLayoutIfNeeded.

FrameCaret::updateAppearance() is called in updateStyleAndLayoutIfNeeded(). So
if shouldStopBlinkingDueToTypingCommand() is true, we repeatedly stop and start
the caret timer.

Because FrameSelection::didSetSelectionDeprecated() calls stopCaretBlinkTimer(),
we always restart the caret timer after every selection change. checking
shouldStopBlinkingDueToTypingCommand() is unnecessary.

The core part of the CL is just to remove
shouldStopBlinkingDueToTypingCommand(), however this CL adds a lot of code to
make the code testable.
- Add FrameCaret::recreateCaretBlinkTimerForTesting(RefPtr<WebTaskRunner>)
- Add FrameSelection::frameCaretForTesting()
- Add support of delayed task to FakeWebTaskRunner

BUG=692900
==========

[commit-bot: I haz the power, 2017-03-01 02:06:17]

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

[tkent, 2017-03-01 02:07:07]

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

[tkent, 2017-03-01 02:07:08]

yosin@, would you review this please?

[tkent, 2017-03-01 02:07:46]

Description was changed from

==========
Editing: Do not restart a caret timer on every updateStyleAndLayoutIfNeeded.

FrameCaret::updateAppearance() is called in updateStyleAndLayoutIfNeeded(). So
if shouldStopBlinkingDueToTypingCommand() is true, we repeatedly stop and start
the caret timer.

Because FrameSelection::didSetSelectionDeprecated() calls stopCaretBlinkTimer(),
we always restart the caret timer after every selection change. checking
shouldStopBlinkingDueToTypingCommand() is unnecessary.

The core part of the CL is just to remove
shouldStopBlinkingDueToTypingCommand(), however this CL adds a lot of code to
make the code testable.
- Add FrameCaret::recreateCaretBlinkTimerForTesting(RefPtr<WebTaskRunner>)
- Add FrameSelection::frameCaretForTesting()
- Add support of delayed task to FakeWebTaskRunner

BUG=692900
==========

to

==========
Editing: Do not restart a caret timer on every updateStyleAndLayoutIfNeeded.

FrameCaret::updateAppearance() is called in updateStyleAndLayoutIfNeeded(). So
if shouldStopBlinkingDueToTypingCommand() is true, we repeatedly stop and start
the caret timer.

Because FrameSelection::didSetSelectionDeprecated() calls stopCaretBlinkTimer(),
we always restart the caret timer after every selection change. Checking
shouldStopBlinkingDueToTypingCommand() is unnecessary.

The core part of the CL is just to remove
shouldStopBlinkingDueToTypingCommand(), however this CL adds a lot of code to
make the code testable.
- Add FrameCaret::recreateCaretBlinkTimerForTesting(RefPtr<WebTaskRunner>)
- Add FrameSelection::frameCaretForTesting()
- Add support of delayed task to FakeWebTaskRunner

BUG=692900
==========

[tkent, 2017-03-01 02:58:33]

Description was changed from

==========
Editing: Do not restart a caret timer on every updateStyleAndLayoutIfNeeded.

FrameCaret::updateAppearance() is called in updateStyleAndLayoutIfNeeded(). So
if shouldStopBlinkingDueToTypingCommand() is true, we repeatedly stop and start
the caret timer.

Because FrameSelection::didSetSelectionDeprecated() calls stopCaretBlinkTimer(),
we always restart the caret timer after every selection change. Checking
shouldStopBlinkingDueToTypingCommand() is unnecessary.

The core part of the CL is just to remove
shouldStopBlinkingDueToTypingCommand(), however this CL adds a lot of code to
make the code testable.
- Add FrameCaret::recreateCaretBlinkTimerForTesting(RefPtr<WebTaskRunner>)
- Add FrameSelection::frameCaretForTesting()
- Add support of delayed task to FakeWebTaskRunner

BUG=692900
==========

to

==========
Editing: Fix caret blinking after a typing.

FrameCaret::updateAppearance() is called in updateStyleAndLayoutIfNeeded(). So
if shouldStopBlinkingDueToTypingCommand() is true, we repeatedly stop and start
the caret timer.

Because FrameSelection::didSetSelectionDeprecated() calls stopCaretBlinkTimer(),
we always restart the caret timer after every selection change. Checking
shouldStopBlinkingDueToTypingCommand() is unnecessary.

The core part of the CL is just to remove
shouldStopBlinkingDueToTypingCommand(), however this CL adds a lot of code to
make the code testable.
- Add FrameCaret::recreateCaretBlinkTimerForTesting(RefPtr<WebTaskRunner>)
- Add FrameSelection::frameCaretForTesting()
- Add support of delayed task to FakeWebTaskRunner

BUG=692900
==========

[yosin_UTC9, 2017-03-01 03:21:07]

lgtm w/ *optional* style nits

Thanks for introducing testable timer which I could not!
m(_ _)m

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

https://codereview.chromium.org/2723003002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/FrameCaret.cpp:88: if (!shouldBlink) {
nit: I prefer early-return style to reduce indentation.

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

https://codereview.chromium.org/2723003002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/FrameCaretTest.cpp:33: bool
m_wasRunningLayoutTest;
nit: Use |const bool| and initialized in ctor to avoid uninitialized member
variable?

https://codereview.chromium.org/2723003002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/scheduler/test/fake_web_task_runner.cc
(right):

https://codereview.chromium.org/2723003002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/test/fake_web_task_runner.cc:117:
for (auto i = data_->FindRunnableTask(); i != data_->task_queue_.end();
nit: It is better to use "it" or something to avoid one character variable.

[tkent, 2017-03-01 04:08:29]

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

[commit-bot: I haz the power, 2017-03-01 04:08:42]

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

[tkent, 2017-03-01 05:50:41]

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

https://codereview.chromium.org/2723003002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/FrameCaret.cpp:88: if (!shouldBlink) {
On 2017/03/01 at 03:21:07, yosin_UTC9 wrote:
> nit: I prefer early-return style to reduce indentation.

Done.

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

https://codereview.chromium.org/2723003002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/FrameCaretTest.cpp:33: bool
m_wasRunningLayoutTest;
On 2017/03/01 at 03:21:07, yosin_UTC9 wrote:
> nit: Use |const bool| and initialized in ctor to avoid uninitialized member
variable?

Done.

https://codereview.chromium.org/2723003002/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/scheduler/test/fake_web_task_runner.cc
(right):

https://codereview.chromium.org/2723003002/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/test/fake_web_task_runner.cc:117:
for (auto i = data_->FindRunnableTask(); i != data_->task_queue_.end();
On 2017/03/01 at 03:21:07, yosin_UTC9 wrote:
> nit: It is better to use "it" or something to avoid one character variable.

Done.

[tkent, 2017-03-01 05:50:46]

The CQ bit was unchecked by tkent@chromium.org

[tkent, 2017-03-01 05:50:48]

The CQ bit was checked by tkent@chromium.org

[tkent, 2017-03-01 05:50:49]

The patchset sent to the CQ was uploaded after l-g-t-m from yosin@chromium.org
Link to the patchset: https://codereview.chromium.org/2723003002/#ps20001
(title: "Apply review comments")

[commit-bot: I haz the power, 2017-03-01 05:50:58]

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-01 05:55:04]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1488347448662450,
"parent_rev": "824475890477cf97f9fef981993d628cf3d4a8cb", "commit_rev":
"045a3dd143fabf6787774090a255dc0eda649c59"}

[commit-bot: I haz the power, 2017-03-01 05:55:43]

Description was changed from

==========
Editing: Fix caret blinking after a typing.

FrameCaret::updateAppearance() is called in updateStyleAndLayoutIfNeeded(). So
if shouldStopBlinkingDueToTypingCommand() is true, we repeatedly stop and start
the caret timer.

Because FrameSelection::didSetSelectionDeprecated() calls stopCaretBlinkTimer(),
we always restart the caret timer after every selection change. Checking
shouldStopBlinkingDueToTypingCommand() is unnecessary.

The core part of the CL is just to remove
shouldStopBlinkingDueToTypingCommand(), however this CL adds a lot of code to
make the code testable.
- Add FrameCaret::recreateCaretBlinkTimerForTesting(RefPtr<WebTaskRunner>)
- Add FrameSelection::frameCaretForTesting()
- Add support of delayed task to FakeWebTaskRunner

BUG=692900
==========

to

==========
Editing: Fix caret blinking after a typing.

FrameCaret::updateAppearance() is called in updateStyleAndLayoutIfNeeded(). So
if shouldStopBlinkingDueToTypingCommand() is true, we repeatedly stop and start
the caret timer.

Because FrameSelection::didSetSelectionDeprecated() calls stopCaretBlinkTimer(),
we always restart the caret timer after every selection change. Checking
shouldStopBlinkingDueToTypingCommand() is unnecessary.

The core part of the CL is just to remove
shouldStopBlinkingDueToTypingCommand(), however this CL adds a lot of code to
make the code testable.
- Add FrameCaret::recreateCaretBlinkTimerForTesting(RefPtr<WebTaskRunner>)
- Add FrameSelection::frameCaretForTesting()
- Add support of delayed task to FakeWebTaskRunner

BUG=692900

Review-Url: https://codereview.chromium.org/2723003002
Cr-Commit-Position: refs/heads/master@{#453858}
Committed:
https://chromium.googlesource.com/chromium/src/+/045a3dd143fabf6787774090a255...
==========

[commit-bot: I haz the power, 2017-03-01 05:55:44]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/045a3dd143fabf6787774090a255...