WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
62324
[Chromium] Crash when closing a tab with accelerated 2d canvas
https://bugs.webkit.org/show_bug.cgi?id=62324
Summary
[Chromium] Crash when closing a tab with accelerated 2d canvas
Justin Novosad
Reported
2011-06-08 14:17:10 PDT
This is in reference to chromium
bug 85309
Repro steps posted here:
http://code.google.com/p/chromium/issues/detail?id=85309
This is a Chrome release blocker
Attachments
Patch
(2.64 KB, patch)
2011-06-08 15:23 PDT
,
Justin Novosad
no flags
Details
Formatted Diff
Diff
Patch
(3.35 KB, patch)
2011-06-08 15:49 PDT
,
Justin Novosad
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Justin Novosad
Comment 1
2011-06-08 15:23:30 PDT
Created
attachment 96493
[details]
Patch
Justin Novosad
Comment 2
2011-06-08 15:26:50 PDT
No need to worry about cr-* test builds that will fail. This patch depends on a skia change that has not yet rolled in. Dependent change is pending review here:
http://codereview.appspot.com/4584048/
Stephen White
Comment 3
2011-06-08 15:29:48 PDT
This looks ok to me, but jamesr and brian should take a look.
James Robinson
Comment 4
2011-06-08 15:30:39 PDT
Comment on
attachment 96493
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=96493&action=review
You need to explain what's going on here, preferably in the ChangeLog.
> Source/WebCore/ChangeLog:8 > + No new tests. (OOPS!)
This will fail an svn presubmit
Justin Novosad
Comment 5
2011-06-08 15:49:48 PDT
Created
attachment 96497
[details]
Patch
Brian Salomon
Comment 6
2011-06-08 15:52:10 PDT
Comment on
attachment 96497
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=96497&action=review
> Source/WebCore/platform/graphics/gpu/SharedGraphicsContext3D.cpp:126 > + GrSafeUnref(m_grContext);
Minor: GrSafeUnref is now redundant, can be m_grContext->unref();
James Robinson
Comment 7
2011-06-08 15:52:23 PDT
Comment on
attachment 96497
[details]
Patch Are the two fixes for the same thing or not?
WebKit Review Bot
Comment 8
2011-06-08 15:54:05 PDT
Comment on
attachment 96497
[details]
Patch
Attachment 96497
[details]
did not pass chromium-ews (chromium-xvfb): Output:
http://queues.webkit.org/results/8776842
Justin Novosad
Comment 9
2011-06-08 15:57:37 PDT
(In reply to
comment #7
)
> (From update of
attachment 96497
[details]
) > Are the two fixes for the same thing or not?
It fixes the same test case. The callback was the first thing to crash. When I fixed that, there was another crash waiting behind it.
Justin Novosad
Comment 10
2011-06-08 16:00:44 PDT
The skia change that fixes the build failure reported by the review bot is now in ToT skia:
http://code.google.com/p/skia/source/detail?r=1551
James Robinson
Comment 11
2011-06-08 16:07:32 PDT
You need to update the skia rev in Source/WebKit/chromium/DEPS before landing this. You also need to be sure the DEPS file in chromium trunk is updated before landing this. Ping this bug again for review after you've done those two. This can't land before then.
James Robinson
Comment 12
2011-06-08 16:13:23 PDT
Comment on
attachment 96497
[details]
Patch R- until DEPS rolls, please flip back to review? once they've rolled in.
Vangelis Kokkevis
Comment 13
2011-06-08 18:25:15 PDT
(In reply to
comment #12
)
> (From update of
attachment 96497
[details]
) > R- until DEPS rolls, please flip back to review? once they've rolled in.
James, can you r+ this and I'll cq+ once the skia deps roll lands?
James Robinson
Comment 14
2011-06-08 19:36:12 PDT
Sure, update the bug when the rolls land. Where are the bugs/codereviews/whatever for the rolls?
Vangelis Kokkevis
Comment 15
2011-06-08 21:41:13 PDT
(In reply to
comment #14
)
> Sure, update the bug when the rolls land. Where are the bugs/codereviews/whatever for the rolls?
Skia rolled past 1551 which is what was needed for this CL to land
http://src.chromium.org/viewvc/chrome?view=rev&revision=88481
James Robinson
Comment 16
2011-06-08 21:43:36 PDT
It has to roll here as well or this will break bots:
http://trac.webkit.org/browser/trunk/Source/WebKit/chromium/DEPS
should just need to increment chromium_rev to 88481 and make sure that already builds. I'll start the process for that if nobody else has started it yet.
James Robinson
Comment 17
2011-06-08 21:47:26 PDT
Rev at
https://bugs.webkit.org/show_bug.cgi?id=62353
, seeing how the EWS bots like it.
James Robinson
Comment 18
2011-06-08 22:25:18 PDT
Roll going through EWS here:
https://bugs.webkit.org/show_bug.cgi?id=62353
James Robinson
Comment 19
2011-06-08 22:53:23 PDT
Comment on
attachment 96497
[details]
Patch OK, Source/WebKit/chromium/DEPS roll landed (hope it sticks)
WebKit Review Bot
Comment 20
2011-06-08 23:40:55 PDT
Comment on
attachment 96497
[details]
Patch Clearing flags on attachment: 96497 Committed
r88429
: <
http://trac.webkit.org/changeset/88429
>
WebKit Review Bot
Comment 21
2011-06-08 23:41:01 PDT
All reviewed patches have been landed. Closing bug.
James Robinson
Comment 22
2011-06-08 23:42:37 PDT
drovering...
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