Thus you will const_cast<Node*>() in Editing functions outside of Position like
RootEditableElement()?
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/editing/EditingUtilities.cpp?l=407&gs=cpp%253Ablink%253A%253ARootEditableElement(const%2Bblink%253A%253ANode%2B%2526)%2540chromium%252F..%252F..%252Fthird_party%252FWebKit%252FSource%252Fcore%252Fediting%252FEditingUtilities.cpp%257Cdef&gsn=RootEditableElement&ct=xref_usages

>#c1, change return value to |const Node*| is controversial.

Here is an example[1] which changes ComputeNodeBeforePosition() to return |const Node*|.

It seems returning |const Node*| increases |const_cast<Node*>()| at least editing/commands/.
But, it is good signal we make |Position| invalid.

Is it OK to increase |const_cast<Node*>()|?



[1] http://crrev.com/2951073002: Make ComputeNodeBeforePosition() to return const Node*

Guys, I've tried to change the Position class to hold a const pointer to Node just out of curiosity. And I have a couple of quiestions for changes I had to do to compile the code.
Main problem is that we can't return |Node*| from Position::AnchorNode method.
If I change result type from |Node*| to |const Node*| then the folowing problems occur:
- Expressions like |Node* node = pos.AnchorNode();|.
  It is trivial to add |const| to node pointer variable.
- A lot of calls to |EnclosingBlock(pos.AnchorNode())|.
  I see that there is an overload for EnclosingBlock that takes a Position object.  Is call to  |EnclosingBlock(pos.AnchorNode())| fully equivalent to |EnclosingBlock(pos)|?
  Can we just make a trivial replacement in those places?
- At the moment non-const |Node*| from position quite often passed to functions 
  like |RemoveNode()| that requires a non-const |Node*|. What can we do with that?
  Obvious answer to all that is to have a non-const version of Position::AnchorNode method with const_cast in it.
  But thats not so cool.

Also a lof of functions takes a |Node*| but it can be a |const Node*| or even |const Node&|. Looks like we need to do a massive inspection of these functions. For example, NodeTraversal::HighestAncestorOrSelf() takes and returns |Node&|. Should we make an alternative overload or change the original function to take/return |const Node&| ?
WDYT?

iceman@: Thanks for the investigation.

Theoretically, Position has two types of clients:
1. Use a Position to inspect a certain part of DOM/Layout tree
2. Use a Position to indicate a certain part of DOM tree to be modified

The theoretical blocker is type 2 because they really need Position to return |Node*| instead of |const Node*| as AnchorNode(). For type 1, they are fine to take |const Node* AnchorNode()| -- just that a lot of code has to be refactored.

Fortunately, most of type 2 clients are in editing/commands. We may want to do something different there, but I don't have a clear idea right now.

> If I change result type from |Node*| to |const Node*| then the folowing problems occur:
> - Expressions like |Node* node = pos.AnchorNode();|.
>   It is trivial to add |const| to node pointer variable.

Yes for type-1 clients.

> - A lot of calls to |EnclosingBlock(pos.AnchorNode())|.
>   I see that there is an overload for EnclosingBlock that takes a Position object.  Is call to |EnclosingBlock(pos.AnchorNode())| fully equivalent to |EnclosingBlock(pos)|?
>   Can we just make a trivial replacement in those places?

This is a type-1 client. Should be the other way around as indicated by the code:

Element* EnclosingBlock(Node* node, EditingBoundaryCrossingRule rule) {
  return EnclosingBlock(FirstPositionInOrBeforeNodeDeprecated(node), rule);
}

So doing trivial replacements there seems fine.

> - At the moment non-const |Node*| from position quite often passed to functions 
>   like |RemoveNode()| that requires a non-const |Node*|. What can we do with that?
>   Obvious answer to all that is to have a non-const version of Position::AnchorNode method with const_cast in it.
>   But thats not so cool.

I don't have a good idea for type-2 clients now. Maybe editing commands should use their own Position types?

I have a question about methods like setStart/setEnd of Range class, [1]
It that possible to pass |const Node*| to these methods?
I'm not sure if the signature should be exactly |(Node* ref_node, unsigned offset)| because these functions are exposed to JS.

[1] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/dom/Range.h?sq=package:chromium&l=81

Today I've compile my patch [1].
It is a clearly WIP at the moment, and not ready-to-merge or even ready for real code review.
But it can give us a picture of changes that should be done.
And I have a lot of questions.

yosin@ and  xiaochengh@, it would be great if you can take a brief look at my change and give me some advises.

I will continue to work on that and extract some simple parts into separate CLs.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/793820

This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot