[webkit-dev] What is XBL?
What is XBL? As far as I can tell, there's no way this code can even build. It's incredibly stale. Can we remove it? Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] What is XBL?
http://en.wikipedia.org/wiki/XBL , I guess? XBL (XML Binding Language) is an XML-based markup language used to declare the behavior and look of XUL-widgets and XML elements. I don't think we need that :-) Kenneth On Wed, Aug 25, 2010 at 4:22 AM, Adam Barth aba...@webkit.org wrote: What is XBL? As far as I can tell, there's no way this code can even build. It's incredibly stale. Can we remove it? Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev -- Kenneth Rohde Christiansen Technical Lead / Senior Software Engineer Qt Labs Americas, Nokia Technology Institute, INdT Phone +55 81 8895 6002 / E-mail kenneth.christiansen at openbossa.org http://codeposts.blogspot.com ﹆﹆﹆ ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] We need OwnPtrHashMap
Sorry for the late-night webkit-dev spam, but in deploying adoptPtr, I've noticed a number of places where have a HashMap that owns its values as OwnPtrs. Unfortunately, this very clumsy currently. Each instance of this pattern has its own way of hacking around the problem, which might or might not result in memory errors. We really should have an OwnPtrHashMap (to complement RefPtrHashMap) that understands how to handle this case correctly. Is anyone interested in working this problem? Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] What is XBL? or XBRL?
From: kenneth.christian...@gmail.com Date: Wed, 25 Aug 2010 04:24:49 -0300 To: aba...@webkit.org CC: webkit-dev@lists.webkit.org Subject: Re: [webkit-dev] What is XBL? http://en.wikipedia.org/wiki/XBL , I guess? XBL (XML Binding Language) is an XML-based markup language used to declare the behavior and look of XUL-widgets and XML elements. I don't think we need that :-) ( sorry if this is more incoherent than normal hotmail really having problems lately, imagine that ) This reminds me of another issue exemplified by XBRL or maybe RSS would be more familiar, http://en.wikipedia.org/wiki/XBRL http://www.xbrl.org/Home/ http://xbrl.sec.gov/ that I have seen developing between webpages designed for human readability ( lots of pictures or pdf's from which text can not be extracted in a sane format) and those that allow further data processing without a human baby sitter such as structured documents in an XML format. The big focus of the browser is generally to allow interactive access to graphics information- browse if you will. However, this limits the ability to use structured documents for automated data processing, as if anyone would do that with a computer. Personally, anything that can help make better use of structured data files while reasonably being within the scope of browsing should be considered. I guess you could say that this should all just be delegated to plugins for certain content types and probably largely so. Most webpages are designed to describe a human readable page but it is quite reasonable for a user to browse to a data file and have different hanlders for different platforms. RSS I would say usses essentially one type of data file that fits into this catagory. Kenneth On Wed, Aug 25, 2010 at 4:22 AM, Adam Barth wrote: What is XBL? As far as I can tell, there's no way this code can even build. It's incredibly stale. Can we remove it? Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev -- Kenneth Rohde Christiansen Technical Lead / Senior Software Engineer Qt Labs Americas, Nokia Technology Institute, INdT Phone +55 81 8895 6002 / E-mail kenneth.christiansen at openbossa.org http://codeposts.blogspot.com ﹆﹆﹆ ___ 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] Place For new Core Module
On Tue, Aug 24, 2010 at 11:50 PM, Chinmaya Sn chinm...@gmail.com wrote: Thanks All. I think I am getting the general idea of what can get into to WebKit and how things fit. Right now both the standard and implementation are WIP, spec is intended to be a Web standard. At this point, neither the standard nor the implementation are public. Even if you want to use your prototype implementation to inform your spec's design, I'd highly recommend bringing up your general ideas on some standards list ASAP. These lists often have a lot of smart people with a lot of varied experience. Often they'll be able to point out major flaws in your idea or other technologies you might want to look at. And, at very least, it'll give you an idea of how your final spec will be received. I'd highly suggest at least floating the general concepts around some standards lists otherwise there's a good chance your proposal will be dead on arrival. On Tue, Aug 24, 2010 at 11:59 PM, Eric Seidel e...@webkit.org wrote: Secret ports have an absolutely horrible track record of ever catching up with public WebKit. Only one has ever been successful to my knowledge (Chromium) and I'm not even sure we could call it full success yet. (They've spent 2 years attempting to fully catch up, and yet you can't build a useable binary out of webkit.org -- although that's very close!) Apple's iPhone fork has (or had, maybe this has changed) one full-time person who's job is constantly merging. I'm not sure how close to tip-of-tree they're able to stay. It's possible that you work with the most fantastic engineers WebKit will ever see. But let me caution you: if CableKit forks in secret, you're very likely to always be months if not years behind and riddled with security vulnerabilities. :) Furthermore, specs take a long time and a lot of buy-in. Spec-work done in secret is unlikely to end up ever getting the buy-in it needs to become a spec. Best of luck. +1 J ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] Checking allocation failures [was: Re: Naked new considered harmful]
Am 24.08.2010 19:46, schrieb Adam Barth: One thing Darin and I discussed at WWDC (yes, this email has been a long time coming) is better programming patterns to prevent memory leaks. As I'm sure you know, whenever you allocate a RefCounted object, you need to call adoptRef to prevent a memory leak by adopting the object's initial reference: adoptRef(new FancyRefCountedObject()) We now have an ASSERT that enforces this programming pattern, so we should be able to catch errors pretty easily. Recently, Darin also introduced an analogous function for OwnPtr: adoptPtr(new NiftyNonRefCountedObject()) What adoptPtr does is shove the newly allocated object into a PassOwnPtr, which together with OwnPtr, makes sure we don't leak the object. By using one of these two functions every time we call new, it's easy to see that we don't have any memory leaks. In the cases where we have an intentional memory leak (e.g., for a static), please use the leakPtr() member function to document the leak. In writing new code, please avoid using naked calls to new. If you're making an object that you expect to be heap-allocated, please add a create method, similar to the create method we use for RefCounted objects: static PassOwnPtrNiftyNonRefCountedObject create(ParamClass* param) { return adoptPtr(new NiftyNonRefCountedObject(param)); } You should also make the constructor non-public so folks are forced to call the create method. (Of course, stack-allocated objects should have public constructors.) I'm going through WebCore and removing as many naked news and I can. At some point, we'll add a stylebot rule to flag violations for new code. If you'd like to help out, pick a directory and change all the calls to new to use adoptRef or adoptPtr, as appropriate. this reminds me that I've always been wondering about checks for allocation failure in WebCore (versus concerns about leaks). A plain call to new may throw an std::bad_alloc exception. If this is not considered, it may leave objects in an invalid state, even when using objects such as RefPtr to wrap arround allocations. I don't remember any specific place in the WebCore code where it would be a problem, I just don't remember seeing any code that deals with allocation failures. Is this a design choice somehow? If so, is there some online documentation/discussion about it? (Tried to google it but found nothing...) Best regards, -Stephan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] What is XBL?
http://en.wikipedia.org/wiki/XBL , I guess? FYI XBL has been superseded by XBL 2.0 that is not backward compatible with the first version [1]. Thus our code is just obsolete. Regards, Julien [1] http://www.w3.org/TR/xbl/#relationship ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] What is XBL?
On Aug 25, 2010, at 9:11 AM, Julien Chaffraix wrote: http://en.wikipedia.org/wiki/XBL , I guess? FYI XBL has been superseded by XBL 2.0 that is not backward compatible with the first version [1]. Thus our code is just obsolete. All XBL-related code in WebKit is for XBL2. It seems as if WebKit had working XBL2 support, most of the form controls added over the last couple of years could have been defined declaratively in markup rather than requiring the significant amount of C++ code that they currently do. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] What is XBL?
The existing code is very confused. For example, it doesn't seem to know that RefCounted objects start with an initial ref. My guess is that it was written a long time ago and then was abandoned. As the project moved forward, this code got left behind. Adam On Wed, Aug 25, 2010 at 9:26 AM, Dimitri Glazkov dglaz...@chromium.org wrote: Hear hear. I am still very interesting in having something like XBL in WebKit and am now to the point of having good chunk of time to work on this. Having said that, I am not sure how much of the current XBL-related code remains useful. If it's not, we might just as well zap it and start with a clean slate. :DG On Wed, Aug 25, 2010 at 9:18 AM, Dan Bernstein m...@apple.com wrote: On Aug 25, 2010, at 9:11 AM, Julien Chaffraix wrote: http://en.wikipedia.org/wiki/XBL , I guess? FYI XBL has been superseded by XBL 2.0 that is not backward compatible with the first version [1]. Thus our code is just obsolete. All XBL-related code in WebKit is for XBL2. It seems as if WebKit had working XBL2 support, most of the form controls added over the last couple of years could have been defined declaratively in markup rather than requiring the significant amount of C++ code that they currently do. ___ 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 ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Checking allocation failures [was: Re: Naked new considered harmful]
On Wed, Aug 25, 2010 at 7:09 AM, Stephan Assmus supersti...@gmx.de wrote: Am 24.08.2010 19:46, schrieb Adam Barth: One thing Darin and I discussed at WWDC (yes, this email has been a long time coming) is better programming patterns to prevent memory leaks. As I'm sure you know, whenever you allocate a RefCounted object, you need to call adoptRef to prevent a memory leak by adopting the object's initial reference: adoptRef(new FancyRefCountedObject()) We now have an ASSERT that enforces this programming pattern, so we should be able to catch errors pretty easily. Recently, Darin also introduced an analogous function for OwnPtr: adoptPtr(new NiftyNonRefCountedObject()) What adoptPtr does is shove the newly allocated object into a PassOwnPtr, which together with OwnPtr, makes sure we don't leak the object. By using one of these two functions every time we call new, it's easy to see that we don't have any memory leaks. In the cases where we have an intentional memory leak (e.g., for a static), please use the leakPtr() member function to document the leak. In writing new code, please avoid using naked calls to new. If you're making an object that you expect to be heap-allocated, please add a create method, similar to the create method we use for RefCounted objects: static PassOwnPtrNiftyNonRefCountedObject create(ParamClass* param) { return adoptPtr(new NiftyNonRefCountedObject(param)); } You should also make the constructor non-public so folks are forced to call the create method. (Of course, stack-allocated objects should have public constructors.) I'm going through WebCore and removing as many naked news and I can. At some point, we'll add a stylebot rule to flag violations for new code. If you'd like to help out, pick a directory and change all the calls to new to use adoptRef or adoptPtr, as appropriate. this reminds me that I've always been wondering about checks for allocation failure in WebCore (versus concerns about leaks). A plain call to new may throw an std::bad_alloc exception. If this is not considered, it may leave objects in an invalid state, even when using objects such as RefPtr to wrap arround allocations. I don't remember any specific place in the WebCore code where it would be a problem, I just don't remember seeing any code that deals with allocation failures. Is this a design choice somehow? If so, is there some online documentation/discussion about it? (Tried to google it but found nothing...) Failed allocations in WebKit call CRASH(). This prevents us from ending up in an inconsistent state. Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] What is XBL?
I recommend removing the XBL code. Hyatt started it years ago. Jullien works on it for a summer as my GSoC mentee. Anyone who wants to write a fully working XBL2 implementation can look in svn history. As-is, it's untested, unbuildable, and a drag on the project. -eric On Wed, Aug 25, 2010 at 12:22 AM, Adam Barth aba...@webkit.org wrote: What is XBL? As far as I can tell, there's no way this code can even build. It's incredibly stale. Can we remove it? Adam ___ 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] What is XBL?
I think Hyatt should comment on this before we decide. I do think that removing the code would be good but he may have some additional insight. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] What is XBL?
There seem to be a bunch of files missing too. For example, I can't find XBLDocument or XMLBindingsManager. I don't mean to hate on XBL, I just started looking at it when deploying adoptPtr and was surprised by what I found, that's all. Adam On Wed, Aug 25, 2010 at 10:13 AM, Darin Adler da...@apple.com wrote: I think Hyatt should comment on this before we decide. I do think that removing the code would be good but he may have some additional insight. -- Darin ___ 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] What is XBL?
Ok. I've posted a patch: https://bugs.webkit.org/show_bug.cgi?id=44621 Adam On Wed, Aug 25, 2010 at 10:17 AM, Adam Barth aba...@webkit.org wrote: There seem to be a bunch of files missing too. For example, I can't find XBLDocument or XMLBindingsManager. I don't mean to hate on XBL, I just started looking at it when deploying adoptPtr and was surprised by what I found, that's all. Adam On Wed, Aug 25, 2010 at 10:13 AM, Darin Adler da...@apple.com wrote: I think Hyatt should comment on this before we decide. I do think that removing the code would be good but he may have some additional insight. -- Darin ___ 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] Making GRefPtr more general
On Tue, Aug 24, 2010 at 5:31 PM, Darin Adler da...@apple.com wrote: If overloading can be used to determine how to do the take a ref and release a ref functions, then many different subsystems can share a single PassRefPtr/RefPtr class. One complication in the GObject case, is that the argument type for g_object_ref(...) and g_object_unref(..) is void*. Perhaps there is some way to engineer around this. Martin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Bad Qt API?
Furthermore, any loops like this: for (RefPtrNode child = m_element-firstChild(); child;) { which allow synchronous javascript execution (i.e. take an ExceptionCode parameter) are vulnerable to crashes/security holes. :( All of those enclose* functions use such loops. :( -eric On Wed, Aug 25, 2010 at 11:47 AM, Eric Seidel e...@webkit.org wrote: My comments apply to all of the enclose* APIs in that file. On Wed, Aug 25, 2010 at 11:46 AM, Eric Seidel e...@webkit.org wrote: /*! Encloses the contents of this element with the result of parsing \a markup. This element becomes the child of the deepest descendant within \a markup. \sa encloseWith() */ void QWebElement::encloseContentsWith(const QString markup) http://trac.webkit.org/browser/trunk/WebKit/qt/Api/qwebelement.cpp#L1248 These enclose methods use at least 2 deprecated parts of parser code (HTMLElement::endTagRequirement() and HTMLElement::deprecatedCreateContextualFragment()). They're clear layering violations, and make little sense to me. Who wants to call this API? Can it be removed from Qt? -eric ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Bad Qt API?
My comments apply to all of the enclose* APIs in that file. On Wed, Aug 25, 2010 at 11:46 AM, Eric Seidel e...@webkit.org wrote: /*! Encloses the contents of this element with the result of parsing \a markup. This element becomes the child of the deepest descendant within \a markup. \sa encloseWith() */ void QWebElement::encloseContentsWith(const QString markup) http://trac.webkit.org/browser/trunk/WebKit/qt/Api/qwebelement.cpp#L1248 These enclose methods use at least 2 deprecated parts of parser code (HTMLElement::endTagRequirement() and HTMLElement::deprecatedCreateContextualFragment()). They're clear layering violations, and make little sense to me. Who wants to call this API? Can it be removed from Qt? -eric ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] Bad Qt API?
/*! Encloses the contents of this element with the result of parsing \a markup. This element becomes the child of the deepest descendant within \a markup. \sa encloseWith() */ void QWebElement::encloseContentsWith(const QString markup) http://trac.webkit.org/browser/trunk/WebKit/qt/Api/qwebelement.cpp#L1248 These enclose methods use at least 2 deprecated parts of parser code (HTMLElement::endTagRequirement() and HTMLElement::deprecatedCreateContextualFragment()). They're clear layering violations, and make little sense to me. Who wants to call this API? Can it be removed from Qt? -eric ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Bad Qt API?
Could you file bugs for these? Also, do you know of any other way of accomplishing the same behaviour without having security issues/crashes? Kenneth On Wed, Aug 25, 2010 at 3:51 PM, Eric Seidel e...@webkit.org wrote: Furthermore, any loops like this: for (RefPtrNode child = m_element-firstChild(); child;) { which allow synchronous javascript execution (i.e. take an ExceptionCode parameter) are vulnerable to crashes/security holes. :( All of those enclose* functions use such loops. :( -eric On Wed, Aug 25, 2010 at 11:47 AM, Eric Seidel e...@webkit.org wrote: My comments apply to all of the enclose* APIs in that file. On Wed, Aug 25, 2010 at 11:46 AM, Eric Seidel e...@webkit.org wrote: /*! Encloses the contents of this element with the result of parsing \a markup. This element becomes the child of the deepest descendant within \a markup. \sa encloseWith() */ void QWebElement::encloseContentsWith(const QString markup) http://trac.webkit.org/browser/trunk/WebKit/qt/Api/qwebelement.cpp#L1248 These enclose methods use at least 2 deprecated parts of parser code (HTMLElement::endTagRequirement() and HTMLElement::deprecatedCreateContextualFragment()). They're clear layering violations, and make little sense to me. Who wants to call this API? Can it be removed from Qt? -eric ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev -- Kenneth Rohde Christiansen Technical Lead / Senior Software Engineer Qt Labs Americas, Nokia Technology Institute, INdT Phone +55 81 8895 6002 / E-mail kenneth.christiansen at openbossa.org http://codeposts.blogspot.com ﹆﹆﹆ ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Bad Qt API?
They are methods for manipulating the DOM, modelled after jQuery. The documentation should explain pretty well what they do. Is it possible to accomplish the same behaviour with the new parser API? We need to continue supporting this API due to our binary compatibility, plus because of the fact that it was added due to customer request. Cheers, Kenneth On Wed, Aug 25, 2010 at 3:46 PM, Eric Seidel e...@webkit.org wrote: /*! Encloses the contents of this element with the result of parsing \a markup. This element becomes the child of the deepest descendant within \a markup. \sa encloseWith() */ void QWebElement::encloseContentsWith(const QString markup) http://trac.webkit.org/browser/trunk/WebKit/qt/Api/qwebelement.cpp#L1248 These enclose methods use at least 2 deprecated parts of parser code (HTMLElement::endTagRequirement() and HTMLElement::deprecatedCreateContextualFragment()). They're clear layering violations, and make little sense to me. Who wants to call this API? Can it be removed from Qt? -eric ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev -- Kenneth Rohde Christiansen Technical Lead / Senior Software Engineer Qt Labs Americas, Nokia Technology Institute, INdT Phone +55 81 8895 6002 / E-mail kenneth.christiansen at openbossa.org http://codeposts.blogspot.com ﹆﹆﹆ ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Bad Qt API?
Btw, Eric, We have unit tests for the QWebElement API in WebKit/qt/tests/qwebelement/ so you can see the use-cases, if that helps. Cheers, Kenneth On Wed, Aug 25, 2010 at 3:58 PM, Kenneth Rohde Christiansen kenneth.christian...@gmail.com wrote: They are methods for manipulating the DOM, modelled after jQuery. The documentation should explain pretty well what they do. Is it possible to accomplish the same behaviour with the new parser API? We need to continue supporting this API due to our binary compatibility, plus because of the fact that it was added due to customer request. Cheers, Kenneth On Wed, Aug 25, 2010 at 3:46 PM, Eric Seidel e...@webkit.org wrote: /*! Encloses the contents of this element with the result of parsing \a markup. This element becomes the child of the deepest descendant within \a markup. \sa encloseWith() */ void QWebElement::encloseContentsWith(const QString markup) http://trac.webkit.org/browser/trunk/WebKit/qt/Api/qwebelement.cpp#L1248 These enclose methods use at least 2 deprecated parts of parser code (HTMLElement::endTagRequirement() and HTMLElement::deprecatedCreateContextualFragment()). They're clear layering violations, and make little sense to me. Who wants to call this API? Can it be removed from Qt? -eric ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev -- Kenneth Rohde Christiansen Technical Lead / Senior Software Engineer Qt Labs Americas, Nokia Technology Institute, INdT Phone +55 81 8895 6002 / E-mail kenneth.christiansen at openbossa.org http://codeposts.blogspot.com ﹆﹆﹆ -- Kenneth Rohde Christiansen Technical Lead / Senior Software Engineer Qt Labs Americas, Nokia Technology Institute, INdT Phone +55 81 8895 6002 / E-mail kenneth.christiansen at openbossa.org http://codeposts.blogspot.com ﹆﹆﹆ ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] What is XBL?
The code can be removed. dave On Aug 25, 2010, at 12:17 PM, Adam Barth wrote: There seem to be a bunch of files missing too. For example, I can't find XBLDocument or XMLBindingsManager. I don't mean to hate on XBL, I just started looking at it when deploying adoptPtr and was surprised by what I found, that's all. Adam On Wed, Aug 25, 2010 at 10:13 AM, Darin Adler da...@apple.com wrote: I think Hyatt should comment on this before we decide. I do think that removing the code would be good but he may have some additional insight. -- Darin ___ 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 ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] We need OwnPtrHashMap
Sorry for the late-night webkit-dev spam, but in deploying adoptPtr, I've noticed a number of places where have a HashMap that owns its values as OwnPtrs. Unfortunately, this very clumsy currently. Each instance of this pattern has its own way of hacking around the problem, which might or might not result in memory errors. We really should have an OwnPtrHashMap (to complement RefPtrHashMap) that understands how to handle this case correctly. To clarify: Optimization aside, HashMapRefPtrT, U and HashMapT, RefPtrU work just fine. RefPtrHashMap was an optimization. The feature it needed, which HashMap did not provide, was the ability to look up a value by a non-key type (raw pointer), without first converting to key type (RefPtr), since converting to RefPtr would cause refcount churn. We couldn't find a way to do this using HashTraits without using casts that violated strict aliasing rules. In contrast, HashMapOwnPtrT, U and HashMapT, OwnPtrU don't work at all. They don't compile, since OwnPtr is not copyable. If you made OwnPtr copyable, they would accidentally delete items during get() and resize() operations. A HashMap that owns its values wants to do something special when an item is overwritten, removed, or implicitly removed by HashMap destruction, but it doesn't want to do anything special when an item is copied or extracted. I think the best way to achieve this with HashMap might be a hash trait, rather than literally putting an OwnPtr in the map. Specifically, the trait would be one willRemove callback, which took an iterator to an item, and one willRemoveAll callback, which took a begin iterator. These callbacks would default to empty inline functions. But maybe there's a way to use special hash traits and translators to eliminate all or nearly all the C++ copying operations. Geoff ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Checking allocation failures [was: Re: Naked new considered harmful]
Am 25.08.2010 18:35, schrieb Adam Barth: On Wed, Aug 25, 2010 at 7:09 AM, Stephan Assmussupersti...@gmx.de wrote: Am 24.08.2010 19:46, schrieb Adam Barth: One thing Darin and I discussed at WWDC (yes, this email has been a long time coming) is better programming patterns to prevent memory leaks. As I'm sure you know, whenever you allocate a RefCounted object, you need to call adoptRef to prevent a memory leak by adopting the object's initial reference: adoptRef(new FancyRefCountedObject()) We now have an ASSERT that enforces this programming pattern, so we should be able to catch errors pretty easily. Recently, Darin also introduced an analogous function for OwnPtr: adoptPtr(new NiftyNonRefCountedObject()) What adoptPtr does is shove the newly allocated object into a PassOwnPtr, which together with OwnPtr, makes sure we don't leak the object. By using one of these two functions every time we call new, it's easy to see that we don't have any memory leaks. In the cases where we have an intentional memory leak (e.g., for a static), please use the leakPtr() member function to document the leak. In writing new code, please avoid using naked calls to new. If you're making an object that you expect to be heap-allocated, please add a create method, similar to the create method we use for RefCounted objects: static PassOwnPtrNiftyNonRefCountedObjectcreate(ParamClass* param) { return adoptPtr(new NiftyNonRefCountedObject(param)); } You should also make the constructor non-public so folks are forced to call the create method. (Of course, stack-allocated objects should have public constructors.) I'm going through WebCore and removing as many naked news and I can. At some point, we'll add a stylebot rule to flag violations for new code. If you'd like to help out, pick a directory and change all the calls to new to use adoptRef or adoptPtr, as appropriate. this reminds me that I've always been wondering about checks for allocation failure in WebCore (versus concerns about leaks). A plain call to new may throw an std::bad_alloc exception. If this is not considered, it may leave objects in an invalid state, even when using objects such as RefPtr to wrap arround allocations. I don't remember any specific place in the WebCore code where it would be a problem, I just don't remember seeing any code that deals with allocation failures. Is this a design choice somehow? If so, is there some online documentation/discussion about it? (Tried to google it but found nothing...) Failed allocations in WebKit call CRASH(). This prevents us from ending up in an inconsistent state. Ok, but WebKit is used on devices where allocation failure is somewhat of a realistic possibility, no? Wouldn't it be desirable to handle such a situation gracefully? (I.e. display of an error message when trying to open one more tab rather than crashing the entire browser, for example.) Hopefully I am not missing something obvious. Best regards, -Stephan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Checking allocation failures [was: Re: Naked new considered harmful]
On Wed, Aug 25, 2010 at 2:31 PM, Stephan Assmus supersti...@gmx.de wrote: Am 25.08.2010 18:35, schrieb Adam Barth: On Wed, Aug 25, 2010 at 7:09 AM, Stephan Assmussupersti...@gmx.de wrote: Am 24.08.2010 19:46, schrieb Adam Barth: One thing Darin and I discussed at WWDC (yes, this email has been a long time coming) is better programming patterns to prevent memory leaks. As I'm sure you know, whenever you allocate a RefCounted object, you need to call adoptRef to prevent a memory leak by adopting the object's initial reference: adoptRef(new FancyRefCountedObject()) We now have an ASSERT that enforces this programming pattern, so we should be able to catch errors pretty easily. Recently, Darin also introduced an analogous function for OwnPtr: adoptPtr(new NiftyNonRefCountedObject()) What adoptPtr does is shove the newly allocated object into a PassOwnPtr, which together with OwnPtr, makes sure we don't leak the object. By using one of these two functions every time we call new, it's easy to see that we don't have any memory leaks. In the cases where we have an intentional memory leak (e.g., for a static), please use the leakPtr() member function to document the leak. In writing new code, please avoid using naked calls to new. If you're making an object that you expect to be heap-allocated, please add a create method, similar to the create method we use for RefCounted objects: static PassOwnPtrNiftyNonRefCountedObject create(ParamClass* param) { return adoptPtr(new NiftyNonRefCountedObject(param)); } You should also make the constructor non-public so folks are forced to call the create method. (Of course, stack-allocated objects should have public constructors.) I'm going through WebCore and removing as many naked news and I can. At some point, we'll add a stylebot rule to flag violations for new code. If you'd like to help out, pick a directory and change all the calls to new to use adoptRef or adoptPtr, as appropriate. this reminds me that I've always been wondering about checks for allocation failure in WebCore (versus concerns about leaks). A plain call to new may throw an std::bad_alloc exception. If this is not considered, it may leave objects in an invalid state, even when using objects such as RefPtr to wrap arround allocations. I don't remember any specific place in the WebCore code where it would be a problem, I just don't remember seeing any code that deals with allocation failures. Is this a design choice somehow? If so, is there some online documentation/discussion about it? (Tried to google it but found nothing...) Failed allocations in WebKit call CRASH(). This prevents us from ending up in an inconsistent state. Ok, but WebKit is used on devices where allocation failure is somewhat of a realistic possibility, no? Wouldn't it be desirable to handle such a situation gracefully? (I.e. display of an error message when trying to open one more tab rather than crashing the entire browser, for example.) Hopefully I am not missing something obvious. Dunno. The crash-on-allocation failure assumption is baked into lots of lines of code. Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Checking allocation failures [was: Re: Naked new considered harmful]
On Wed, Aug 25, 2010 at 2:34 PM, Adam Barth aba...@webkit.org wrote: On Wed, Aug 25, 2010 at 2:31 PM, Stephan Assmus supersti...@gmx.de wrote: Am 25.08.2010 18:35, schrieb Adam Barth: On Wed, Aug 25, 2010 at 7:09 AM, Stephan Assmussupersti...@gmx.de wrote: Am 24.08.2010 19:46, schrieb Adam Barth: One thing Darin and I discussed at WWDC (yes, this email has been a long time coming) is better programming patterns to prevent memory leaks. As I'm sure you know, whenever you allocate a RefCounted object, you need to call adoptRef to prevent a memory leak by adopting the object's initial reference: adoptRef(new FancyRefCountedObject()) We now have an ASSERT that enforces this programming pattern, so we should be able to catch errors pretty easily. Recently, Darin also introduced an analogous function for OwnPtr: adoptPtr(new NiftyNonRefCountedObject()) What adoptPtr does is shove the newly allocated object into a PassOwnPtr, which together with OwnPtr, makes sure we don't leak the object. By using one of these two functions every time we call new, it's easy to see that we don't have any memory leaks. In the cases where we have an intentional memory leak (e.g., for a static), please use the leakPtr() member function to document the leak. In writing new code, please avoid using naked calls to new. If you're making an object that you expect to be heap-allocated, please add a create method, similar to the create method we use for RefCounted objects: static PassOwnPtrNiftyNonRefCountedObjectcreate(ParamClass* param) { return adoptPtr(new NiftyNonRefCountedObject(param)); } You should also make the constructor non-public so folks are forced to call the create method. (Of course, stack-allocated objects should have public constructors.) I'm going through WebCore and removing as many naked news and I can. At some point, we'll add a stylebot rule to flag violations for new code. If you'd like to help out, pick a directory and change all the calls to new to use adoptRef or adoptPtr, as appropriate. this reminds me that I've always been wondering about checks for allocation failure in WebCore (versus concerns about leaks). A plain call to new may throw an std::bad_alloc exception. If this is not considered, it may leave objects in an invalid state, even when using objects such as RefPtr to wrap arround allocations. I don't remember any specific place in the WebCore code where it would be a problem, I just don't remember seeing any code that deals with allocation failures. Is this a design choice somehow? If so, is there some online documentation/discussion about it? (Tried to google it but found nothing...) Failed allocations in WebKit call CRASH(). This prevents us from ending up in an inconsistent state. Ok, but WebKit is used on devices where allocation failure is somewhat of a realistic possibility, no? Wouldn't it be desirable to handle such a situation gracefully? (I.e. display of an error message when trying to open one more tab rather than crashing the entire browser, for example.) Hopefully I am not missing something obvious. Dunno. The crash-on-allocation failure assumption is baked into lots of lines of code. We do have a tryFastMalloc() path that returns NULL on allocation failure instead of crashing. It's used in some pieces of code that are carefully written to handle an allocation failure gracefully. However, this is rare. In general it's very difficult to recover from an allocation failure in an arbitrary piece of code with an arbitrary callstack. - James Adam ___ 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] Checking allocation failures [was: Re: Naked new considered harmful]
Am 25.08.2010 23:34, schrieb Adam Barth: On Wed, Aug 25, 2010 at 2:31 PM, Stephan Assmussupersti...@gmx.de wrote: Am 25.08.2010 18:35, schrieb Adam Barth: On Wed, Aug 25, 2010 at 7:09 AM, Stephan Assmussupersti...@gmx.de wrote: Am 24.08.2010 19:46, schrieb Adam Barth: One thing Darin and I discussed at WWDC (yes, this email has been a long time coming) is better programming patterns to prevent memory leaks. As I'm sure you know, whenever you allocate a RefCounted object, you need to call adoptRef to prevent a memory leak by adopting the object's initial reference: adoptRef(new FancyRefCountedObject()) We now have an ASSERT that enforces this programming pattern, so we should be able to catch errors pretty easily. Recently, Darin also introduced an analogous function for OwnPtr: adoptPtr(new NiftyNonRefCountedObject()) What adoptPtr does is shove the newly allocated object into a PassOwnPtr, which together with OwnPtr, makes sure we don't leak the object. By using one of these two functions every time we call new, it's easy to see that we don't have any memory leaks. In the cases where we have an intentional memory leak (e.g., for a static), please use the leakPtr() member function to document the leak. In writing new code, please avoid using naked calls to new. If you're making an object that you expect to be heap-allocated, please add a create method, similar to the create method we use for RefCounted objects: static PassOwnPtrNiftyNonRefCountedObject create(ParamClass* param) { return adoptPtr(new NiftyNonRefCountedObject(param)); } You should also make the constructor non-public so folks are forced to call the create method. (Of course, stack-allocated objects should have public constructors.) I'm going through WebCore and removing as many naked news and I can. At some point, we'll add a stylebot rule to flag violations for new code. If you'd like to help out, pick a directory and change all the calls to new to use adoptRef or adoptPtr, as appropriate. this reminds me that I've always been wondering about checks for allocation failure in WebCore (versus concerns about leaks). A plain call to new may throw an std::bad_alloc exception. If this is not considered, it may leave objects in an invalid state, even when using objects such as RefPtr to wrap arround allocations. I don't remember any specific place in the WebCore code where it would be a problem, I just don't remember seeing any code that deals with allocation failures. Is this a design choice somehow? If so, is there some online documentation/discussion about it? (Tried to google it but found nothing...) Failed allocations in WebKit call CRASH(). This prevents us from ending up in an inconsistent state. Ok, but WebKit is used on devices where allocation failure is somewhat of a realistic possibility, no? Wouldn't it be desirable to handle such a situation gracefully? (I.e. display of an error message when trying to open one more tab rather than crashing the entire browser, for example.) Hopefully I am not missing something obvious. Dunno. The crash-on-allocation failure assumption is baked into lots of lines of code. I just thought that if my observations are correct, and on the subject of advertising a certain way to write code (with regards to your initial email), perhaps new code (and eventually old code) should also follow a guideline that allows to handle allocation failures gracefully. For example, if no allocations are to be done in constructors, but rather within a dedicated init() method, objects remain always valid, even if init() throws half-way through, and they could be deallocated gracefully. Best regards, -Stephan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] We need OwnPtrHashMap
On Aug 25, 2010, at 1:46 PM, Geoffrey Garen wrote: Sorry for the late-night webkit-dev spam, but in deploying adoptPtr, I've noticed a number of places where have a HashMap that owns its values as OwnPtrs. Unfortunately, this very clumsy currently. Each instance of this pattern has its own way of hacking around the problem, which might or might not result in memory errors. We really should have an OwnPtrHashMap (to complement RefPtrHashMap) that understands how to handle this case correctly. To clarify: Optimization aside, HashMapRefPtrT, U and HashMapT, RefPtrU work just fine. RefPtrHashMap was an optimization. The feature it needed, which HashMap did not provide, was the ability to look up a value by a non-key type (raw pointer), without first converting to key type (RefPtr), since converting to RefPtr would cause refcount churn. We couldn't find a way to do this using HashTraits without using casts that violated strict aliasing rules. In contrast, HashMapOwnPtrT, U and HashMapT, OwnPtrU don't work at all. They don't compile, since OwnPtr is not copyable. If you made OwnPtr copyable, they would accidentally delete items during get() and resize() operations. I think it is possible to make a specialization of HashMap that secretly uses a HashMap of raw pointers under the covers. That would fix the copies internal to HashTable. However, you'd have to decide on the correct semantics for get/set operations and possibly tweak their return types. In particular, for an OwnPtr value (likely the common case), you'd want get() to return a raw pointer, set() to take a PassOwnPtr, and take() to return a PassOwnPtr. You probably would not want to use OwnPtr anywhere in the API. This doesn't quite entirely match the HashMap contract, but it's close enough to be useful. (Note: I'm probably one of the people with enough template-fu to code this, but I probably won't have time in the very near future.) A HashMap that owns its values wants to do something special when an item is overwritten, removed, or implicitly removed by HashMap destruction, but it doesn't want to do anything special when an item is copied or extracted. I think the best way to achieve this with HashMap might be a hash trait, rather than literally putting an OwnPtr in the map. Specifically, the trait would be one willRemove callback, which took an iterator to an item, and one willRemoveAll callback, which took a begin iterator. These callbacks would default to empty inline functions. But maybe there's a way to use special hash traits and translators to eliminate all or nearly all the C++ copying operations. I don't think HashTraits is the right solution. It doesn't give as good type safety as a HashMap variant that takes PassOwnPtr for all set/add/whatever type functions. Also, HashTraits are about a type, but whether the HashMap takes single ownership is not intrinsic to the type, it is a question for the individual HashMap. You could validly have one HashMap that owns all its values (of type Foo*) and another HashMap with the same value space that does not. The only error would be multiple HashMaps trying to own the same object. Using PassOwnPtr as the way to transfer ownership to the HashMap would prevent this. Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Throwing SECURITY_ERR on cross-origin window.location property accesses
Just to clarify, I'm only looking to add exception throwing to cross-origin property accesses of the location object, not for the whole window object. As for real-world uses of location.href throwing an exception, a new comment just got added to: http://code.google.com/p/chromium/issues/detail?id=17325#c16 Comment 16 by r...@electronicinsight.com, Today (4 minutes ago) There is definitely real-world use case for this for WebKit. At Sprout (http://sproutinc.com) we have a generic platform to design mobile HTML5 ads. We allow designers to link elements in top window and new window. If they choose top window, in Android 2.2 the top window link will just refresh the current page when doing window.top.location.href = url. We need a way to test for the security exception and then do window.open(url) instead when that security error is trapped. Mihai On Tue, Aug 17, 2010 at 3:28 PM, Adam Barth aba...@webkit.org wrote: On Mon, Aug 16, 2010 at 7:49 PM, Maciej Stachowiak m...@apple.com wrote: On Aug 13, 2010, at 9:59 AM, Mihai Parparita wrote: On Fri, Aug 13, 2010 at 12:42 AM, Maciej Stachowiak m...@apple.com wrote: 1) It means the access control goes in fewer places - we don't have to have access control on every document property, only window properties. I'm not quite sure I understand this. At least for the location object, I see an explicit check in JSLocationCustom.cpp: http://trac.webkit.org/browser/trunk/WebCore/bindings/js/JSLocationCustom.cpp#L71. Throwing the exception would happen there too (it won't require custom getters for each property). Yes, a small handful of objects that hang off of Window are accessible cross-origin and have their own security checks. However, one property of Window is handled very differently between WebKit-based browsers and what the spec calls for. someCrossOriginWindow.document returns undefined in WebKit-based browsers, but it returns a document object per the spec which then does its own security check on every property access. This is unfortunate because the attack surface is increased and performance suffers. And it's not Web-compatible to just throw on access to the document property - Web apps expect they can always ask for the document, and it's the *next* property access that throws, which is fulfilled by returning undefined since accessing a property of the undefined value throws in JS. For cases like the location object, it doesn't matter much either way. I believe the spec no longer exposes someCrossOriginWindow.document. (As to whether it throws a security exception or returns null, I'd have to look more carefully.) Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Throwing SECURITY_ERR on cross-origin window.location property accesses
On Mon, Aug 16, 2010 at 2:16 PM, Sam Weinig sam.weinig at gmail.com wrote: I am not sure I agree. Does our behavior actually cause any real bugs in the places you have tracked down? The log spam really doesn't seem like an issue, we can remove it if we want to, but have found it useful in the past. There is definitely a bug and a real-world use case for this for WebKit. At Sprout (http://sproutinc.com) we have a generic platform to design mobile HTML5 ads which are served inside an IFRAME. We allow designers to link elements in top window and new window. If they choose top window, in Android 2.2 when we do window.top.location.href = url instead of going to the URL or just halting code execution, the browser refreshes the current top-level page. We need a way to test for the security exception or at least detect some other property of window.top and then do window.open(url) instead when that security error is trapped or we detect different-origin. -- Rob Barreca Director of Development Sprout, Inc. Mobile: 808.224.1905 Confidential and Proprietary Property of Sprout; Do not distribute. The information contained in this email is confidential. This information is intended for use only by the individual to whom it is addressed. If you are not the intended recipient, you are hereby notified that any use, dissemination, distribution or copying of this communication and its contents is strictly prohibited. If you have received this email in error, please immediately notify the sender by return email and delete this email and attachments, and destroy all copies. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Throwing SECURITY_ERR on cross-origin window.location property accesses
On Aug 25, 2010, at 6:06 PM, Rob Barreca wrote: We need a way to test for the security exception or at least detect some other property of window.top and then do window.open(url) instead when that security error is trapped or we detect different-origin. Does checking the value of window.top.location.href afterward work? Or maybe that doesn’t happen until some unpredictable amount of time later when the load makes progress? -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Throwing SECURITY_ERR on cross-origin window.location property accesses
On Wed, Aug 25, 2010 at 3:23 PM, Darin Adler da...@apple.com wrote: Does checking the value of window.top.location.href afterward work? Or maybe that doesn’t happen until some unpredictable amount of time later when the load makes progress? Hey Darin, thanks for the reply. Unfortunately checking the href afterward does not work. A couple problems, (1) we can't access the value at all because the browser prevents the actual reading of the value since window.top is different-origin so it comes back empty string, and even if we could read the href the big problem at least in Android 2.2 is that (2) the browser refreshes the page when the unsafe JS access happens so the user is already being navigated away in essence. Right now we have a big hack detecting Android 2.2 and always opening links with window.open(), but I know there are other WebKit use-cases we'll need to account for that we haven't got a good repro on yet. Being able to detect different-origin cases ahead of time (or try/catch) would really improve the user experience of this IFRAMEd content. Best, -- Rob Barreca Director of Development Sprout, Inc. Mobile: 808.224.1905 Confidential and Proprietary Property of Sprout; Do not distribute. The information contained in this email is confidential. This information is intended for use only by the individual to whom it is addressed. If you are not the intended recipient, you are hereby notified that any use, dissemination, distribution or copying of this communication and its contents is strictly prohibited. If you have received this email in error, please immediately notify the sender by return email and delete this email and attachments, and destroy all copies. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev