Re: [webkit-dev] Touch operation corrupts screen when specifying other than overflow:visible in css
Hi, my team made the workaround to fix this problem and has submitted it
as a patch to get a review.
https://bugs.webkit.org/show_bug.cgi?id=99842
Index: Source/WebCore/platform/graphics/win/GraphicsContextCairoWin.cpp
===
--- Source/WebKit/win/WebView.cpp (revision 132968)
+++ Source/WebKit/win/WebView.cpp (working copy)
@@ -1663,7 +1663,7 @@
return false;
// We negate here since panning up moves the content up, but moves the
scrollbar down.
-
m_gestureTargetNode-renderer()-enclosingLayer()-scrollByRecursively(IntSize(-deltaX,
-deltaY));
+
m_gestureTargetNode-renderer()-enclosingLayer()-scrollByRecursively(IntSize(-deltaX,
-deltaY), WebCore::RenderLayer::ScrollOffsetClamping::ScrollOffsetClamped);
if (!(UpdatePanningFeedbackPtr() BeginPanningFeedbackPtr()
EndPanningFeedbackPtr())) {
CloseGestureInfoHandlePtr()(gestureHandle);
Hideki
***
Hideki Yoshida
Embedded Software Division
NEC System Technologies, Ltd.
E-MAIL:yoshida-...@necst.nec.co.jp
TEL:+81-78-991-5566FAX:+81-78-991-5568
***
Hi, I filed this problem to https://bugs.webkit.org with
test case as an HTML file after I was suggested to do so.
Bug id 99842.
I am trying to find a workaround to resolve this problem.
If someone has it, post it, please.
Hi,
On a windows 7 tablet, PAN operation(=scroll) causes
corruption of screen.
Does anybody know how to resolve this or have the fix?
How to reproduce.
1. Prepare a HTML contents which have an element specifying
other than visible to the property overflow in css.
2. Load the contents with webkit
3. Operate the touch operaion, PAN on the element.
Problem
The content in the element protrudes outside the placeholder
for it and can disappear.
The build version
Webkit.exe on r131112 for Nightly builds
We guess Source\WebKit\win\WebView.cpp has some bugs on this
issue.
Here is the sample contents to reproduce problem. You will see the
problem if you PAN on the field for overflow:auto.
--
HTML
HEADTITLEpan with css:overflow/TITLE/HEAD
BODY
font size=+2
div style=border: 2px solid blue; padding: 5px 5px 5px 5px;
overflow:visible;
overflow:visible
/div
br
div style=border: 2px solid red; padding: 5px 5px 5px 5px;
overflow:auto;
overflow:auto
/div
/font
/BODY
/HTML
--
Hideki
***
Hideki Yoshida
Embedded Software Division
NEC System Technologies, Ltd.
E-MAIL:yoshida-...@necst.nec.co.jp
***
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev
[webkit-dev] About text decoration wavy style implementation on Skia platform
Hi all, I am implementing the CSS3 text decoration style wavy for Skia platform [3], and I would like to know a few things (please let me know if this is not the right mailing list to ask this): - There is already a spelling line style implementation which mimics wavy style behavior, only with its own colors and theme. While Skia uses a custom, inline bitmap for rendering the red waves, other toolkits such as Cairo and Qt implements a function called drawErrorUnderline which renders the waves based on a given bounding rectangle. These are triggered by the drawLineForDocumentMarker function. - Individual implementations of the wavy style are popping up for each platform (ie. Qt [1] and Cairo [2] already have their implementations pending review). I see we can go both ways here: 1. Adapt the already implemented functions to support custom colors (as defined by -webkit-text-decoration-color, for example). While this looks easier on Qt and Cairo platforms, since drawErrorUnderline already gives everything we need, we would need to find a consensus on the custom bitmap usage for Skia platform. Of course, this would include updating the Qt [1] and Cairo [2] patches to support this approach. 2. Continue with individual implementations of the wavy style as we're doing now. I'm afraid this option would involve code duplication. IMO we should adopt first option, rename the drawErrorUnderline to drawWavyLine (as suggested by KyungTae Kim), and find a solution for the Skia custom bitmap approach. What do you guys think? Links: [1] https://bugs.webkit.org/show_bug.cgi?id=93507 [2] https://bugs.webkit.org/show_bug.cgi?id=94110 [3] https://bugs.webkit.org/show_bug.cgi?id=93509 Best, -- Bruno de Oliveira Abinader Software Engineer @ basysKom GmbH WebKit committer / Nokia Certified Qt Specialist ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] About text decoration wavy style implementation on Skia platform
On Mon, Nov 5, 2012 at 5:18 AM, Bruno Abinader brunoabina...@gmail.com wrote: Hi all, I am implementing the CSS3 text decoration style wavy for Skia platform [3], and I would like to know a few things (please let me know if this is not the right mailing list to ask this): - There is already a spelling line style implementation which mimics wavy style behavior, only with its own colors and theme. While Skia uses a custom, inline bitmap for rendering the red waves, other toolkits such as Cairo and Qt implements a function called drawErrorUnderline which renders the waves based on a given bounding rectangle. These are triggered by the drawLineForDocumentMarker function. - Individual implementations of the wavy style are popping up for each platform (ie. Qt [1] and Cairo [2] already have their implementations pending review). I see we can go both ways here: 1. Adapt the already implemented functions to support custom colors (as defined by -webkit-text-decoration-color, for example). While this looks easier on Qt and Cairo platforms, since drawErrorUnderline already gives everything we need, we would need to find a consensus on the custom bitmap usage for Skia platform. Of course, this would include updating the Qt [1] and Cairo [2] patches to support this approach. 2. Continue with individual implementations of the wavy style as we're doing now. I'm afraid this option would involve code duplication. IMO we should adopt first option, rename the drawErrorUnderline to drawWavyLine (as suggested by KyungTae Kim), and find a solution for the Skia custom bitmap approach. What do you guys think? The spelling underline on OS X is not a wavy line. If you end up changing the skia code, please make sure the chromium/mac spelling underline doesn't become a wavy line. Nico Links: [1] https://bugs.webkit.org/show_bug.cgi?id=93507 [2] https://bugs.webkit.org/show_bug.cgi?id=94110 [3] https://bugs.webkit.org/show_bug.cgi?id=93509 Best, -- Bruno de Oliveira Abinader Software Engineer @ basysKom GmbH WebKit committer / Nokia Certified Qt Specialist ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] About text decoration wavy style implementation on Skia platform
2012/11/5 Nico Weber tha...@chromium.org The spelling underline on OS X is not a wavy line. If you end up changing the skia code, please make sure the chromium/mac spelling underline doesn't become a wavy line. Nico Interesting point. Said this, I'd say it is so far a matter of porting the drawErrorUnderline() code from Cairo/Qt into Skia and use it instead of modifying drawLineForDocumentMarker. For Qt/Cairo implementations, instead of duplicating code, just make use of the already implemented function with attention to use proper text decoration colors. -- Bruno de Oliveira Abinader Software Engineer @ basysKom GmbH WebKit committer / Nokia Certified Qt Specialist ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] …Inlines.h vs …InlineMethods.h
On Nov 5, 2012, at 8:47 AM, Geoffrey Garen gga...@apple.com wrote: (2) Adopt the convention that any class using *Inlines.h defines all inline functions defined out-of-line in *Inlines.h This is a proposal I don’t agree with. Making a firm rule here here means that every larger inline function in one of these classes becomes a special case from a client point of view, where previously only functions actually affected by complex dependencies were special cases. The proposed design requires adding a FooInlines.h include to source files that use that function, when any function moves into FooInlines.h. This can happen any time a function is made inline or when a short inline function gets longer. This could affect any file that was using that function but was not previously using one of the other functions in FooInlines.h. That is a burden I would prefer the project not take on; it would make refactoring more difficult. Further, this proposal does not solve the problem of getting this wrong if we don’t actually try an appropriate build. As with today’s similar problems, include mistakes won’t be noticed if we are compiling with inlining off as we do on, say, Mac debug builds. Choosing what to put in FooInlines.h based on header dependencies hurts my brain. In the name of making it easier to write the headers correctly and slightly easier to find functions, this will make it much more common to have to include a separate header file, without a clear rule for when you need that include. The rule today is that we can just include the class’s header and use functions from that header, with a limited number of exceptions where we have to include another file. Putting more functions into separate files will spread this to other functions, making the problems with it worse. I don't want to compute the set of all header dependencies when trying to find a function definition. We should not compute header dependencies in cases like this. Instead we should look in all the source files, Foo.h, FooInlines.h, and Foo.cpp. Just as today we have to look in both Foo.h and Foo.cpp since we don’t know whether the function is inlined or not. Also, I don't want to move things around when dependencies change. It would be good if we can accommodate you. But I don’t want to have to change all call sites when the complexity or inlining status of a member function changes. The rest of your proposal is something I agree on. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
[webkit-dev] Does anyone still use the TestFailures app?
http://build.webkit.org/TestFailures/ I think Adam Roben was working on this a year or so ago. It appears to be broken at the moment (it's likely that I broke it, in fact), but before I spend much time fixing it I thought I'd check. I've never actually used it myself, so I'm not sure what all it was supposed to do; it looks like it overlaps in functionality some with the flakiness dashboard, but was probably written before the flakiness dashboard worked with the build.webkit.org bots and everyone was converted to using NRWT. If anyone is still using it (or would if it was actually working) in preference to the flakiness dashboard, can you let me know why? Ideally I'd like to get rid of it and roll any good features it had into the flakiness dashboard, but I'm happy to fix it and/or keep it around if it does other things I'm not aware of or if people are still using it. Cheers, -- Dirk ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Does anyone still use the TestFailures app?
Hi, We use it and http://build.webkit.sed.hu/TestFailures/ for gardening, it is the first step we usually do if determining who broke what about the waterfall isn't trivial. On http://build.webkit.sed.hu/TestFailures/ we use a very old copy of test failures and it still works fine. Dirk Pranke írta: http://build.webkit.org/TestFailures/ I think Adam Roben was working on this a year or so ago. It appears to be broken at the moment (it's likely that I broke it, in fact), but before I spend much time fixing it I thought I'd check. It works for me more or less, but I got strange link names: http/tests/security/cross-origin-plugin-private-browsing-toggled.html: [object DocumentFragment] Maybe one of the garden-o-matic patches broke it somehow. I've never actually used it myself, so I'm not sure what all it was supposed to do; it looks like it overlaps in functionality some with the flakiness dashboard, but was probably written before the flakiness dashboard worked with the build.webkit.org bots and everyone was converted to using NRWT. If anyone is still using it (or would if it was actually working) in preference to the flakiness dashboard, can you let me know why? We still use it, because it is very simple, it works almost always, it isn't hakced day by day and its output is very very simple. We get the result - which revision broke a given test, which are the related bug reports - with _one_ click on the name of the slave. It is more complicated to do same thing on flakiness dashboard: - select webkit.org from group - select a given slave - select tests with wrong expectations - (unselect flaky) - find manually the last good revision for test by test but it is _impossible_ if the breakage is too old Ideally I'd like to get rid of it and roll any good features it had into the flakiness dashboard, but I'm happy to fix it and/or keep it around if it does other things I'm not aware of or if people are still using it. Please don't remove this good and simple tool, we use it day by day. br, Ossy ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Does anyone still use the TestFailures app?
On Mon, Nov 5, 2012 at 12:47 PM, Osztrogonac Csaba o...@inf.u-szeged.hu wrote: Hi, We use it and http://build.webkit.sed.hu/TestFailures/ for gardening, it is the first step we usually do if determining who broke what about the waterfall isn't trivial. On http://build.webkit.sed.hu/TestFailures/ we use a very old copy of test failures and it still works fine. Dirk Pranke írta: http://build.webkit.org/TestFailures/ I think Adam Roben was working on this a year or so ago. It appears to be broken at the moment (it's likely that I broke it, in fact), but before I spend much time fixing it I thought I'd check. It works for me more or less, but I got strange link names: http/tests/security/cross-origin-plugin-private-browsing-toggled.html: [object DocumentFragment] Maybe one of the garden-o-matic patches broke it somehow. I've never actually used it myself, so I'm not sure what all it was supposed to do; it looks like it overlaps in functionality some with the flakiness dashboard, but was probably written before the flakiness dashboard worked with the build.webkit.org bots and everyone was converted to using NRWT. If anyone is still using it (or would if it was actually working) in preference to the flakiness dashboard, can you let me know why? We still use it, because it is very simple, it works almost always, it isn't hakced day by day and its output is very very simple. We get the result - which revision broke a given test, which are the related bug reports - with _one_ click on the name of the slave. It is more complicated to do same thing on flakiness dashboard: - select webkit.org from group - select a given slave - select tests with wrong expectations - (unselect flaky) - find manually the last good revision for test by test but it is _impossible_ if the breakage is too old Ideally I'd like to get rid of it and roll any good features it had into the flakiness dashboard, but I'm happy to fix it and/or keep it around if it does other things I'm not aware of or if people are still using it. Please don't remove this good and simple tool, we use it day by day. Thanks, Ossy! I guess I'll figure out how to fix it and go from there :). -- Dirk ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Does anyone still use the TestFailures app?
I use it from time to time, but I think we really need merge it with garden-o-matic; they do much of the same stuff. Simon On Nov 5, 2012, at 12:21 PM, Dirk Pranke dpra...@chromium.org wrote: http://build.webkit.org/TestFailures/ I think Adam Roben was working on this a year or so ago. It appears to be broken at the moment (it's likely that I broke it, in fact), but before I spend much time fixing it I thought I'd check. I've never actually used it myself, so I'm not sure what all it was supposed to do; it looks like it overlaps in functionality some with the flakiness dashboard, but was probably written before the flakiness dashboard worked with the build.webkit.org bots and everyone was converted to using NRWT. If anyone is still using it (or would if it was actually working) in preference to the flakiness dashboard, can you let me know why? Ideally I'd like to get rid of it and roll any good features it had into the flakiness dashboard, but I'm happy to fix it and/or keep it around if it does other things I'm not aware of or if people are still using it. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] …Inlines.h vs …InlineMethods.h
Just so I understand, you're recommending we move functions like Document::guardDeref() http://trac.webkit.org/browser/trunk/Source/WebCore/dom/Document.h#L240 out of the class declaration, but leave them in the header file (e.g., in Document.h). Correct. And, if we decided to add DocumentInlines.h, Document::guardDeref() would move to DocumentInlines.h. In either case, Document::canContainRangeEndPoint() would not change at all. Geoff ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] …Inlines.h vs …InlineMethods.h
The proposed design requires adding a FooInlines.h include to source files that use that function, when any function moves into FooInlines.h. This can happen any time a function is made inline or when a short inline function gets longer. You convinced me; I hadn't considered this burden. Le me amend: (2) Adopt the convention that any class using *Inlines.h defines all inline functions defined out-of-line in *Inlines.h To (2) Adopt the convention that nothing goes into *Inlines.h by default, and functions are added on demand as we discover functions that cause compile failures or long compile times. Geoff ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] …Inlines.h vs …InlineMethods.h
Geoffrey Garen wrote: The proposed design requires adding a FooInlines.h include to source files that use that function, when any function moves into FooInlines.h. This can happen any time a function is made inline or when a short inline function gets longer. You convinced me; I hadn't considered this burden. Le me amend: (2) Adopt the convention that any class using *Inlines.h defines all inline functions defined out-of-line in *Inlines.h To (2) Adopt the convention that nothing goes into *Inlines.h by default, and functions are added on demand as we discover functions that cause compile failures or long compile times. Hi Geoff, sorry to stick my nose in it but Mozilla uses WebKit code (YARR, 3) so we care. The strong reason we've found beyond compile failures and long compile times (or really, this is the underlying cause of either compile failures or, alternatively, long compile times): inline method implementations are not appropriate to put in interface definitions. In other words, Foo.h declaring class Foo can focus on interface over implementation, even with a few short inline methods defined within the class or right after it -- but larger inlines may be required for performance, and their bodies can easily depend on headers not properly part of Foo.h-as-interface, which should therefore not bootleg along via nested #includes. Whereas FooInlines.h can nest its implementation dependencies freely. Implementation vs. interface distinctions can be fuzzy, but we've found it helpful to use this as a razor when shaving header files with inlines, before compile errors or compile time problems bite. /be Geoff ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] …Inlines.h vs …InlineMethods.h
On Nov 5, 2012, at 4:15 PM, Brendan Eich bren...@mozilla.org wrote: Geoffrey Garen wrote: The proposed design requires adding a FooInlines.h include to source files that use that function, when any function moves into FooInlines.h. This can happen any time a function is made inline or when a short inline function gets longer. You convinced me; I hadn't considered this burden. Le me amend: (2) Adopt the convention that any class using *Inlines.h defines all inline functions defined out-of-line in *Inlines.h To (2) Adopt the convention that nothing goes into *Inlines.h by default, and functions are added on demand as we discover functions that cause compile failures or long compile times. Hi Geoff, sorry to stick my nose in it but Mozilla uses WebKit code (YARR, 3) so we care. The strong reason we've found beyond compile failures and long compile times (or really, this is the underlying cause of either compile failures or, alternatively, long compile times): inline method implementations are not appropriate to put in interface definitions. In other words, Foo.h declaring class Foo can focus on interface over implementation, even with a few short inline methods defined within the class or right after it -- but larger inlines may be required for performance, and their bodies can easily depend on headers not properly part of Foo.h-as-interface, which should therefore not bootleg along via nested #includes. Whereas FooInlines.h can nest its implementation dependencies freely. Implementation vs. interface distinctions can be fuzzy, but we've found it helpful to use this as a razor when shaving header files with inlines, before compile errors or compile time problems bite. I think that the total time spent fixing dependencies due to inline methods being in the main header is going to be less than the total time spent having to search through multiple headers when doing normal work. As I've pointed out in past messages in this thread, we have classes where the best documentation of a method is the method's body - hence having the body inline is a big win for productivity. This may have more to due with how JSC is laid out. I think the last time I encountered a need to put a method body outside of the main header was over a month ago, if not more. -F /be Geoff ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] …Inlines.h vs …InlineMethods.h
Filip Pizlo wrote: On Nov 5, 2012, at 4:15 PM, Brendan Eichbren...@mozilla.org wrote: Implementation vs. interface distinctions can be fuzzy, but we've found it helpful to use this as a razor when shaving header files with inlines, before compile errors or compile time problems bite. I think that the total time spent fixing dependencies due to inline methods being in the main header is going to be less than the total time spent having to search through multiple headers when doing normal work. Oh, for sure -- it doesn't make sense to expend effort changing existing code just to match a vague rule about separating implementation from interface. Sorry if I seemed to suggest that. I started from a general inline method implementations are not appropriate to put in interface definitions assertion but allowed for even with a few short inline methods defined within the class or right after it. I should have allowed for other reasons not to split out FooInline.h. As I've pointed out in past messages in this thread, we have classes where the best documentation of a method is the method's body - hence having the body inline is a big win for productivity. Agreed. This may have more to due with how JSC is laid out. I think the last time I encountered a need to put a method body outside of the main header was over a month ago, if not more. That's cool. If you end up needing all the relevant headers and the topological sort is straightforward, fewer files wins. /be ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] Making more use of ScriptWrappable
To update this thread: I've now got this working in the V8 bindings.
The next step is to make this work in the JSC bindings. If you're
interested in the details, the work will occur on
https://bugs.webkit.org/show_bug.cgi?id=101279.
On Thu, Nov 1, 2012 at 10:51 AM, Alexey Proskuryakov a...@apple.com wrote:
Do you have a rough estimate of how large of a win we are talking about?
As a simple example, adding ScriptWrappable as a base class for
DOMImplementation makes document.implementation 23% faster, at least
as measured with the V8 bindings (see
https://bug-101279-attachments.webkit.org/attachment.cgi?id=172440).
Although I doubt that document.implementation itself is a performance
bottleneck, using ScriptWrappable more widely seems likely to improve
both performance and memory usage.
On Thu, Nov 1, 2012 at 4:10 PM, Maciej Stachowiak m...@apple.com wrote:
Sounds like a good idea. Three additional thoughts:
(1) It would be best to choose the objects to apply this to in some
data-driven way.
Do you have a suggestion for what data to use? As far as I can tell,
adding ScriptWrappable as a base class is a win whenever at least half
of the instances of the object have JavaScript wrappers (in the main
world):
A) It's always faster to get and set the JavaScript wrapper with
ScriptWrappable.
B) In terms of memory, we pay 1*sizeof(void*) for every instance with
ScriptWrappable compared to 2*sizeof(void*) for every instance that
has a JavaScript wrapper in the non-ScriptWrappable case (discounting
the fact that Hashtable actually seems to keep a constant fraction of
its buckets free).
(2) If we have an IDL attribute, I think it should be named by the effect it
has, not the possible conceptual-level reason for applying it.
[ScriptWrappable] or [InlineWrapper] or something. Because it's not a
judgment call, it is a statement about the code.
Turns out we don't need the IDL attribute (see the next question).
(3) I suspect that we can handle this without adding an IDL attribute at all.
C++ overloaded functions could let the bindings do something different for
objects that inherit ScriptWrappable from ones that do not in a generic way,
without having to explicitly tell the bindings layer about the ways to do it.
Consider the ways unwrap() and toJS() are done. We don't have to say anything
special in the IDL or have any interface-specific knowledge in the bindings,
C++ overloading takes care of it.
Thanks for the suggestion. I got this work (at least for the V8
bindings---JSC is next on my list). To make something
ScriptWrappable, you just need to add ScriptWrappable as a base class:
-class DOMImplementation {
+class DOMImplementation : public ScriptWrappable {
I'm not super excited about the name given that all DOM objects are
wrappable by script. If folks have thoughts about a better name, I'd
appreciate suggestions.
Thanks,
Adam
On Thu, Nov 1, 2012 at 10:36 AM, Adam Barth aba...@webkit.org wrote:
We currently use two different approaches for associating JavaScript
wrappers with DOM objects. For some objects, we store the wrapper
inline in the object itself by making object inherit from
ScriptWrappable. For other types of objects, we use a HashMap to
translate the object into a JavaScript wrapper.
Whether to use ScriptWrappable or a HashMap is a trade-off that
depends on the workload. For DOM objects that rarely have a
JavaScript wrapper, using a HashMap is more memory efficient because
we don't need to store a large number of null pointers in objects that
do not have wrappers. By contrast, if an object almost always has a
JavaScript wrapper, using ScriptWrappable is both faster (because we
avoid the hash table lookup) and uses less memory (because we don't
need to store both the key and the value in the HashMap---we just need
to store the value in the object itself).
Today, we use ScriptWrappable for Nodes only, but we would benefit by
making more use of ScriptWrappable, particularly for DOM objects that
almost always have JavaScript wrappers. For example, XMLHttpRequest
objects exist only when created by script, which means that every
XMLHttpRequest object has a JavaScript wrapper.
My plan is to introduce an interface-level IDL attribute named
something like [OftenHasJSWrapper] that informs the code generator
that the object inherits from ScriptWrappable and that we should make
use of the inline wrapper. We can then deploy this attribute as
appropriate throughout WebCore to reduce memory usage and improve
performance.
Please let me know if you have any feedback.
Thanks!
Adam
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] …Inlines.h vs …InlineMethods.h
These rules generally look great. One suggestion:
On Nov 5, 2012, at 8:47 AM, Geoffrey Garen wrote:
(5) Adopt the convention that any function that is not as trivial as int x()
{ return m_x; } moves out of the class declaration.
How about we simplify this slightly to:
(5) Adopt the convention that any function that is nontrivial should be moved
out of the class declaration.
We can give an example as to what might constitute trivial if we wish (e.g. is
a one liner), but I think leaving a little wiggle room to allow developers to
apply common sense would be a good thing. While moving all complex functions
out of class definitions sounds good, for some small classes being able to
leave some very simple functions in the class declaration might not hurt, and
might make the code easier to work with. E.g.:
int y()
{
ASSERT(m_y != BAD_VALUE);
return m_y;
}
cheers,
G.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] …Inlines.h vs …InlineMethods.h
In other words, Foo.h declaring class Foo can focus on interface over implementation, even with a few short inline methods defined within the class or right after it -- but larger inlines may be required for performance, and their bodies can easily depend on headers not properly part of Foo.h-as-interface, which should therefore not bootleg along via nested #includes. Whereas FooInlines.h can nest its implementation dependencies freely. Maybe we should amend again: (2) Adopt the convention that the following functions got into *Inlines.h: (2a) Functions that cause circular header dependencies (2b) Functions that cause a header to #include another header that clients would not otherwise #include An example of (2a): JSValue::toWTFStringInline(). An example of (2b): Node::renderStyle(). Geoff ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] …Inlines.h vs …InlineMethods.h
(5) Adopt the convention that any function that is not as trivial as int
x() { return m_x; } moves out of the class declaration.
How about we simplify this slightly to:
(5) Adopt the convention that any function that is nontrivial should be moved
out of the class declaration.
We can give an example as to what might constitute trivial if we wish (e.g.
is a one liner), but I think leaving a little wiggle room to allow developers
to apply common sense would be a good thing. While moving all complex
functions out of class definitions sounds good, for some small classes being
able to leave some very simple functions in the class declaration might not
hurt, and might make the code easier to work with. E.g.:
int y()
{
ASSERT(m_y != BAD_VALUE);
return m_y;
}
If possible, I'd like to establish clarity on what trivial or nontrivial
means. This can save us debates in future patch reviews about what's trivial
enough.
To me, trivial means short. My straw-man proposal is one-line functions
only.
Failing that, I would at least like to pick some number. Maybe 6 lines, since
that's just enough for a branch with an early return.
Thought complexity notwithstanding, non-trivial functions mainly get in the way
of reading an interface because they take up space. int x() { return m_x; }
is fine by me because it doesn't add any lines of code over int x();.
Notably, the next shortest function possible in WebKit style after one line is
five lines. That means that I see 5X less of the class declaration in one
screenful of code, and I have to do 5X more scrolling before I can see the data
members in a class. To me, that's a significant blow to readability for a
slight improvement in write-ability. Given that reading is more common than
writing, I'm inclined to argue that 1 line functions are not worth it.
In general, I think brevity in class declarations is particularly important. I
often find myself needing to read all of the public interfaces of a class, or
look at a declaration in relation to a prior public or private keyword, or
scroll past the interface declarations to get to the data members. (In
contrast, I rarely need to read all of the function implementations of a class
at once, or scroll to a specific lexical location among a set of function
implementations.) Within some limits, I'm willing to write code more slowly so
I can read declarations more quickly.
Geoff
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev
Re: [webkit-dev] …Inlines.h vs …InlineMethods.h
On Nov 5, 2012, at 9:27 PM, Geoffrey Garen gga...@apple.com wrote:
(5) Adopt the convention that any function that is not as trivial as int
x() { return m_x; } moves out of the class declaration.
How about we simplify this slightly to:
(5) Adopt the convention that any function that is nontrivial should be
moved out of the class declaration.
We can give an example as to what might constitute trivial if we wish (e.g.
is a one liner), but I think leaving a little wiggle room to allow
developers to apply common sense would be a good thing. While moving all
complex functions out of class definitions sounds good, for some small
classes being able to leave some very simple functions in the class
declaration might not hurt, and might make the code easier to work with.
E.g.:
int y()
{
ASSERT(m_y != BAD_VALUE);
return m_y;
}
If possible, I'd like to establish clarity on what trivial or nontrivial
means. This can save us debates in future patch reviews about what's trivial
enough.
To me, trivial means short. My straw-man proposal is one-line functions
only.
Failing that, I would at least like to pick some number. Maybe 6 lines, since
that's just enough for a branch with an early return.
Thought complexity notwithstanding, non-trivial functions mainly get in the
way of reading an interface because they take up space. int x() { return
m_x; } is fine by me because it doesn't add any lines of code over int
x();. Notably, the next shortest function possible in WebKit style after one
line is five lines. That means that I see 5X less of the class declaration in
one screenful of code, and I have to do 5X more scrolling before I can see
the data members in a class. To me, that's a significant blow to readability
for a slight improvement in write-ability. Given that reading is more common
than writing, I'm inclined to argue that 1 line functions are not worth it.
In general, I think brevity in class declarations is particularly important.
I often find myself needing to read all of the public interfaces of a class,
or look at a declaration in relation to a prior public or private
keyword, or scroll past the interface declarations to get to the data
members. (In contrast, I rarely need to read all of the function
implementations of a class at once, or scroll to a specific lexical location
among a set of function implementations.) Within some limits, I'm willing to
write code more slowly so I can read declarations more quickly.
I prefer wiggle room for decisions regarding where to put method bodies. I
agree that we should use *Inlines.h instead of hijacking other class's headers.
But beyond that, I would prefer to go with Gavin's suggestion rather than
imposing a rigid rule.
To me the decision of where to put a method body comes down to weighing two use
cases:
A) Reading the interface that a class provides.
B) Making changes to the class.
Inline methods being truly inline in the class body aids (B) while impeding
(A). Non-tiny methods being out-of-line aids (A) while impeding (B).
For most of the classes with which I am familiar, (B) appears to be more common
than (A) by virtue of those classes having very few consumers. I would guess
that the typical class in JavaScriptCore is only directly used from a handful
places. So, it's uncommon that you're just going to be wondering about the
class's interface. It's much more likely that if you're thinking about that
class, you're goal is to change it. At that point, having more of the class's
guts collocated in one place is a good thing. (Of course for methods that
ought to be out-of-line in a .cpp file there's nothing we can do - but at least
we can simplify life for inline methods.)
I suspect that there are also classes for which (A) is an unlikely use case
just because it has low utility - say, because it's a gnarly enough class that
just knowing the method names reveals too little information.
MarkedBlock.h is a good example of a class that has good encapsulation, but
that has very few consumers by virtue of it being a class that is mostly
internal to one of the two spaces managed by our GC. Hence, the most common
use case for touching MarkedBlock.h is not to read how it interacts with
other classes (since there are not many classes that it interacts with) but to
change MarkedBlock itself. Whenever I have to touch MarkedBlock, I find myself
annoyed by methods appearing in two places. Adding methods and changing their
signatures is tedious. And understanding the methods is also tedious - for
example I rarely want to know whether MarkedBlock has an isLive() method, but I
often want to know how isLive() works.
A counterexample to this would be something like Vector, where (A) is orders of
magnitude more likely than (B). Also, I rarely want to know how Vector works.
I'm happy to assume that it just does the good things. So of course for
