Bug 58173

Summary: [Qt] embed checksums in PNGs written by Qt-DRT
Product: WebKit Reporter: Tony Chang <tony>
Component: New BugsAssignee: Tony Chang <tony>
Status: RESOLVED FIXED    
Severity: Normal CC: ossy
Priority: P2 Keywords: Qt, QtTriaged
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Bug Depends on: 58243    
Bug Blocks: 56286    
Attachments:
Description Flags
Patch
none
fix license
none
crash log
none
Patch kling: review+

Tony Chang
Reported 2011-04-08 16:19:02 PDT
[qt] embed checksums in PNGs written by DRT QT
Attachments
Patch (13.99 KB, patch)
2011-04-08 16:36 PDT, Tony Chang 		no flags
DetailsFormatted DiffDiff
fix license (13.87 KB, patch)
2011-04-08 16:41 PDT, Tony Chang 		no flags
DetailsFormatted DiffDiff
crash log (8.04 KB, text/plain)
2011-04-11 05:49 PDT, Csaba Osztrogonác 		no flags
Details
Patch (1.14 KB, patch)
2011-04-12 11:37 PDT, Tony Chang 		kling: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Tony Chang
Comment 1 2011-04-08 16:36:03 PDT
Created attachment 88894 [details] Patch
Tony Chang
Comment 2 2011-04-08 16:41:35 PDT
Created attachment 88895 [details] fix license
Eric Seidel (no email)
Comment 3 2011-04-10 15:57:55 PDT
Comment on attachment 88895 [details] fix license View in context: https://bugs.webkit.org/attachment.cgi?id=88895&action=review > Tools/DumpRenderTree/CyclicRedundancyCheck.cpp:36 > +static void makeCrcTable(unsigned int crcTable[256]) Why not just "unsigned"? I don't think the "int" adds anything here. > Tools/DumpRenderTree/qt/DumpRenderTree.pro:32 > + ../CyclicRedundancyCheck.h \ Does the .pro normally use ../ relative paths?
Csaba Osztrogonác
Comment 4 2011-04-11 04:57:58 PDT
Comment on attachment 88895 [details] fix license View in context: https://bugs.webkit.org/attachment.cgi?id=88895&action=review Unfortunately DRT crashes sometimes with this patch. r- now due to crashes. I will check it, and try to help fixing it. >> Tools/DumpRenderTree/qt/DumpRenderTree.pro:32 >> + ../CyclicRedundancyCheck.h \ > > Does the .pro normally use ../ relative paths? It works, but we usually add the directory to INCLUDEPATH instead of using relative header paths.
Csaba Osztrogonác
Comment 5 2011-04-11 04:58:41 PDT
*** Bug 58021 has been marked as a duplicate of this bug. ***
Csaba Osztrogonác
Comment 6 2011-04-11 05:49:15 PDT
Created attachment 88995 [details] crash log I got this crash log with the following command: $ WebKitBuild/Release/bin/DumpRenderTree LayoutTests/svg/W3C-SVG-1.1-SE/paths-dom-02-f.svg I can't reproduce this crash in debug mode and/or without your patch. If we don't pass --pixel-tests, your code doesn't run. I don't understand what and why cause this strange crash.
Tony Chang
Comment 7 2011-04-11 09:54:51 PDT
Splitting the 64bit fix into a separate bug while I look into the 64bit test crash.
Tony Chang
Comment 8 2011-04-11 10:48:30 PDT
(In reply to comment #6) > Created an attachment (id=88995) [details] > crash log > > I got this crash log with the following command: > $ WebKitBuild/Release/bin/DumpRenderTree LayoutTests/svg/W3C-SVG-1.1-SE/paths-dom-02-f.svg > > I can't reproduce this crash in debug mode and/or without your patch. > > If we don't pass --pixel-tests, your code doesn't run. > I don't understand what and why cause this strange crash. I suspect this crash has to do with including FastMalloc.cpp. When writing this patch, I wasn't sure if it was OK to use wtf types (Vector.h) in QT's DRT. Up until now, DRT hasn't had any dependencies on wtf except for Assertions.h. If I can't use wtf, I could either switch to std::vector or duplicate the code (might be simpler since other code probably wants to use WTF::Vector).
Tony Chang
Comment 9 2011-04-12 11:37:12 PDT
Created attachment 89233 [details] Patch
Tony Chang
Comment 10 2011-04-12 11:38:04 PDT
(In reply to comment #9) > Created an attachment (id=89233) [details] > Patch QT++ for exposing comment writing/reading in the QImage API.
Andreas Kling
Comment 11 2011-04-12 12:01:46 PDT
Comment on attachment 89233 [details] Patch Cool! r=me
Tony Chang
Comment 12 2011-04-12 12:10:53 PDT
Committed r83619: <http://trac.webkit.org/changeset/83619>
Note You need to log in before you can comment on or make changes to this bug.