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
Created attachment 96493 [details] Patch
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/
This looks ok to me, but jamesr and brian should take a look.
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
Created attachment 96497 [details] Patch
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();
Comment on attachment 96497 [details] Patch Are the two fixes for the same thing or not?
Comment on attachment 96497 [details] Patch Attachment 96497 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/8776842
(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.
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
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.
Comment on attachment 96497 [details] Patch R- until DEPS rolls, please flip back to review? once they've rolled in.
(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?
Sure, update the bug when the rolls land. Where are the bugs/codereviews/whatever for the rolls?
(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
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.
Rev at https://bugs.webkit.org/show_bug.cgi?id=62353, seeing how the EWS bots like it.
Roll going through EWS here: https://bugs.webkit.org/show_bug.cgi?id=62353
Comment on attachment 96497 [details] Patch OK, Source/WebKit/chromium/DEPS roll landed (hope it sticks)
Comment on attachment 96497 [details] Patch Clearing flags on attachment: 96497 Committed r88429: <http://trac.webkit.org/changeset/88429>
All reviewed patches have been landed. Closing bug.
drovering...