Mark the createVisiblePosition overloads as deprecated

Mark current createVisiblePosition overloads deprecated

This patch renames createVisiblePosition() (all overloads) to
createVisiblePositionDeprecated() to discourage creating a VisiblePosition
in dirty layout and waiting for layout update lazily. This patch also
introduces new createVisiblePosition() which assumes clean layout.

This patch is a preparation for hoisting the layout update call from the old
createVisiblePosition(). In follow up patches, the callers should ensure
clean layout by themselves and call the new createVisiblePosition() instead.

This patch is a mechanical substitution
's/createVisiblePosition(/createVisiblePositionDeprecated(/g', followed
by the following changes:
- Introduce the new createVisiblePosition() in VisiblePosition.h/cpp.
- Revert the substitution in modules/accessibility/AXLayoutObject.cpp,
  since callers there already ensure clean layout

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

[Xiaocheng, 2016-09-15 12:01:38]

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

[commit-bot: I haz the power, 2016-09-15 12:01:51]

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

[Xiaocheng, 2016-09-15 12:12:18]

Description was changed from

==========
Mark current createVisiblePosition overloads deprecated

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

to

==========
Mark current createVisiblePosition overloads deprecated

This patch renames createVisiblePosition() (all overloads) to
createVisiblePositionDeprecated() to discourage creating a VisiblePosition
in dirty layout and waiting for layout update lazily. This patch also
introduces new createVisiblePosition() which assumes clean layout.

This patch is a preparation for hoisting the layout update call from the old
createVisiblePosition(). In follow up patches, the callers should ensure
clean layout by themselves and call the new createVisiblePosition() instead.

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

[Xiaocheng, 2016-09-15 12:14:34]

Description was changed from

==========
Mark current createVisiblePosition overloads deprecated

This patch renames createVisiblePosition() (all overloads) to
createVisiblePositionDeprecated() to discourage creating a VisiblePosition
in dirty layout and waiting for layout update lazily. This patch also
introduces new createVisiblePosition() which assumes clean layout.

This patch is a preparation for hoisting the layout update call from the old
createVisiblePosition(). In follow up patches, the callers should ensure
clean layout by themselves and call the new createVisiblePosition() instead.

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

to

==========
Mark current createVisiblePosition overloads deprecated

This patch renames createVisiblePosition() (all overloads) to
createVisiblePositionDeprecated() to discourage creating a VisiblePosition
in dirty layout and waiting for layout update lazily. This patch also
introduces new createVisiblePosition() which assumes clean layout.

This patch is a preparation for hoisting the layout update call from the old
createVisiblePosition(). In follow up patches, the callers should ensure
clean layout by themselves and call the new createVisiblePosition() instead.

This patch is a mechanical substitution
's/createVisiblePosition(/createVisiblePositionDeprecated(/g', plus the
introduction of the new createVisiblePosition() in VisiblePosition.h/cpp.

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

[Xiaocheng, 2016-09-15 12:20:35]

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

[commit-bot: I haz the power, 2016-09-15 12:20:50]

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

[Xiaocheng, 2016-09-15 12:22:20]

Description was changed from

==========
Mark current createVisiblePosition overloads deprecated

This patch renames createVisiblePosition() (all overloads) to
createVisiblePositionDeprecated() to discourage creating a VisiblePosition
in dirty layout and waiting for layout update lazily. This patch also
introduces new createVisiblePosition() which assumes clean layout.

This patch is a preparation for hoisting the layout update call from the old
createVisiblePosition(). In follow up patches, the callers should ensure
clean layout by themselves and call the new createVisiblePosition() instead.

This patch is a mechanical substitution
's/createVisiblePosition(/createVisiblePositionDeprecated(/g', plus the
introduction of the new createVisiblePosition() in VisiblePosition.h/cpp.

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

to

==========
Mark current createVisiblePosition overloads deprecated

This patch renames createVisiblePosition() (all overloads) to
createVisiblePositionDeprecated() to discourage creating a VisiblePosition
in dirty layout and waiting for layout update lazily. This patch also
introduces new createVisiblePosition() which assumes clean layout.

