Re: [webkit-dev] Why is ResourceHandle.cpp in WebKit/chromium/src

2010-09-10 Thread Darin Fisher
We use this pattern a lot.  You can see that ResourceHandle is implemented
in terms of WebKit API (WebURLLoader).

The alternative is to introduce extra interfaces between WebCore and WebKit.
 That has the benefit of isolating WebCore as you say, but it adds the cost
of more layering and more indirection.  We already have a lot of layering
and indirection when you consider what lives on the other side of the WebKit
API (in the Chromium repository).

One small thing I have wanted to see done is to move all such classes to a
special subdirectory of WebKit/chromium/src to make it clearer that they are
in this special category.

I've noticed that other ports that have their ResourceHandle implementation
in WebCore/platform resort to using WebKit level classes and interfaces
directly to achieve a similar effect.  Doing so has similar
abstraction-breaking issues.

With the advent of WebKit2, there was a discussion about this topic as well.
 The desire for WebKit2 to support WebCore being built as a separate DSO was
one of the primary reasons given for adding the extra layer of indirection
between WebCore and WebKit for platform level things.  (It has been a
non-goal of the Chromium port to build WebCore as a separate DSO.)

There are a lot more interfaces that would be needed besides just
ResourceHandle{Client}.  At last count it was fairly substantial.  It would
be good to take stock of the entire set before considering a change.

-Darin


On Fri, Sep 10, 2010 at 5:41 PM, Adam Barth  wrote:

> In investigating the loader, I noticed that Chromium's
> ResourceHandle.cpp is in WebKit/chromium/src.  ResourceHandle is a
> WebCore/platform abstract.  Most of the other implementations of
> ResourceHandle are in
> WebCore/platform/mumble/ResourceHandleMumble.cpp.
>
> Is there a reason why Chromium uses a different design?  Normally, we
> need a use a virtual function to jump from WebCore to WebKit/chromium.
>  It looks like we're avoiding that in this case, at the cost of
> blurring the layer boundaries.
>
> Adam
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


[webkit-dev] Why is ResourceHandle.cpp in WebKit/chromium/src

2010-09-10 Thread Adam Barth
In investigating the loader, I noticed that Chromium's
ResourceHandle.cpp is in WebKit/chromium/src.  ResourceHandle is a
WebCore/platform abstract.  Most of the other implementations of
ResourceHandle are in
WebCore/platform/mumble/ResourceHandleMumble.cpp.

Is there a reason why Chromium uses a different design?  Normally, we
need a use a virtual function to jump from WebCore to WebKit/chromium.
 It looks like we're avoiding that in this case, at the cost of
blurring the layer boundaries.

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


Re: [webkit-dev] Why is the Windows WebKit2 bot slower than the SnowLeopard WebKit2 bot? (was: Re: Leopard Debug and Windows Debug test bots are too slow)

2010-09-10 Thread Sam Weinig
On Fri, Sep 10, 2010 at 5:15 AM, Adam Roben  wrote:

> On Sep 10, 2010, at 8:11 AM, Adam Roben wrote:
>
> > It's interesting that ~45 minutes elapse between builds for the Windows
> WebKit2 bot, but only ~18 minutes elapse between builds for the SnowLeopard
> WebKit2 bot. We should figure out where the extra time is being spent!
>
> Looking at <
> http://build.webkit.org/builders/Windows%20Debug%20%28WebKit2%20Tests%29/builds/633>
> vs. <
> http://build.webkit.org/builders/SnowLeopard%20Intel%20Release%20%28WebKit2%20Tests%29/builds/1624
> >:
>
> jscore-test took 267 seconds on Windows vs. 18 seconds on SnowLeopard
> layout-test took 1711 seconds on Windows vs. 601 seconds on SnowLeopard
>
> That adds up to an extra 22.5 minutes on Windows, which explains most of
> the difference between the two.
>
> Perhaps the biggest issue is that Windows is using Debug builds, while Mac
> is using Release? Maybe we should change the Windows WebKit2 bot to use
> Release builds.
>
>
I think changing to use release would make sense.

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


[webkit-dev] Test on all platforms

2010-09-10 Thread François Sausset
Hi all,

I'm working on a bug concerning xHeight() for fonts without an "x" glyph:
https://bugs.webkit.org/show_bug.cgi?id=41535

I spotted it on the mac platform with "Apple Symbols" font, but other platforms 
could also be affected: a lot of platforms are making a guess of 0.56 * 
ascent() for xHeight when no "x" glyph exists, but it is probably erroneous and 
should be 2/3.

Thus I need some volunteers to test the following layout test on other 
platforms:
https://bugs.webkit.org/attachment.cgi?id=66398
The green square should stay properly aligned even when zooming and STIX fonts 
should be disabled if installed.

This test is perhaps not sufficient to spot the problem as it uses MathML that 
is not enabled on all platforms and it uses CSS declaration "Symbol" for font 
that then differs on each platform. For instance, "Apple Symbols" font is used 
on the mac platform. I don't know for other platforms.

If some people have time to test it on different platforms (apart from the mac 
one) with a font without a "x" glyph, it would be great!


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


Re: [webkit-dev] [webkit-changes] [67188] trunk/WebKit/chromium

2010-09-10 Thread Adam Barth
Interesting.  If you look at the patch:

https://bugs.webkit.org/attachment.cgi?id=67061&action=prettypatch

You'll see a stray minus line in the ChangeLog.  That must have
confused the bot.  We'll add another validation step to prevent this
case.  Thanks for the report.

Adam


On Fri, Sep 10, 2010 at 9:53 AM, Alexey Proskuryakov  wrote:
>
> Do we know why this (incorrect commit messages) happens? With the new
> support for committer name rewriting, this has become even more confusing.
> - WBR, Alexey Proskuryakov
> 10.09.2010, в 4:36, da...@apple.com написал(а):
>
> Revision 67188 Author da...@apple.com Date 2010-09-10 04:36:40 -0700 (Fri,
> 10 Sep 2010)
>
> Log Message
>
> 2010-09-08  Darin Adler  
>
> Reviewed by Adam Barth.
>
> Move functions from Frame to Editor as planned
> https://bugs.webkit.org/show_bug.cgi?id=45218
>
> * src/ContextMenuClientImpl.cpp:
> (WebKit::selectMisspelledWord):
> (WebKit::ContextMenuClientImpl::getCustomMenuFromDefaultItems):
> * src/WebFrameImpl.cpp:
> (WebKit::WebFrameImpl::find):
> (WebKit::WebFrameImpl::stopFinding):
> (WebKit::WebFrameImpl::scopeStringMatches):
> * src/WebViewImpl.cpp:
> (WebKit::WebViewImpl::caretOrSelectionBounds):
> Changed call sites to use editor().
>
> Modified Paths
>
> trunk/WebKit/chromium/ChangeLog
> trunk/WebKit/chromium/public/WebDOMEvent.h
>
> Diff
>
> Modified: trunk/WebKit/chromium/ChangeLog (67187 => 67188)
>
> --- trunk/WebKit/chromium/ChangeLog   2010-09-10 10:57:49 UTC (rev 67187)
> +++ trunk/WebKit/chromium/ChangeLog   2010-09-10 11:36:40 UTC (rev 67188)
> @@ -53,13 +53,22 @@
>  * src/WebKit.cpp:
>  (WebKit::areLayoutTestImagesOpaque): Make linux match windows.
>
> +2010-09-10  Jay Civelli  
> +
> +Reviewed by Darin Fisher.
> +
> +Add the destructor to WebDOMEvent to prevent a leak.
> +https://bugs.webkit.org/show_bug.cgi?id=45287
> +
> +* public/WebDOMEvent.h:
> +(WebKit::WebDOMEvent::~WebDOMEvent):
> +
>  2010-09-09  Chris Guillory  
>
>  Reviewed by Chris Fleizach.
>
>  Add methods used to determine accessibility state.
>  https://bugs.webkit.org/show_bug.cgi?id=45434
> -
>
>  * public/WebAccessibilityObject.h:
>  * src/WebAccessibilityObject.cpp:
>
> Modified: trunk/WebKit/chromium/public/WebDOMEvent.h (67187 => 67188)
>
> --- trunk/WebKit/chromium/public/WebDOMEvent.h2010-09-10 10:57:49 UTC 
> (rev
> 67187)
> +++ trunk/WebKit/chromium/public/WebDOMEvent.h2010-09-10 11:36:40 UTC 
> (rev
> 67188)
> @@ -50,6 +50,8 @@
>  BubblingPhase  = 3
>  };
>
> +~WebDOMEvent() { reset(); }
> +
>  WebDOMEvent() : m_private(0) { }
>  WebDOMEvent(const WebDOMEvent& e) : m_private(0) { assign(e); }
>  WebDOMEvent& operator=(const WebDOMEvent& e)
>
> ___
> webkit-changes mailing list
> webkit-chan...@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
>
>
>
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev


Re: [webkit-dev] [webkit-changes] [67188] trunk/WebKit/chromium

2010-09-10 Thread Darin Adler
On Sep 10, 2010, at 9:53 AM, Alexey Proskuryakov wrote:

> Do we know why this (incorrect commit messages) happens?

I do not know, but have a suspicion.

I suspect it’s a downstream symptom of incorrect merging of the change log. 
Perhaps webkit-patch could enforce a rule that changes to the change log need 
to be at the top of the file.

-- Darin

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


Re: [webkit-dev] [webkit-changes] [67188] trunk/WebKit/chromium

2010-09-10 Thread Alexey Proskuryakov

Do we know why this (incorrect commit messages) happens? With the new support 
for committer name rewriting, this has become even more confusing.

