Bug 45133 – [chromium] Make a public flag for how DRT generates bitmaps on Linux

• Component: New Bugs
• Status: RESOLVED FIXED

[Tony Chang]

[chromium] Make a public flag for how DRT generates bitmaps on Linux

[Tony Chang]

Created attachment 66413 [details]
Patch

[Tony Chang]

I'm trying to fix http://code.google.com/p/chromium/issues/detail?id=21386.  Here is my plan:

1) Expose a variable in WebKit that determines whether or not to call makeOpaque (this patch).
2) In Chromium, switch test_shell to use that flag.
3) In WebKit, flip the flag and checkin a bunch of new checksums.
4) In Chromium, remove the flag in test_shell and have us always call makeOpaque on Linux and Windows.
5) In WebKit, remove the flag in DRT and remove the variable added in (1).

I'm not too excited about where I put the variable or how I named it.  Suggestions welcome.

[Darin Fisher (:fishd, Google)]

Comment on attachment 66413 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=66413&action=prettypatch

> WebKit/chromium/public/DumpRenderTreeDefaults.h:36
> +extern const bool makeLinuxChecksumOpaque;
it seems like this would be better done without a globally exported variable.
how about a setter and a getter in the usual fashion?  also, why the DumpRenderTree
namespace in the WebKit API's public directory?

can you just place these methods in WebKit.h next to setLayoutTestMode/layoutTestMode?

[Tony Chang]

Created attachment 66424 [details]
Patch

[Tony Chang]

(In reply to comment #3)
> can you just place these methods in WebKit.h next to setLayoutTestMode/layoutTestMode?

I didn't see WebKit.h.  Yes, this is a much better place for this.

[Tony Chang]

Darin, can you take a look at the revised patch?

[Darin Fisher (:fishd, Google)]

Comment on attachment 66424 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=66424&action=prettypatch

> WebKit/chromium/public/WebKit.h:60
> +WEBKIT_API bool makeLinuxChecksumOpaque();
this method name sounds like something that is a command.  hey you, go make it opaque.
but the signature looks more like a getter.  how about "isLinuxChecksumOpaque"?

that said, i'm not sure what it means for a checksum to be opaque.  hmm...  how about
"areLayoutTestImagesOpaque"?

R=me modulo this re-naming suggestion

[Darin Fisher (:fishd, Google)]

Also, if the comment could say that this flag describes the format of layout test image baselines that would be helpful.

[Tony Chang]

Committed r66914: <http://trac.webkit.org/changeset/66914>