This patch is a preparation for hoisting the layout update call from the old
createVisiblePosition(). In follow up patches, the callers should ensure
clean layout by themselves and call the new createVisiblePosition() instead.

This patch is a mechanical substitution
's/createVisiblePosition(/createVisiblePositionDeprecated(/g', followed
by the following changes:
- Introduce the new createVisiblePosition() in VisiblePosition.h/cpp.
- Still use |createVisiblePosition()| in AXLayoutObject.cpp since callers
  there already ensure clean layout

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

[Xiaocheng, 2016-09-15 12:23:13]

Description was changed from

==========
Mark current createVisiblePosition overloads deprecated

This patch renames createVisiblePosition() (all overloads) to
createVisiblePositionDeprecated() to discourage creating a VisiblePosition
in dirty layout and waiting for layout update lazily. This patch also
introduces new createVisiblePosition() which assumes clean layout.

This patch is a preparation for hoisting the layout update call from the old
createVisiblePosition(). In follow up patches, the callers should ensure
clean layout by themselves and call the new createVisiblePosition() instead.

This patch is a mechanical substitution
's/createVisiblePosition(/createVisiblePositionDeprecated(/g', followed
by the following changes:
- Introduce the new createVisiblePosition() in VisiblePosition.h/cpp.
- Still use |createVisiblePosition()| in AXLayoutObject.cpp since callers
  there already ensure clean layout

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

to

==========
Mark current createVisiblePosition overloads deprecated

This patch renames createVisiblePosition() (all overloads) to
createVisiblePositionDeprecated() to discourage creating a VisiblePosition
in dirty layout and waiting for layout update lazily. This patch also
introduces new createVisiblePosition() which assumes clean layout.

This patch is a preparation for hoisting the layout update call from the old
createVisiblePosition(). In follow up patches, the callers should ensure
clean layout by themselves and call the new createVisiblePosition() instead.

This patch is a mechanical substitution
's/createVisiblePosition(/createVisiblePositionDeprecated(/g', followed
by the following changes:
- Introduce the new createVisiblePosition() in VisiblePosition.h/cpp.
- Revert the substitution in modules/accessibility/AXLayoutObject.cpp,
  since callers there already ensure clean layout

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

[commit-bot: I haz the power, 2016-09-15 13:31:48]

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

[commit-bot: I haz the power, 2016-09-15 13:31:48]

Dry run: This issue passed the CQ dry run.

[Xiaocheng, 2016-09-15 13:43:45]

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

[Xiaocheng, 2016-09-15 13:43:46]

PTAL.

[tkent, 2016-09-16 00:13:41]

lgtm

[Xiaocheng, 2016-09-16 01:09:27]

The CQ bit was checked by xiaochengh@chromium.org

[commit-bot: I haz the power, 2016-09-16 01:10:18]

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-09-16 01:24:37]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-16 01:26:32]

Description was changed from

==========
Mark current createVisiblePosition overloads deprecated

This patch renames createVisiblePosition() (all overloads) to
createVisiblePositionDeprecated() to discourage creating a VisiblePosition
in dirty layout and waiting for layout update lazily. This patch also
introduces new createVisiblePosition() which assumes clean layout.

This patch is a preparation for hoisting the layout update call from the old
createVisiblePosition(). In follow up patches, the callers should ensure
clean layout by themselves and call the new createVisiblePosition() instead.

This patch is a mechanical substitution
's/createVisiblePosition(/createVisiblePositionDeprecated(/g', followed
by the following changes:
- Introduce the new createVisiblePosition() in VisiblePosition.h/cpp.
- Revert the substitution in modules/accessibility/AXLayoutObject.cpp,
  since callers there already ensure clean layout

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

to

==========
Mark current createVisiblePosition overloads deprecated

This patch renames createVisiblePosition() (all overloads) to
createVisiblePositionDeprecated() to discourage creating a VisiblePosition
in dirty layout and waiting for layout update lazily. This patch also
introduces new createVisiblePosition() which assumes clean layout.

This patch is a preparation for hoisting the layout update call from the old
createVisiblePosition(). In follow up patches, the callers should ensure
clean layout by themselves and call the new createVisiblePosition() instead.

This patch is a mechanical substitution
's/createVisiblePosition(/createVisiblePositionDeprecated(/g', followed
by the following changes:
- Introduce the new createVisiblePosition() in VisiblePosition.h/cpp.
- Revert the substitution in modules/accessibility/AXLayoutObject.cpp,
  since callers there already ensure clean layout

BUG=647219

Committed: https://crrev.com/31805260d90d814face4ed1735a6ff2755692f6d
Cr-Commit-Position: refs/heads/master@{#419069}
==========

[commit-bot: I haz the power, 2016-09-16 01:26:33]

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

[yosin_UTC9, 2016-09-16 02:01:22]

lgtm

Thanks for doing this!

This is the first step of tons of changes!
I put some comments for following steps.

https://codereview.chromium.org/2341053002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/Range.cpp (right):

https://codereview.chromium.org/2341053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Range.cpp:1463: void Range::expand(const
String& unit, ExceptionState& exceptionState)
We should get rid of Range#expand() since it is Blink/WebKit specific Web API.
I filed http://crbug.com/647533 for tracking.

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

https://codereview.chromium.org/2341053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/DOMSelection.cpp:246:
m_frame->selection().moveTo(createVisiblePositionDeprecated(selection.end()));
Let's use |FrameSelection::moveTo(const Position&, ...)| version and get rid of
|FrameSelection::moveTo(const VisiblePositon&, ...)| version.

https://codereview.chromium.org/2341053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/DOMSelection.cpp:261:
m_frame->selection().moveTo(createVisiblePositionDeprecated(selection.start()));
Let's use |FrameSelection::moveTo(const Position&, ...)| version and get rid of
|FrameSelection::moveTo(const VisiblePositon&, ...)| version.

https://codereview.chromium.org/2341053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/DOMSelection.cpp:292: VisiblePosition
visibleBase = createVisiblePositionDeprecated(createPosition(baseNode,
baseOffset));
Since |VisibleSelection| ctor converts passed |Position| objects to
|VisiblePosition| in |VisibleSelection::validate()|, we can just pass |Position|
to |VisibleSelection| ctor.

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

https://codereview.chromium.org/2341053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/PendingSelection.cpp:62: SelectionType
selectionType = originalSelection.getSelectionType();
Let's make |PendingSelection::calcVisbileSelection()| not to use
|createVisbilePositionDeprecaetd()| first. This is called in rendering pipeline
after LayoutClean.

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

https://codereview.chromium.org/2341053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/RelocatablePositionTest.cpp:29:
EXPECT_EQ(createVisiblePositionDeprecated(expectedPosition).deepEquivalent(),
createVisiblePositionDeprecated(trackedPosition).deepEquivalent());
Can we stop using |createVisbilePositionDeprecated()| for testing here?

https://codereview.chromium.org/2341053002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/HitTestResult.cpp (right):

https://codereview.chromium.org/2341053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/HitTestResult.cpp:364: {
TODO: Since |HitTestResult::isMissspelled()| is called by one call site in
|SelectionController::sendContextMenuEvent()|, we should move |isMisspelled()|
to |SelectionController|.

https://codereview.chromium.org/2341053002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/testing/Internals.cpp (right):

https://codereview.chromium.org/2341053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/testing/Internals.cpp:2401: if (!node ||
!node->layoutObject())
We should update layout before using |Node::layoutObject()|.

https://codereview.chromium.org/2341053002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/WebSurroundingText.cpp (right):

https://codereview.chromium.org/2341053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/WebSurroundingText.cpp:52: if (!node ||
!node->layoutObject())
We should update layout before using |Node::layoutObject()|.

[Xiaocheng, 2016-09-16 06:56:57]

