CanvasRenderingContext2D::setFont argument may reference destroyed object
Created attachment 181773 [details] Patch
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."
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.
(In reply to comment #3) > Should take out the braces here too. I wonder how come this passed the style check...
Created attachment 181792 [details] Patch for landing
Comment on attachment 181792 [details] Patch for landing Clearing flags on attachment: 181792 Committed r139144: <http://trac.webkit.org/changeset/139144>
All reviewed patches have been landed. Closing bug.