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

[Sam Weinig]

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 <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
> 
> _______________________________________________
> webkit-changes mailing list
> webkit-changes at lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

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