[webkit-dev] Proposal to Reorganize Position Classes

[Ryosuke Niwa]

To update you all, my colleagues and I have made a design
document<https://docs.google.com/document/d/1vzzi_wQHO0GtLnu-1IYutUKbcpAYlHqZwQlwbfLuowM/edit?hl=en&authkey=CLOa9cgN>.
 You should all be able to view the document.

I've given editing rights to people who have replied to this thread but
please let me know if you're interested in commenting on the document.

Best,
Ryosuke Niwa
Software Engineer
Google Inc.

On Tue, Feb 1, 2011 at 6:51 PM, Ryosuke Niwa <rniwa at webkit.org> wrote:

> Hi all,
> *
> *
> *While I'm not intending to write a patch for the following proposal
> anytime soon, I'd really like to get your feedback.*
> *
> *
> As you might all know, Position classes in WebCore need refactoring and
> cleanup.  Part of it is due to legacy editing positions that do many
> "magics", and we're currently in the process of removing them (see the bug
> 52099 <http://webkit.org/b/52099>).  However, even if we didn't have
> legacy editing positions, we still have rather clattered code base - there
> are many global functions in visible_units.cpp and VisiblePosition is used
> all over in WebCore/rendering - and reorganization of Position-related
> classes is needed (see the bug 52098 <http://webkit.org/b/52098>).
>
> I spent the last couple months to figure out what's the right solution and
> realized that there are generally two use cases of VisiblePosition:
>
>    - To differentiate upstream/downstream to express caret positions
>    - To iterate through editable regions to implement editing commands
>
> For the former use case, triggering layout or respecting editing boundary
> is not needed at all, and the use of VisiblePosition seems an overkill.  I
> concluded that these two use cases should be addressed by two different
> classes.
>
> So I propose to implement the following 3 classes in lieu of the existing
> Position and VisiblePosition:
>
>    - *DOMPosition* or *SimplePosition* - This class solely works on DOM,
>    and is agnostic of renderers and other parts of WebCore.  It's meant to be
>    used in utility functions and classes or passed along between renderers and
>    WebCore/dom.  It doesn't know anything about upstream or downstream.
>    - *RenderedPosition* - This positions is an enhanced version of the
>    current Position.  It represents every possible visible position and or DOM
>    position and communicates vocabularies between WebCore/rendering and
>    WebCore/editing.  It knows about line boundary and upstream/downstream but
>    it doesn't trigger a layout, doesn't canonicalize, and doesn't know anything
>    about editing boundary.  Its life-time is tied to layout cycle, and needs to
>    be re-created every time DOM mutation or style change occurs.  Many
>    functions in visible_units.cpp can belong to this class as member functions.
>     Furthermore, PositionIterator could be merged into this class because
>    RenderedPosition can cache pointers and other invariants as needed since
>    RenderedPosition's lifetime is tied to layout cycle.  It can also share some
>    code with TextIterator as well.
>    - *EditingPosition* or *VisiblePosition* - This class is almost
>    identical to the existing VisiblePosition.  It knows everything DOMPosition
>    and RenderedPosition knows, and respects editing boundary.  A significant
>    difference with VisiblePosition is that this class remembers the editable
>    root element to which it belongs.  So when crossing editing boundary, it can
>    figure out whether or not we're still inside the same root editable root or
>    not.  It also knows how to canonicalize itself so editing commands can
>    canonicalize positions as needed although canonicalization could be
>    optional.  I'm also not sure if this class should trigger a layout inside
>    its constructor like VisiblePosition does or not yet.
>
> The introduction of RenderedPosition is particularly useful in rendering
> engine because it allows to express any caret/insertion point position with
> a guarantee that it doesn't trigger a layout.
>
> Now, I request for your comments!
>
> Best regards,
> Ryosuke Niwa
> Software Engineer
> Google Inc.
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20110419/24869856/attachment.html>