Bug 106385 – CanvasRenderingContext2D::setFont argument may reference destroyed object

• Component: New Bugs
• Status: RESOLVED FIXED

[Justin Novosad]

CanvasRenderingContext2D::setFont argument may reference destroyed object

[Justin Novosad]

Created attachment 181773 [details]
Patch

[Abhishek Arya]

Comment on attachment 181773 [details]
Patch

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

> Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:2091
> +    String newFontSafeCopy(newFont); // In case newFont is a ref to a string touched by realizeSaves

The comment could be improved like "Create a string copy since newFont can be deleted inside realizeSaves."

[Darin Adler]

Comment on attachment 181773 [details]
Patch

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

> Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:2093
> +    modifiableState().m_unparsedFont = newFontSafeCopy;

If this was a RefPtr then we’d want to do a release here to avoid reference count churn.

Since it’s a String, we *could* avoid the churn by doing a swap here instead of assignment. But I’m thinking that’s too ugly for the tiny performance win.

> Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:2379
>      if (!state().m_realizedFont) {
> -        // Create temporary string object to hold ref count in case
> -        // state().m_unparsedFont in unreffed by call to realizeSaves in
> -        // setFont.
> -        String unparsedFont(state().m_unparsedFont);
> -        setFont(unparsedFont);
> +        setFont(state().m_unparsedFont);
>      }

Should take out the braces here too.

[Justin Novosad]

(In reply to comment #3)

> Should take out the braces here too.

I wonder how come this passed the style check...

[Justin Novosad]

Created attachment 181792 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 181792 [details]
Patch for landing

Clearing flags on attachment: 181792

Committed r139144: <http://trac.webkit.org/changeset/139144>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.