Prune createVisiblePositionDeprecated from EditingUtilities

Prune createVisiblePositionDeprecated from EditingUtilities

BUG=647219
Committed: https://crrev.com/f32fe929bc1d68561d8bfddb1ab0e631b8390b01
Cr-Commit-Position: refs/heads/master@{#424698}

[Xiaocheng, 2016-10-12 06:20:17]

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

[commit-bot: I haz the power, 2016-10-12 06:20:37]

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

[Xiaocheng, 2016-10-12 07:08:01]

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

[Xiaocheng, 2016-10-12 07:08:02]

PTAL.

[yosin_UTC9, 2016-10-12 07:32:00]

It seems it is hard to read/write code to maintain clean layout tree. Updating
layout tree Just before calling function requiring clean layout tree is easier.

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

https://codereview.chromium.org/2413633002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/commands/ApplyStyleCommand.cpp:835:
document().updateStyleAndLayoutIgnorePendingStylesheets();
How about moving updateStyleAndLayoutXXX() to just before
fixRangeAndApplyInlineStyle() to be more clear where we need updateSAL().

It seems we can move updateSAL() at L803 for fixRangeAndApplyInlineStyle() at
L830.

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

https://codereview.chromium.org/2413633002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/commands/InsertListCommand.cpp:71:
document().updateStyleAndLayoutIgnorePendingStylesheets();
It is better to call updateSAL() just before |canMergeLists()|.

https://codereview.chromium.org/2413633002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/commands/InsertListCommand.cpp:79:
document().updateStyleAndLayoutIgnorePendingStylesheets();
Please move this updateSAL() just before |canMergeLists()| at L90.

Two calls of updateSAL() should not be slow.

[Xiaocheng, 2016-10-12 07:43:30]

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

[commit-bot: I haz the power, 2016-10-12 07:43:52]

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

[Xiaocheng, 2016-10-12 07:45:57]

Updated. PTAL.

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

https://codereview.chromium.org/2413633002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/commands/ApplyStyleCommand.cpp:835:
document().updateStyleAndLayoutIgnorePendingStylesheets();
On 2016/10/12 at 07:32:00, Yosi_UTC9 wrote:
> How about moving updateStyleAndLayoutXXX() to just before
fixRangeAndApplyInlineStyle() to be more clear where we need updateSAL().
> 
> It seems we can move updateSAL() at L803 for fixRangeAndApplyInlineStyle() at
L830.

The call at L803 seems pretty normal to me. The new updateSAL call is move to
just above fixRangeAndApplyInlineStyle

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

https://codereview.chromium.org/2413633002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/commands/InsertListCommand.cpp:71:
document().updateStyleAndLayoutIgnorePendingStylesheets();
On 2016/10/12 at 07:32:00, Yosi_UTC9 wrote:
> It is better to call updateSAL() just before |canMergeLists()|.

Done.

https://codereview.chromium.org/2413633002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/editing/commands/InsertListCommand.cpp:79:
document().updateStyleAndLayoutIgnorePendingStylesheets();
On 2016/10/12 at 07:32:00, Yosi_UTC9 wrote:
> Please move this updateSAL() just before |canMergeLists()| at L90.
> 
> Two calls of updateSAL() should not be slow.

Done.

[yosin_UTC9, 2016-10-12 08:08:48]

lgtm

Easier to understand.
Thanks!

[commit-bot: I haz the power, 2016-10-12 09:18:33]

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

[commit-bot: I haz the power, 2016-10-12 09:18:34]

Dry run: This issue passed the CQ dry run.

[Xiaocheng, 2016-10-12 09:21:59]

The CQ bit was checked by xiaochengh@chromium.org

[commit-bot: I haz the power, 2016-10-12 09:22:12]

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, 2016-10-12 09:27:22]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-10-12 09:28:59]

Description was changed from

==========
Prune createVisiblePositionDeprecated from EditingUtilities

BUG=647219
==========

to

==========
Prune createVisiblePositionDeprecated from EditingUtilities

BUG=647219

Committed: https://crrev.com/f32fe929bc1d68561d8bfddb1ab0e631b8390b01
Cr-Commit-Position: refs/heads/master@{#424698}
==========

[commit-bot: I haz the power, 2016-10-12 09:29:00]

Patchset 2 (id:??) landed as
https://crrev.com/f32fe929bc1d68561d8bfddb1ab0e631b8390b01
Cr-Commit-Position: refs/heads/master@{#424698}