[webkit-changes] [101241] trunk/Source/WebKit/mac

[Mark Rowe]

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


> On Tue, Nov 29, 2011 at 3:26 AM, Mark Rowe <mrowe at apple.com> 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, hayato at chromium.org wrote:
> 
>> Revision
>> 101241
>> Author
>> hayato at chromium.org
>> 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
>> webkit-changes at lists.webkit.org
>> http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
> 
> 
> 
> 
> -- 
> Hayato

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-changes/attachments/20111128/81810cd6/attachment.html>