- WBR, Alexey Proskuryakov

10.09.2010, в 4:36, da...@apple.com написал(а):

> Revision
> 67188
> Author
> da...@apple.com
> Date
> 2010-09-10 04:36:40 -0700 (Fri, 10 Sep 2010)
> Log Message
> 
> 2010-09-08  Darin Adler  
> 
> Reviewed by Adam Barth.
> 
> Move functions from Frame to Editor as planned
> https://bugs.webkit.org/show_bug.cgi?id=45218
> 
> * src/ContextMenuClientImpl.cpp:
> (WebKit::selectMisspelledWord):
> (WebKit::ContextMenuClientImpl::getCustomMenuFromDefaultItems):
> * src/WebFrameImpl.cpp:
> (WebKit::WebFrameImpl::find):
> (WebKit::WebFrameImpl::stopFinding):
> (WebKit::WebFrameImpl::scopeStringMatches):
> * src/WebViewImpl.cpp:
> (WebKit::WebViewImpl::caretOrSelectionBounds):
> Changed call sites to use editor().
> Modified Paths
> 
> trunk/WebKit/chromium/ChangeLog
> trunk/WebKit/chromium/public/WebDOMEvent.h
> Diff
> 
> Modified: trunk/WebKit/chromium/ChangeLog (67187 => 67188)
> 
> --- trunk/WebKit/chromium/ChangeLog   2010-09-10 10:57:49 UTC (rev 67187)
> +++ trunk/WebKit/chromium/ChangeLog   2010-09-10 11:36:40 UTC (rev 67188)
> @@ -53,13 +53,22 @@
>  * src/WebKit.cpp:
>  (WebKit::areLayoutTestImagesOpaque): Make linux match windows.
>  
> +2010-09-10  Jay Civelli  
> +
> +Reviewed by Darin Fisher.
> +
> +Add the destructor to WebDOMEvent to prevent a leak.
> +https://bugs.webkit.org/show_bug.cgi?id=45287
> +
> +* public/WebDOMEvent.h:
> +(WebKit::WebDOMEvent::~WebDOMEvent):
> +
>  2010-09-09  Chris Guillory  
>  
>  Reviewed by Chris Fleizach.
>  
>  Add methods used to determine accessibility state.
>  https://bugs.webkit.org/show_bug.cgi?id=45434
> -
>  
>  * public/WebAccessibilityObject.h:
>  * src/WebAccessibilityObject.cpp:
> Modified: trunk/WebKit/chromium/public/WebDOMEvent.h (67187 => 67188)
> 
> --- trunk/WebKit/chromium/public/WebDOMEvent.h2010-09-10 10:57:49 UTC 
> (rev 67187)
> +++ trunk/WebKit/chromium/public/WebDOMEvent.h2010-09-10 11:36:40 UTC 
> (rev 67188)
> @@ -50,6 +50,8 @@
>  BubblingPhase  = 3
>  };
>  
> +~WebDOMEvent() { reset(); }
> +
>  WebDOMEvent() : m_private(0) { }
>  WebDOMEvent(const WebDOMEvent& e) : m_private(0) { assign(e); }
>  WebDOMEvent& operator=(const WebDOMEvent& e)
> ___
> webkit-changes mailing list
> webkit-chan...@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes


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


[webkit-dev] Why is the Windows WebKit2 bot slower than the SnowLeopard WebKit2 bot? (was: Re: Leopard Debug and Windows Debug test bots are too slow)

2010-09-10 Thread Adam Roben
On Sep 10, 2010, at 8:11 AM, Adam Roben wrote:

> It's interesting that ~45 minutes elapse between builds for the Windows 
> WebKit2 bot, but only ~18 minutes elapse between builds for the SnowLeopard 
> WebKit2 bot. We should figure out where the extra time is being spent!

Looking at 

 vs. 
:

jscore-test took 267 seconds on Windows vs. 18 seconds on SnowLeopard
layout-test took 1711 seconds on Windows vs. 601 seconds on SnowLeopard

That adds up to an extra 22.5 minutes on Windows, which explains most of the 
difference between the two.

Perhaps the biggest issue is that Windows is using Debug builds, while Mac is 
using Release? Maybe we should change the Windows WebKit2 bot to use Release 
builds.

-Adam

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


Re: [webkit-dev] Leopard Debug and Windows Debug test bots are too slow

2010-09-10 Thread Adam Roben
On Sep 10, 2010, at 2:20 AM, Ryosuke Niwa wrote:

> Windows Debug (WebKit2 Tests)

I cleared out most of the old pending builds for this bot to try to help it 
catch up.

It's interesting that ~45 minutes elapse between builds for the Windows WebKit2 
bot, but only ~18 minutes elapse between builds for the SnowLeopard WebKit2 
bot. We should figure out where the extra time is being spent!

-Adam

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