I put this FIXME in http://trac.webkit.org/changeset/54205
Created attachment 47925 [details]
I'm not sure if config.h is the best place to put the constant values, but I chose this header because Qt port doesn't include LayoutTestController.h nor DumpRenderTree.h
Comment on attachment 47925 [details]
Good goal. But why not share these in a header as plain old C++ constants? It seems wrong to put these into config.h as macros. Macros are deprecated and should only be used when there is no better solution. And config.h should be kept as simple as possible. We're working longer term to simplify further.
I suggest we just move maxViewWidth and maxViewHeight from DumpRenderTree.mm into LayoutTestController.h and use those in LayoutTestController.cpp.
Created attachment 48129 [details]
Created attachment 48130 [details]
The changelog of Patch v2 was wrong, sorry.
> I suggest we just move maxViewWidth and maxViewHeight from DumpRenderTree.mm
> into LayoutTestController.h and use those in LayoutTestController.cpp.
Thanks for your review! OK, I moved them into LayoutTestController.h. I'm not sure if we can share the value with Qt, but sharing the value among mac, win, and gtk is still nice.
Created attachment 48324 [details]
In Patch v4, maxViewWidth/Height are static member variables of LayoutTestController (in Patch v3 I used static member *function*). I'm not sure which is the preferable way. Also, I'm not sure if we want m_ prefixes for static member variables. Any suggestions will be appreciated, thanks.
Comment on attachment 48324 [details]
Woohoo! Yay for more shared code!
Committed r54942: <http://trac.webkit.org/changeset/54942>