Created attachment 39542 [details] The patch The existing Chromium Mac render theme code is forked from an ancient version of RenderThemeMac. This patch brings that file up to date with the current version for ease of maintenance. (To be investigated: determining if we can merge the two files so they don't have to be kept in sync.) Please see http://codereview.chromium.org/174242 for the initial review.
A quick comment: the diff is complete hash. It's best to compare a patched RenderThemeChromiumMac to RenderThemeMac.
Created attachment 39543 [details] Updated patch; the .gypi file caught unwanted changes
FYI, general behavior seems to be that you can mark cq=? if you want to make sure the reviewer sets cq+ when reviewing.
Comment on attachment 39543 [details] Updated patch; the .gypi file caught unwanted changes Can you please: * include bug URL * use svn cp to keep the history and minimize the amount of perceived changes made?
prepare-ChangeLog --bug 29243 will automatically fill in the bug stuff for you. see prepare-ChangeLog --help
Created attachment 39601 [details] New patch
(In reply to comment #4) > (From update of attachment 39543 [details]) > Can you please: > > * include bug URL > * use svn cp to keep the history and minimize the amount of perceived changes > made? Can you keep the svn cp status through a patch? I didn't think it was possible and would need someone to help me land this anyway.
(In reply to comment #7) > (In reply to comment #4) > > (From update of attachment 39543 [details] [details]) > > Can you please: > > > > * include bug URL > > * use svn cp to keep the history and minimize the amount of perceived changes > > made? > > Can you keep the svn cp status through a patch? I didn't think it was possible > and would need someone to help me land this anyway. No, but it makes reviewing and landing it a heckuva lot easier :). If you're looking for a rubberstamp, you've got it. But whoever lands this would have to cry tears of pain trying to svn cp, then apply this patch.
Created attachment 39603 [details] Patch with svn cp used to mark origins of files OK, used svn cp to mark files' origins.
Doesn't svn-create-patch store copy information? I thought it did. Also, if there are any tears-of-pain needed, then the commit-queue won't be of use to you, because it doesn't know how to cry. It just knows how to make you cry when it rejects your patch. :)
Comment on attachment 39603 [details] Patch with svn cp used to mark origins of files > Index: WebCore/ChangeLog ... > + http://crbug.com/19604 No need for the crbug reference since you mention the bugs.webkit.org link. > Index: WebCore/platform/graphics/FloatPoint.h > +#if (PLATFORM(MAC) && !defined(NSGEOMETRY_TYPES_SAME_AS_CGGEOMETRY_TYPES)) \ > + || (PLATFORM(CHROMIUM) && PLATFORM(DARWIN)) nit: indent by 8 spaces instead of 6. normal indent is 4 spaces, so 2x that seems appropriate for a line continuation. > Index: WebCore/platform/graphics/FloatRect.h ... > +#if (PLATFORM(MAC) && !defined(NSGEOMETRY_TYPES_SAME_AS_CGGEOMETRY_TYPES)) \ > + || (PLATFORM(CHROMIUM) && PLATFORM(DARWIN)) ditto > Index: WebCore/platform/graphics/FloatSize.h ... > +#if (PLATFORM(MAC) && !defined(NSGEOMETRY_TYPES_SAME_AS_CGGEOMETRY_TYPES)) \ > + || (PLATFORM(CHROMIUM) && PLATFORM(DARWIN)) ditto > Index: WebCore/platform/graphics/IntRect.h ... > +#if (PLATFORM(MAC) && !defined(NSGEOMETRY_TYPES_SAME_AS_CGGEOMETRY_TYPES)) \ > + || (PLATFORM(CHROMIUM) && PLATFORM(DARWIN)) ... > +#if (PLATFORM(MAC) && !defined(NSGEOMETRY_TYPES_SAME_AS_CGGEOMETRY_TYPES)) \ > + || (PLATFORM(CHROMIUM) && PLATFORM(DARWIN)) ditto
Created attachment 39605 [details] Update for fishd's comments
Comment on attachment 39605 [details] Update for fishd's comments Needs downstream change.
(In reply to comment #13) > (From update of attachment 39605 [details]) > Needs downstream change. Dmitri, I can coordinate with the WebKit gardener. Can we land this?
Landed as http://trac.webkit.org/changeset/48469.
Created attachment 39708 [details] Quick fix to make it compile Oops, there was a line that referred to a non-existent member in the media theme drawing routines. The media drawing is TBD, so for now we're just pulling it out.
Comment on attachment 39708 [details] Quick fix to make it compile r=me.
Fix landed as http://trac.webkit.org/changeset/48481.