New issue
Advanced search Search tips

Issue 825077 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task



Sign in to add a comment

We should not use Node::MaxCharacterOffset()

Project Member Reported by yosin@chromium.org, Mar 23 2018

Issue description

We should use ToCharacterData(node)->length() instead of Node::MaxCharacterOffset() to avoid to use indirection call.

Because of default implementation of Node::MaxCharacterOffset() = NOTREACEHD()
and all call sites call it for CharacterData. Hence, there are no good reasons
to use Node::MaxCharacterOffset().

 
The patch of https://chromium-review.googlesource.com/c/chromium/src/+/979894, is under review.

@yosin, after patch, it has no use of Node::MaxCharacterOffset(), is it ok to remove it?
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 29 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a8da51c40cb8f23d002c6345df414572efcce316

commit a8da51c40cb8f23d002c6345df414572efcce316
Author: Yuhong Sha <yuhong.sha@samsung.com>
Date: Thu Mar 29 06:45:56 2018

Deprecate to use Node::MaxCharacterOffset() api

Use ToCharacterData(node)->length() instead of Node::MaxCharacterOffset()
to avoid to use indirection call.

Because has the default implementation of Node::MaxCharacterOffset() = NOTREACEHD()
and all call sites call it for CharacterData.
Hence, it is better to deprecate to use Node::MaxCharacterOffset().

Bug: 825077

Signed-off-by: Yuhong Sha <yuhong.sha@samsung.com>
Change-Id: Ia7c44c0f92551295eb3e51cdd576ac172c2aaf25
Reviewed-on: https://chromium-review.googlesource.com/979894
Commit-Queue: srirama chandra sekhar <srirama.m@samsung.com>
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546749}
[modify] https://crrev.com/a8da51c40cb8f23d002c6345df414572efcce316/third_party/WebKit/Source/core/dom/RangeBoundaryPoint.h
[modify] https://crrev.com/a8da51c40cb8f23d002c6345df414572efcce316/third_party/WebKit/Source/core/editing/EditingStrategy.cpp
[modify] https://crrev.com/a8da51c40cb8f23d002c6345df414572efcce316/third_party/WebKit/Source/core/editing/EditingStyleUtilities.cpp
[modify] https://crrev.com/a8da51c40cb8f23d002c6345df414572efcce316/third_party/WebKit/Source/core/editing/Position.cpp
[modify] https://crrev.com/a8da51c40cb8f23d002c6345df414572efcce316/third_party/WebKit/Source/core/editing/commands/ApplyBlockElementCommand.cpp
[modify] https://crrev.com/a8da51c40cb8f23d002c6345df414572efcce316/third_party/WebKit/Source/core/editing/commands/ApplyStyleCommand.cpp
[modify] https://crrev.com/a8da51c40cb8f23d002c6345df414572efcce316/third_party/WebKit/Source/core/editing/markers/DocumentMarkerController.cpp
[modify] https://crrev.com/a8da51c40cb8f23d002c6345df414572efcce316/third_party/WebKit/Source/core/editing/suggestion/TextSuggestionController.cpp

Project Member

Comment 3 by bugdroid1@chromium.org, Mar 30 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1871283af3b7878a314d8ca767be526eab8627d9

commit 1871283af3b7878a314d8ca767be526eab8627d9
Author: Yuhong Sha <yuhong.sha@samsung.com>
Date: Fri Mar 30 01:57:11 2018

Remove the useless MaxCharacterOffset() api

It has the default implementation of Node::MaxCharacterOffset() = NOTREACEHD(),
and all call sites call it for CharacterData.

After using ToCharacterData(node)->length() to instead of Node::MaxCharacterOffset(),
The api has no use, so remove it.

Bug: 825077

Signed-off-by: Yuhong Sha <yuhong.sha@samsung.com>
Change-Id: Ic624fd70504cdf68a4b5b1e9808ba8cf41d85dd3
Reviewed-on: https://chromium-review.googlesource.com/985678
Reviewed-by: Kent Tamura <tkent@chromium.org>
Commit-Queue: Kent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#547068}
[modify] https://crrev.com/1871283af3b7878a314d8ca767be526eab8627d9/third_party/WebKit/Source/core/dom/CharacterData.cpp
[modify] https://crrev.com/1871283af3b7878a314d8ca767be526eab8627d9/third_party/WebKit/Source/core/dom/CharacterData.h
[modify] https://crrev.com/1871283af3b7878a314d8ca767be526eab8627d9/third_party/WebKit/Source/core/dom/Node.cpp
[modify] https://crrev.com/1871283af3b7878a314d8ca767be526eab8627d9/third_party/WebKit/Source/core/dom/Node.h

Sign in to add a comment