On 2016/09/16 at 02:01:22, yosin wrote:
> lgtm
> 
> Thanks for doing this!
> 
> This is the first step of tons of changes!
> I put some comments for following steps.
> 
>
https://codereview.chromium.org/2341053002/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/dom/Range.cpp (right):
> 
>
https://codereview.chromium.org/2341053002/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/dom/Range.cpp:1463: void Range::expand(const
String& unit, ExceptionState& exceptionState)
> We should get rid of Range#expand() since it is Blink/WebKit specific Web API.
> I filed http://crbug.com/647533 for tracking.
> 
>
https://codereview.chromium.org/2341053002/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/editing/DOMSelection.cpp (right):
> 
>
https://codereview.chromium.org/2341053002/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/editing/DOMSelection.cpp:246:
m_frame->selection().moveTo(createVisiblePositionDeprecated(selection.end()));
> Let's use |FrameSelection::moveTo(const Position&, ...)| version and get rid
of |FrameSelection::moveTo(const VisiblePositon&, ...)| version.
> 
>
https://codereview.chromium.org/2341053002/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/editing/DOMSelection.cpp:261:
m_frame->selection().moveTo(createVisiblePositionDeprecated(selection.start()));
> Let's use |FrameSelection::moveTo(const Position&, ...)| version and get rid
of |FrameSelection::moveTo(const VisiblePositon&, ...)| version.
> 
>
https://codereview.chromium.org/2341053002/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/editing/DOMSelection.cpp:292: VisiblePosition
visibleBase = createVisiblePositionDeprecated(createPosition(baseNode,
baseOffset));
> Since |VisibleSelection| ctor converts passed |Position| objects to
|VisiblePosition| in |VisibleSelection::validate()|, we can just pass |Position|
to |VisibleSelection| ctor.
> 
>
https://codereview.chromium.org/2341053002/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/editing/PendingSelection.cpp (right):
> 
>
https://codereview.chromium.org/2341053002/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/editing/PendingSelection.cpp:62: SelectionType
selectionType = originalSelection.getSelectionType();
> Let's make |PendingSelection::calcVisbileSelection()| not to use
|createVisbilePositionDeprecaetd()| first. This is called in rendering pipeline
after LayoutClean.
> 
>
https://codereview.chromium.org/2341053002/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/editing/RelocatablePositionTest.cpp
(right):
> 
>
https://codereview.chromium.org/2341053002/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/editing/RelocatablePositionTest.cpp:29:
EXPECT_EQ(createVisiblePositionDeprecated(expectedPosition).deepEquivalent(),
createVisiblePositionDeprecated(trackedPosition).deepEquivalent());
> Can we stop using |createVisbilePositionDeprecated()| for testing here?
> 
>
https://codereview.chromium.org/2341053002/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/layout/HitTestResult.cpp (right):
> 
>
https://codereview.chromium.org/2341053002/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/layout/HitTestResult.cpp:364: {
> TODO: Since |HitTestResult::isMissspelled()| is called by one call site in
|SelectionController::sendContextMenuEvent()|, we should move |isMisspelled()|
to |SelectionController|.
> 
>
https://codereview.chromium.org/2341053002/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/core/testing/Internals.cpp (right):
> 
>
https://codereview.chromium.org/2341053002/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/core/testing/Internals.cpp:2401: if (!node ||
!node->layoutObject())
> We should update layout before using |Node::layoutObject()|.
> 
>
https://codereview.chromium.org/2341053002/diff/20001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/web/WebSurroundingText.cpp (right):
> 
>
https://codereview.chromium.org/2341053002/diff/20001/third_party/WebKit/Sour...
> third_party/WebKit/Source/web/WebSurroundingText.cpp:52: if (!node ||
!node->layoutObject())
> We should update layout before using |Node::layoutObject()|.

Thanks for the comments.

That's already kind of a sketch of what we are going to do next.

[Xiaocheng, 2016-09-16 09:14:00]

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

https://codereview.chromium.org/2341053002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/DOMSelection.cpp:292: VisiblePosition
visibleBase = createVisiblePositionDeprecated(createPosition(baseNode,
baseOffset));
On 2016/09/16 at 02:01:21, Yosi_UTC9 wrote:
> Since |VisibleSelection| ctor converts passed |Position| objects to
|VisiblePosition| in |VisibleSelection::validate()|, we can just pass |Position|
to |VisibleSelection| ctor.

We will still be updating layout here if we are going to hoist layout update
calls from VisibleSelection. Anyway, passing Positions to the VS's ctor seems to
lead to simpler code.