Update layout for mostForward/BackwardCaretPosition in editing/commands

Update layout for mostForward/BackwardCaretPosition in editing/commands

mostForward/BackwardCaretPosition should be, but are currently not,
called with clean layout.

This patch ensures that they are called with clean layout at
several places in editing/commands

BUG=679991
Review-Url: https://codereview.chromium.org/2628693003
Cr-Commit-Position: refs/heads/master@{#443206}
Committed: https://chromium.googlesource.com/chromium/src/+/9438f509f344dd4ded216be3c38505c09f35922d

[Xiaocheng, 2017-01-11 08:26:29]

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

[commit-bot: I haz the power, 2017-01-11 08:26:51]

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-01-11 09:42:49]

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

[commit-bot: I haz the power, 2017-01-11 09:42:50]

Dry run: 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_...)

[Xiaocheng, 2017-01-11 11:52:10]

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

[commit-bot: I haz the power, 2017-01-11 11:52: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-01-11 12:24:38]

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

[commit-bot: I haz the power, 2017-01-11 12:24:38]

Dry run: This issue passed the CQ dry run.

[Xiaocheng, 2017-01-12 04:20:59]

Description was changed from

==========
Update layout for mostForward/BackwardCaretPosition when possible

BUG=679991
==========

to

==========
Update layout for mostForward/BackwardCaretPosition when possible

mostForward/BackwardCaretPosition should be, but are currently not,
called with clean layout.

This patch ensures that they are called with clean layout at
several places in editing/commands

BUG=679991
==========

[Xiaocheng, 2017-01-12 04:21:15]

Description was changed from

==========
Update layout for mostForward/BackwardCaretPosition when possible

mostForward/BackwardCaretPosition should be, but are currently not,
called with clean layout.

This patch ensures that they are called with clean layout at
several places in editing/commands

BUG=679991
==========

to

==========
Update layout for mostForward/BackwardCaretPosition in editing/commands

mostForward/BackwardCaretPosition should be, but are currently not,
called with clean layout.

This patch ensures that they are called with clean layout at
several places in editing/commands

BUG=679991
==========

[Xiaocheng, 2017-01-12 04:21:26]

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

[Xiaocheng, 2017-01-12 04:21:26]

PTAL.

[yosin_UTC9, 2017-01-12 04:52:26]

lgtm w/ nits

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

https://codereview.chromium.org/2628693003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.cpp:1976:
document().updateStyleAndLayoutIgnorePendingStylesheets();
Could you add following comment as same as others:


  // TODO(editing-dev): The use of updateStyleAndLayoutIgnorePendingStylesheets
  // needs to be audited.  See http://crbug.com/590369 for more details.
  // In the long term we should use idle time spell checker to prevent
  // synchronous layout caused by spell checking (see crbug.com/517298).

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

https://codereview.chromium.org/2628693003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/commands/InsertListCommand.cpp:599:
document().updateStyleAndLayoutIgnorePendingStylesheets();
ditto. Please add comment.

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

https://codereview.chromium.org/2628693003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/commands/InsertParagraphSeparatorCommand.cpp:367:
document().updateStyleAndLayoutIgnorePendingStylesheets();
ditto. Please add comment.

[Xiaocheng, 2017-01-12 07:39:43]

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

https://codereview.chromium.org/2628693003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.cpp:1976:
document().updateStyleAndLayoutIgnorePendingStylesheets();
On 2017/01/12 at 04:52:26, Yosi_UTC9 wrote:
> Could you add following comment as same as others:
> 
> 
>   // TODO(editing-dev): The use of
updateStyleAndLayoutIgnorePendingStylesheets
>   // needs to be audited.  See http://crbug.com/590369 for more details.
>   // In the long term we should use idle time spell checker to prevent
>   // synchronous layout caused by spell checking (see crbug.com/517298).

We don't use to put this notes in editing/commands.

[Xiaocheng, 2017-01-12 09:19:17]

The CQ bit was checked by xiaochengh@chromium.org

[commit-bot: I haz the power, 2017-01-12 09:19:37]

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

[yosin_UTC9, 2017-01-12 09:39:55]

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

https://codereview.chromium.org/2628693003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/commands/CompositeEditCommand.cpp:1976:
document().updateStyleAndLayoutIgnorePendingStylesheets();
On 2017/01/12 at 07:39:43, Xiaocheng wrote:
> On 2017/01/12 at 04:52:26, Yosi_UTC9 wrote:
> > Could you add following comment as same as others:
> > 
> > 
> >   // TODO(editing-dev): The use of
updateStyleAndLayoutIgnorePendingStylesheets
> >   // needs to be audited.  See http://crbug.com/590369 for more details.
> >   // In the long term we should use idle time spell checker to prevent
> >   // synchronous layout caused by spell checking (see crbug.com/517298).
> 
> We don't use to put this notes in editing/commands.

Talk offline. We don't need to have a comment mention about pipeline, but
we still don't want to have update layout in document.execCommand()

[1] mention about execCommand re-architecture, so we may want to mention this
in every update layout in editing/commands/

We may have bluk comment insert patch. ;-)


[1] http://crbug.com/680432 Re-architecture document.execCommand implementation

[commit-bot: I haz the power, 2017-01-12 11:07:27]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1484212757072450, "parent_rev":
"4b8fa62ab223d4db0ea5d1a8359cbaeba3d757a6", "commit_rev":
"9438f509f344dd4ded216be3c38505c09f35922d"}

[commit-bot: I haz the power, 2017-01-12 11:07:55]

Description was changed from

==========
Update layout for mostForward/BackwardCaretPosition in editing/commands

mostForward/BackwardCaretPosition should be, but are currently not,
called with clean layout.

This patch ensures that they are called with clean layout at
several places in editing/commands

BUG=679991
==========

to

==========
Update layout for mostForward/BackwardCaretPosition in editing/commands

mostForward/BackwardCaretPosition should be, but are currently not,
called with clean layout.

This patch ensures that they are called with clean layout at
several places in editing/commands

BUG=679991

Review-Url: https://codereview.chromium.org/2628693003
Cr-Commit-Position: refs/heads/master@{#443206}
Committed:
https://chromium.googlesource.com/chromium/src/+/9438f509f344dd4ded216be3c385...
==========

[commit-bot: I haz the power, 2017-01-12 11:07:58]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/9438f509f344dd4ded216be3c385...