WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
107161
imageSmoothingEnabled frequent, unpredictable crashes
https://bugs.webkit.org/show_bug.cgi?id=107161
Summary
imageSmoothingEnabled frequent, unpredictable crashes
sugoi
Reported
2013-01-17 12:46:43 PST
Visit the URL to reproduce the crash. This crash was found in chrome :
http://code.google.com/p/chromium/issues/detail?id=166100
Attachments
Patch
(3.43 KB, patch)
2013-01-17 12:57 PST
,
sugoi
no flags
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
sugoi
Comment 1
2013-01-17 12:57:03 PST
Created
attachment 183249
[details]
Patch
Stephen White
Comment 2
2013-01-17 13:25:59 PST
Comment on
attachment 183249
[details]
Patch Looks good. r=me
WebKit Review Bot
Comment 3
2013-01-17 15:18:51 PST
Comment on
attachment 183249
[details]
Patch Clearing flags on attachment: 183249 Committed
r140057
: <
http://trac.webkit.org/changeset/140057
>
WebKit Review Bot
Comment 4
2013-01-17 15:18:54 PST
All reviewed patches have been landed. Closing bug.
Alexey Proskuryakov
Comment 5
2013-01-18 10:53:59 PST
Comment on
attachment 183249
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=183249&action=review
> Source/WebCore/ChangeLog:3 > + imageSmoothingEnabled frequent, unpredictable crashes
This bug title implies that the issue is mysterious and unreproducible. But there is a test case, so the issue in in fact well understood. The right thing to do would be to re-title the bug to explain what exactly is being fixed, and to use the new title in ChangeLog too.
> Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:2403 > + GraphicsContext* c = drawingContext(); > + if (c) > + c->setImageInterpolationQuality(enabled ? DefaultInterpolationQuality : InterpolationNone);
A slightly better way to write this (and how we usually do it in WebKit) would be: if (GraphicsContext* context = drawingContext()) context->setImageInterpolationQuality(enabled ? DefaultInterpolationQuality : InterpolationNone); Using "c" as variable name is particularly against WebKit coding culture.
> LayoutTests/fast/canvas/canvas-imageSmoothingEnabled-zero-size.html:1 > +
This blank line doesn't make a technical difference, but is surprising.
sugoi
Comment 6
2013-01-18 11:12:35 PST
> > Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:2403 > > + GraphicsContext* c = drawingContext(); > > + if (c) > > + c->setImageInterpolationQuality(enabled ? DefaultInterpolationQuality : InterpolationNone); > > A slightly better way to write this (and how we usually do it in WebKit) would be: > > if (GraphicsContext* context = drawingContext()) > context->setImageInterpolationQuality(enabled ? DefaultInterpolationQuality : InterpolationNone); > > Using "c" as variable name is particularly against WebKit coding culture.
Really ? Because, in that file, I counted this line 40 times : GraphicsContext* c = drawingContext(); And not a single time is it declared within the if() statement. I was actually trying to keep the style of the file I was working in.
Benjamin Poulain
Comment 7
2013-02-01 12:51:38 PST
> Really ? Because, in that file, I counted this line 40 times : > GraphicsContext* c = drawingContext(); > And not a single time is it declared within the if() statement. I was actually trying to keep the style of the file I was working in.
Past mistake are no excuses to avoid doing a good job. I completely agree with Alexey. The ChangeLog is poor and the code should have been better.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug