[webkit-reviews] review granted: [Bug 217846] Add AbstractRange : [Attachment 411623] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has granted Darin Adler <darin at apple.com>'s
request for review:
Bug 217846: Add AbstractRange
https://bugs.webkit.org/show_bug.cgi?id=217846

Attachment 411623: Patch

https://bugs.webkit.org/attachment.cgi?id=411623&action=review




--- Comment #3 from Ryosuke Niwa <rniwa at webkit.org> ---
Comment on attachment 411623
  --> https://bugs.webkit.org/attachment.cgi?id=411623
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=411623&action=review

r=me with the iso-heap changes.

> Source/WebCore/dom/AbstractRange.idl:32
> +    Exposed=Window,

Should we add a runtime flag for this?

> Source/WebCore/dom/Range.h:41
> -class Range : public RefCounted<Range> {
> +class Range final : public AbstractRange {

Please make this iso-heaped.

> Source/WebCore/dom/Range.h:46
> +    Node& startContainer() const final { return m_start.container(); }

It's kind of silly that Range can't use SimpleRange as its internal data
structure.
Then one of these member functions need to be virtual.

> Source/WebCore/dom/StaticRange.h:35
> +class StaticRange final : public AbstractRange, public SimpleRange {

Please make this iso-heaped.

> Source/WebCore/dom/StaticRange.idl:28
> +    JSGenerateToNativeObject,

It occurs to me that StaticRange also needs custom is reachable function.
InputEvent and HighlightRangeGroup hold onto them but we don't keep this
wrapper alive.