On Nov 28, 2011, at 6:35 PM, Mark Rowe wrote: > > On 2011-11-28, at 18:21, Hayato Ito wrote: > >> Let me explain. This is a quick build fix for >> http://code.google.com/p/chromium/issues/detail?id=105530. >> >> - WebSystemInterface.mm is also used by Chromium Mac build. >> - Chromium uses its own WebKitSytemInterfaces.h header file, which is >> maintained in >> http://src.chromium.org/viewvc/chrome/trunk/src/third_party/apple_webkit/ >> At first I tried to modify Chromium's WebKitSystemInterface.h as in >> http://codereview.chromium.org/8718008/. But I thought it might be better to >> add a fix to WebKitSytemInterface.mm itself than to add a change locally to >> Chromium's WebKitSytemInterface.h. >> >> I guess there is better way to fix this. I am sorry that I should have added >> more descriptive change log. > > The fact that you only have a Leopard library in your directory of > WKSI-related things leaves me to believe that what you *really* wanted to add > was !defined(BUILDING_ON_LEOPARD) rather than PLATFORM(MAC). I'd suggest > making that change after verifying that it doesn't cause you any build issues. > > However, if you're maintaining an independent copy of WebKitSystemInterface.h > then it seems like the crazy layering violation of building a subset of files > from within WebKit/mac should also be stopped. That will mean that people > working on the Mac code don't have to be concerned about breaking Chromium > when changing a file that for all intents and purposes appears to be entirely > specific to the Mac port. > > - Mark >
I agree. I doesn't make sense for chromium to be using files in WebKit/mac. That should fixed ASAP. -Sam > >> On Tue, Nov 29, 2011 at 3:26 AM, Mark Rowe <[email protected]> wrote: >> This change appears to be incorrect. WebSystemInterface.mm is a Mac-only >> file, so it's not clear why you're adding PLATFORM(MAC) to it. >> >> - Mark >> >> On 2011-11-28, at 03:01, [email protected] wrote: >> >>> Revision >>> 101241 >>> Author >>> [email protected] >>> Date >>> 2011-11-28 03:01:26 -0800 (Mon, 28 Nov 2011) >>> Log Message >>> >>> Fix chromium canary build after r101215. >>> Unreviewed. Build fix. >>> >>> * WebCoreSupport/WebSystemInterface.mm: >>> (InitWebCoreSystemInterface): >>> Modified Paths >>> >>> trunk/Source/WebKit/mac/ChangeLog >>> trunk/Source/WebKit/mac/WebCoreSupport/WebSystemInterface.mm >>> Diff >>> >>> Modified: trunk/Source/WebKit/mac/WebCoreSupport/WebSystemInterface.mm >>> (101240 => 101241) >>> >>> >>> --- trunk/Source/WebKit/mac/WebCoreSupport/WebSystemInterface.mm >>> 2011-11-28 10:23:02 UTC (rev 101240) >>> +++ trunk/Source/WebKit/mac/WebCoreSupport/WebSystemInterface.mm >>> 2011-11-28 11:01:26 UTC (rev 101241) >>> @@ -166,7 +166,7 @@ >>> INIT(CreateVMPressureDispatchOnMainQueue); >>> #endif >>> >>> -#if !defined(BUILDING_ON_SNOW_LEOPARD) && !defined(BUILDING_ON_LION) >>> +#if PLATFORM(MAC) && !defined(BUILDING_ON_SNOW_LEOPARD) && >>> !defined(BUILDING_ON_LION) >>> INIT(GetMacOSXVersionString); >>> #endif >>> >>> _______________________________________________ >>> webkit-changes mailing list >>> [email protected] >>> http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes >> >> >> >> >> -- >> Hayato > > _______________________________________________ > webkit-changes mailing list > [email protected] > http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
_______________________________________________ webkit-changes mailing list [email protected] http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
