[webkit-dev] WinCE Font work

2009-08-05 Thread Dan Bernstein

Hi,

Today while looking at the patch at https://bugs.webkit.org/show_bug.cgi?id=28021 
 I noticed that the WinCE version of FontCustomPlatformData includes  
CachedFont.h and uses CachedFont.


It is incorrect for classes in the platform layer to have any  
knowledge of higher WebCore layers (essentially, anything outside the  
platform hierarchy). In this case, it also necessitated a platform  
difference in the createFontCustomPlatformData call site in  
CachedFont::ensureCustomFontData(), which is how I noticed it.


I think once you should correct this and any other layering violations  
in the WinCE platform layer, and only then submit patches with changes  
to cross-platform code, if any such changes are still deemed  
necessary. I don’t think https://bugs.webkit.org/show_bug.cgi? 
id=27734 and https://bugs.webkit.org/show_bug.cgi?id=28021 should  
be reviewed until then.


Thanks,
—Dan
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] WinCE Font work

2009-08-05 Thread George Staikos


Yes I agree; I was just going to r- this patch in fact.

Some of these problems arose from the fact that we started this work  
nearly two years ago and were working outside of SVN so it's taking  
some time to realign.


On 5-Aug-09, at 2:13 PM, Dan Bernstein wrote:


Hi,

Today while looking at the patch at https://bugs.webkit.org/ 
show_bug.cgi?id=28021 I noticed that the WinCE version of  
FontCustomPlatformData includes CachedFont.h and uses CachedFont.


It is incorrect for classes in the platform layer to have any  
knowledge of higher WebCore layers (essentially, anything outside  
the platform hierarchy). In this case, it also necessitated a  
platform difference in the createFontCustomPlatformData call site  
in CachedFont::ensureCustomFontData(), which is how I noticed it.


I think once you should correct this and any other layering  
violations in the WinCE platform layer, and only then submit  
patches with changes to cross-platform code, if any such changes  
are still deemed necessary. I don’t think https://bugs.webkit.org/ 
show_bug.cgi?id=27734 and https://bugs.webkit.org/show_bug.cgi? 
id=28021 should be reviewed until then.


Thanks,
—Dan


--
George Staikos
Torch Mobile Inc.
http://www.torchmobile.com/

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev