Bug 120600

Summary: REGRESSION(r154909): caused many crashes on Qt WK2, EFL WK2
Product: WebKit Reporter: Zoltan Arvai <zarvai>
Component: Tools / TestsAssignee: Csaba Osztrogonác <ossy>
Severity: Critical CC: andersca, cmarcelo, commit-queue, dino, jaepark, kadam, kbalazs, kling, luiz, noam, ossy, ryuan.choi, sam, sergio, simon.fraser, yoon, zeno, zherczeg
Priority: P1    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 79668, 120547    
Description Flags
Patch none

Description Zoltan Arvai 2013-09-02 05:18:14 PDT
Many (~80) css3, animations, compositing and transitions tests crashing after the patch.

Crash log said:

crash log for WebProcess (pid <unknown>):
STDOUT: <empty>
STDERR: Received an invalid message "CoordinatedLayerTreeHostProxy.CommitCoordinatedGraphicsState" from the web process.

Comment 1 Csaba Osztrogonác 2013-09-02 05:26:15 PDT
I got same crash with EFL-MiniBrowser on index.hu after scrolling down a little bit. :-/ Reverting r154909 locally solved this crash for me.

It seems there is a seriour bug somewhere in coordinated graphics 
system revealed by r154909.

cc-ing CGFX experts too
Comment 2 Zoltan Arvai 2013-09-02 05:55:57 PDT
Created attachment 210277 [details]

Backtrace for animations/additive-transform-animations.html on Qt WK2.
Comment 3 Csaba Osztrogonác 2013-09-02 06:13:23 PDT
I think I got it, I'll try the following patch immediately:

diff --git a/Source/WebKit2/Shared/CoordinatedGraphics/CoordinatedGraphicsArgumentCoders.cpp b/Source/WebKit2/Shared/CoordinatedGraphics/CoordinatedGraphicsArgumentCoders.cpp
index c277460..4ed58b9 100644
--- a/Source/WebKit2/Shared/CoordinatedGraphics/CoordinatedGraphicsArgumentCoders.cpp
+++ b/Source/WebKit2/Shared/CoordinatedGraphics/CoordinatedGraphicsArgumentCoders.cpp
@@ -727,7 +727,7 @@ bool ArgumentCoder<GraphicsLayerAnimation>::decode(ArgumentDecoder& decoder, Gra
     if (!decoder.decode(keyframesSize))
         return false;
     for (unsigned i = 0; i < keyframesSize; ++i) {
-        float keyTime;
+        double keyTime;
         RefPtr<TimingFunction> timingFunction;
         if (!decoder.decode(keyTime))
             return false;
Comment 4 Csaba Osztrogonác 2013-09-02 06:20:27 PDT
Created attachment 210279 [details]
Comment 5 Csaba Osztrogonác 2013-09-02 06:21:12 PDT
(In reply to comment #4)
> Created an attachment (id=210279) [details]
> Patch

It fixed index.hu with EFL's MiniBrowser. Zoltán, could you 
check if it fixes Qt's layout test crashes too, please?
Comment 6 Csaba Osztrogonác 2013-09-02 06:22:36 PDT
cc-ing WK2 owners too for review
Comment 7 Zoltan Arvai 2013-09-02 06:28:02 PDT
(In reply to comment #5)
> (In reply to comment #4)
> > Created an attachment (id=210279) [details] [details]
> > Patch
> It fixed index.hu with EFL's MiniBrowser. Zoltán, could you 
> check if it fixes Qt's layout test crashes too, please?

Good catch :) Tests running fine with the patch on Qt WK2.
Comment 8 WebKit Commit Bot 2013-09-02 08:42:21 PDT
Comment on attachment 210279 [details]

Clearing flags on attachment: 210279

Committed r154959: <http://trac.webkit.org/changeset/154959>
Comment 9 WebKit Commit Bot 2013-09-02 08:42:25 PDT
All reviewed patches have been landed.  Closing bug.
Comment 10 Sergio Correia (qrwteyrutiyoup) 2013-09-02 17:29:28 PDT
*** Bug 120588 has been marked as a duplicate of this bug. ***