WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
37715
[chromium] crash when dragging images
https://bugs.webkit.org/show_bug.cgi?id=37715
Summary
[chromium] crash when dragging images
Evan Stade
Reported
2010-04-16 11:11:45 PDT
code.google.com/p/chromium/issues/detail?id=41632
Attachments
try1
(1.32 KB, patch)
2010-04-16 11:16 PDT
,
Evan Stade
levin
: review-
Details
Formatted Diff
Diff
test
(7.38 KB, patch)
2010-04-16 20:15 PDT
,
Evan Stade
no flags
Details
Formatted Diff
Diff
style fixes
(7.38 KB, patch)
2010-04-19 11:40 PDT
,
Evan Stade
no flags
Details
Formatted Diff
Diff
style fix2
(7.39 KB, patch)
2010-04-19 11:43 PDT
,
Evan Stade
no flags
Details
Formatted Diff
Diff
fix compile
(
deleted
)
2010-04-19 12:41 PDT
,
Evan Stade
levin
: review-
Details
Formatted Diff
Diff
style, again
(7.38 KB, patch)
2010-04-19 13:37 PDT
,
Evan Stade
no flags
Details
Formatted Diff
Diff
Show Obsolete
(5)
View All
Add attachment
proposed patch, testcase, etc.
Evan Stade
Comment 1
2010-04-16 11:16:45 PDT
Created
attachment 53542
[details]
try1 first time I uploaded a patch using git. Bet it won't apply.
Evan Martin
Comment 2
2010-04-16 11:31:18 PDT
Needs a test or a discussion of why it can't be tested in the changelog. :\
David Levin
Comment 3
2010-04-16 11:49:03 PDT
Comment on
attachment 53542
[details]
try1 What Mr Martin said. Other than that it looks fine.
Evan Stade
Comment 4
2010-04-16 16:03:29 PDT
ok, I found a repro. Go to a page with an SVG, and then hit shift-f5 and drag like mad from the point where the SVG is going to load. After several tries it should crash the renderer. Unfortunately it appears my js abilities are too weak to create a test out of this.
Evan Stade
Comment 5
2010-04-16 20:15:26 PDT
Created
attachment 53589
[details]
test unit test added
WebKit Review Bot
Comment 6
2010-04-16 20:17:06 PDT
Attachment 53589
[details]
did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebKit/chromium/tests/DragImageTest.cpp:33: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] WebKit/chromium/tests/DragImageTest.cpp:35: Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] WebKit/chromium/tests/DragImageTest.cpp:92: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] WebKit/chromium/tests/DragImageTest.cpp:94: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] WebKit/chromium/tests/DragImageTest.cpp:130: Use 0 instead of NULL. [readability/null] [4] WebKit/chromium/tests/DragImageTest.cpp:136: One space before end of line comments [whitespace/comments] [5] Total errors found: 6 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Evan Martin
Comment 7
2010-04-17 14:29:07 PDT
Whoa, we can do unit tests in webkit? Probably want s/m_NativeImage/m_nativeImage/g. The code looks good to me, but I am not a reviewer.
Evan Stade
Comment 8
2010-04-19 11:40:51 PDT
Created
attachment 53696
[details]
style fixes Yes, the unit tests are a new development I believe. (git log says Jan 8 -- not that new apparently) I believe that the header style error is not applicable for unit tests.
WebKit Review Bot
Comment 9
2010-04-19 11:41:40 PDT
Attachment 53696
[details]
did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebKit/chromium/tests/DragImageTest.cpp:33: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] WebKit/chromium/tests/DragImageTest.cpp:35: Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] WebKit/chromium/tests/DragImageTest.cpp:92: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] WebKit/chromium/tests/DragImageTest.cpp:94: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] WebKit/chromium/tests/DragImageTest.cpp:130: Use 0 instead of NULL. [readability/null] [4] Total errors found: 5 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
Evan Stade
Comment 10
2010-04-19 11:43:41 PDT
Created
attachment 53697
[details]
style fix2
WebKit Review Bot
Comment 11
2010-04-19 11:48:06 PDT
Attachment 53697
[details]
did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebKit/chromium/tests/DragImageTest.cpp:33: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] WebKit/chromium/tests/DragImageTest.cpp:35: Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] Total errors found: 2 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
WebKit Review Bot
Comment 12
2010-04-19 12:34:05 PDT
Attachment 53697
[details]
did not build on chromium: Build output:
http://webkit-commit-queue.appspot.com/results/1608607
Evan Stade
Comment 13
2010-04-19 12:41:18 PDT
Created
attachment 53706
[details]
fix compile
WebKit Review Bot
Comment 14
2010-04-19 12:48:22 PDT
Attachment 53706
[details]
did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebKit/chromium/tests/DragImageTest.cpp:33: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] WebKit/chromium/tests/DragImageTest.cpp:35: Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] Total errors found: 2 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
David Levin
Comment 15
2010-04-19 13:33:41 PDT
Comment on
attachment 53706
[details]
fix compile
> diff --git a/WebKit/chromium/tests/DragImageTest.cpp b/WebKit/chromium/tests/DragImageTest.cpp > +class TestImage : public Image { > +public: > + > + explicit TestImage(const IntSize& size) > + : Image(0), > + m_Size(size)
Two nits: m_size (instead of m_Size) and put the comma before the m_size instead of on the previous line. That's all though.
Evan Stade
Comment 16
2010-04-19 13:37:07 PDT
Created
attachment 53715
[details]
style, again done
WebKit Review Bot
Comment 17
2010-04-19 13:43:30 PDT
Attachment 53715
[details]
did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebKit/chromium/tests/DragImageTest.cpp:33: Found other header before a header this file implements. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] WebKit/chromium/tests/DragImageTest.cpp:35: Found header this file implements after other header. Should be: config.h, primary header, blank line, and then alphabetically sorted. [build/include_order] [4] Total errors found: 2 in 5 files If any of these errors are false positives, please file a bug against check-webkit-style.
David Levin
Comment 18
2010-04-19 13:47:01 PDT
Comment on
attachment 53715
[details]
style, again Didn't mark cq+ yet... as it would be nice to wait for the chromium ews to turn green.
David Levin
Comment 19
2010-04-19 13:58:14 PDT
Comment on
attachment 53715
[details]
style, again Changing back to r? to allow ews to pick it up.
WebKit Commit Bot
Comment 20
2010-04-20 06:44:40 PDT
Comment on
attachment 53715
[details]
style, again Clearing flags on attachment: 53715 Committed
r57888
: <
http://trac.webkit.org/changeset/57888
>
WebKit Commit Bot
Comment 21
2010-04-20 06:44:46 PDT
All reviewed patches have been landed. Closing bug.
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