[Webkit-unassigned] [Bug 26457] Build DumpRenderTree under Cairo

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jul 2 11:17:03 PDT 2009


https://bugs.webkit.org/show_bug.cgi?id=26457





------- Comment #9 from bfulgham at webkit.org  2009-07-02 11:17 PDT -------
(In reply to comment #8)
> (From update of attachment 32087 [review])
> > +2009-06-30  U-bfulgham-PC\bfulgham  <bfulgham at webkit.org>
> 
> Looks like you need to set REAL_NAME on this computer.

Fixed (permanently!)

> > +        * DumpRenderTree/PixelDumpSupport.cpp: Added.
> > +        (dumpWebViewAsPixelsAndCompareWithExpected):
> > +        (printPNG):
[...]
> > +        * DumpRenderTree/win/PixelDumpSupportWin.cpp:
> > +        (createBitmapContextFromWebView):
> 
> It would be good to add more detailed comments about what you changed next to
> each function/file.

Done

> I'm not sure I understand why we need to include any port-specific headers
> here. Can the declarations of the required functions move to
> PixelDumpSupport.h?

Done.  I initially thought this header was included in other DumpRenderTree
variants, but since
it was already required these headers it shouldn't be a problem.

> > +#if PLATFORM(WIN)
> > +static const CFStringRef kUTTypePNG = CFSTR("public.png");
> > +#endif
> 
> I don't think this is needed in PixelDumpSupportCairo.

You are right.  Removed.

> Please remove the spaces after the function names.
> 
> pixeldata.data() would be better than &pixeldata[0]. pixeldata should probably
> be renamed to pixelData.

Done.

> > +void computeMD5HashStringForBitmapContext(RefPtr<BitmapContext> context, char hashString[33])
> 
> context's type should be BitmapContext*.

Done.

> > +    // We need to swap the bytes to ensure consistent hashes independently of endianness
> 
> Your code doesn't seem to take endianness into account. Should this turn into a
> FIXME? Be removed entirely?

This was copied from the CG logic.  In theory, someone might run this on a
PowerPC Linux box so this
might be an issue, but I'm not sure how much of an issue this is (no one else
is using it yet, and Windows
is x86-only).

Removing the comment; this will only show up as a problem if the GTK port
starts using this code on PPC or similar systems.  We can address it if that
ever happens.

> > +void dumpBitmap(RefPtr<BitmapContext> context)
> 
> context's type should be BitmapContext*.

Done.

> > \ No newline at end of file

Done.

> > +    RetainPtr<CairoContextRef> m_context;
> 
> It seems unlikely that RetainPtr is the right smart pointer for holding a
> CairoContextRef. Is it really right to use CFRetain/CFRelease with a
> CairoContextRef?

Nope.  This is now fixed.

> > +#endif // PixelDumpSupportCairo_h
> > \ No newline at end of file

Done. 


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.



More information about the webkit-unassigned mailing list