Bug 112072 – Replace static_casts with to* functions - Part 2

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Abhishek Arya]

Replace static_casts with to* functions

[Abhishek Arya]

Created attachment 192570 [details]
Patch

[Philip Rogers]

Comment on attachment 192570 [details]
Patch

Can you add a bit more to the ChangeLog so people aren't confused when they see this in a few years? Just a small summary of what we're doing here.

LGTM otherwise.

[Abhishek Arya]

Created attachment 192579 [details]
Patch

[WebKit Review Bot]

Comment on attachment 192579 [details]
Patch

Clearing flags on attachment: 192579

Committed r145462: <http://trac.webkit.org/changeset/145462>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[WebKit Review Bot]

Re-opened since this is blocked by bug 112097

[Abhishek Arya]

Created attachment 192629 [details]
Patch

[Abhishek Arya]

Loads of toElement() crashes in WebCore::ApplyStyleCommand::applyBlockStyle

http://build.webkit.org/results/Apple%20Lion%20Debug%20WK1%20(Tests)/r145468%20(7365)/results.html

[Ryosuke Niwa]

(In reply to comment #8)
> Loads of toElement() crashes in WebCore::ApplyStyleCommand::applyBlockStyle
> 
> http://build.webkit.org/results/Apple%20Lion%20Debug%20WK1%20(Tests)/r145468%20(7365)/results.html

That's a false positive. We should be using toContainerNode instead.

[Ryosuke Niwa]

Comment on attachment 192629 [details]
Patch

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

> Source/WebCore/editing/ApplyStyleCommand.cpp:294
> -    startRange = TextIterator::rangeFromLocationAndLength(static_cast<Element*>(scope), startIndex, 0, true);
> -    endRange = TextIterator::rangeFromLocationAndLength(static_cast<Element*>(scope), endIndex, 0, true);
> +    startRange = TextIterator::rangeFromLocationAndLength(toElement(scope), startIndex, 0, true);
> +    endRange = TextIterator::rangeFromLocationAndLength(toElement(scope), endIndex, 0, true);

Please use toContainerNode here. scope can be Document.

[Abhishek Arya]

(In reply to comment #10)
> (From update of attachment 192629 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=192629&action=review
> 
> > Source/WebCore/editing/ApplyStyleCommand.cpp:294
> > -    startRange = TextIterator::rangeFromLocationAndLength(static_cast<Element*>(scope), startIndex, 0, true);
> > -    endRange = TextIterator::rangeFromLocationAndLength(static_cast<Element*>(scope), endIndex, 0, true);
> > +    startRange = TextIterator::rangeFromLocationAndLength(toElement(scope), startIndex, 0, true);
> > +    endRange = TextIterator::rangeFromLocationAndLength(toElement(scope), endIndex, 0, true);
> 
> Please use toContainerNode here. scope can be Document.

Thanks a lot Ryosuke for confirming. I don't understand why cr-linux-debug didnt shout at me :(.

[Abhishek Arya]

Created attachment 192643 [details]
Patch

[Ryosuke Niwa]

(In reply to comment #11)
>
> Thanks a lot Ryosuke for confirming. I don't understand why cr-linux-debug didnt shout at me :(.

cr-linux-debug doesn't run tests.

[Build Bot]

Comment on attachment 192643 [details]
Patch

Attachment 192643 [details] did not pass mac-ews (mac):
Output: http://webkit-commit-queue.appspot.com/results/17174047

New failing tests:
platform/mac/fast/speechsynthesis/speech-synthesis-speak.html

[Abhishek Arya]

Lets me check these out locally, they might be real bugs.

Regressions: Unexpected crashes (2)
  editing/selection/selection-modify-crash.html [ Crash ]
  platform/mac/fast/speechsynthesis/speech-synthesis-speak.html [ Crash ]

[Abhishek Arya]

Comment on attachment 192643 [details]
Patch

Nothing crashes locally on mac, going cq+.

[WebKit Review Bot]

Comment on attachment 192643 [details]
Patch

Clearing flags on attachment: 192643

Committed r145562: <http://trac.webkit.org/changeset/145562>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.