Re: [webkit-dev] Protecting against stale pointers in DrawingBuffer and GraphicsContext3D
On Oct 11, 2010, at 5:15 PM, Maciej Stachowiak wrote: On Oct 11, 2010, at 4:03 PM, Chris Marrin wrote: On Oct 11, 2010, at 3:35 PM, James Robinson wrote: On Mon, Oct 11, 2010 at 3:15 PM, Chris Marrin cmar...@apple.com wrote: For accelerated 2D rendering we created a class called DrawingBuffer. This encapsulates the accelerated drawing surface (usually in the GPU) and the compositing layer used to display that surface on the page. The drawing surface (which is called a Framebuffer Object or FBO) is allocated by GraphicsContext3D, so DrawingBuffer needs a reference to that. Currently this is a weak reference. DrawingBuffer has a ::create() method and you pass the GraphicsContext3D to that method. If you destroy the GraphicsContext3D, DrawingBuffer has a stale pointer. If you were to try to destroy the DrawingBuffer it would attempt to use that pointer (to destroy its FBO) and crash or worse. Currently we have an implicit policy that you should never destroy a GraphicsContext3D before its DrawingBuffers are all destroyed. That works fine in the current use case, CanvasRenderingContext2D. And we can follow that same policy when in the future when we use DrawingBuffer in WebGLRenderingContext. My concern is that this sort of implicit policy can lead to errors in the future when other potential clients of these classes use them. So I posted https://bugs.webkit.org/show_bug.cgi?id=47501. In that patch I move the creation of DrawingBuffer to the GraphicsContext3D and keep back pointers to all the DrawingBuffers allocated so they can be cleaned up when GraphicsContext3D is destroyed. In talking to James R. he's concerned this adds unnecessary complexity and would rather stick with the implicit policy. So is this something I should be concerned about, or is an implicit policy sufficient in this case? Before somebody suggests it, I think Chris and I are in agreement that neither GraphicsContext3D nor DrawingBuffer should be RefCounted. They both have clear single-ownership semantics. True, although Simon and I just chatted and he pointed out that Refcounting both classes would solve this problem. The fact that GraphicsContext3D wouldn't need a back pointer to DrawingBuffer means we wouldn't have any circular references. I don't think the overhead of refcounting is an issue here, so maybe that would be a simpler solution? I think having two independent objects that must be deleted in a specific order, or else you crash, is a hazardous design. APIs (even internal APIs) are better when they do not have a way to be used wrong. So, I think this should be changed one way or the other. I have to say that to my taste, refcounting seems like a cleaner solution than ad-hoc weak pointers. I'm skeptical of the claim that refcounting is not good for a heavyweight object. If there's really a time when special resources (VRAM, etc) need to be freed at a known point in program code, then it may be better to have an explicit close type function instead of counting on the destructor. On the other hand, this might end up being roughly equivalent to the clear backpointers solution, but moves the complexity of being in a possibly-invalid state from DrawingBuffer to GraphicsContext3D. Either way, I am pretty confident that a design where objects must be destroyed in a specific order is not the best choice. So it seems like we have two choices: 1) my current patch, which uses backpointers to manage the lifetime of the weak pointers, or 2) refcounting. My current approach has the advantage that the resources are cleared as soon as the DrawingBuffer is destroyed. But it is more complex and therefore more error prone. I think that complexity is manageable so that would be my preferred implementation. But refcounting is simpler and my current patch has a clear() method on DrawingBuffer which gets rid of all the resources. I could leave that method and change to a refcounted model, so the author can control when the resources are destroyed. What do you think, James? - ~Chris cmar...@apple.com ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Protecting against stale pointers in DrawingBuffer and GraphicsContext3D
On Oct 12, 2010, at 9:47 AM, Chris Marrin wrote: But refcounting is simpler and my current patch has a clear() method on DrawingBuffer which gets rid of all the resources. I could leave that method and change to a refcounted model, so the author can control when the resources are destroyed. What do you think, James? I think that the combination of reference counting and explicit cleanup is a good one, relatively easy to program correctly with, and I would lean toward that combination. The main cost of successfully implementing that design is making sure that each function on DrawingBuffer is either harmless to call after clear() or contains an assertion that it is not called after clear() with some solid reason to know it won’t be called. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Protecting against stale pointers in DrawingBuffer and GraphicsContext3D
On Oct 12, 2010, at 10:44 AM, Darin Adler wrote: On Oct 12, 2010, at 9:47 AM, Chris Marrin wrote: But refcounting is simpler and my current patch has a clear() method on DrawingBuffer which gets rid of all the resources. I could leave that method and change to a refcounted model, so the author can control when the resources are destroyed. What do you think, James? I think that the combination of reference counting and explicit cleanup is a good one, relatively easy to program correctly with, and I would lean toward that combination. The main cost of successfully implementing that design is making sure that each function on DrawingBuffer is either harmless to call after clear() or contains an assertion that it is not called after clear() with some solid reason to know it won’t be called. Ok, I'll redo the patch using that technique. And yes, clear() sets the GraphicsContext3D pointer to 0 and I check that pointer for null in each call. - ~Chris cmar...@apple.com ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Protecting against stale pointers in DrawingBuffer and GraphicsContext3D
On Tue, Oct 12, 2010 at 9:47 AM, Chris Marrin cmar...@apple.com wrote: On Oct 11, 2010, at 5:15 PM, Maciej Stachowiak wrote: On Oct 11, 2010, at 4:03 PM, Chris Marrin wrote: On Oct 11, 2010, at 3:35 PM, James Robinson wrote: On Mon, Oct 11, 2010 at 3:15 PM, Chris Marrin cmar...@apple.com wrote: For accelerated 2D rendering we created a class called DrawingBuffer. This encapsulates the accelerated drawing surface (usually in the GPU) and the compositing layer used to display that surface on the page. The drawing surface (which is called a Framebuffer Object or FBO) is allocated by GraphicsContext3D, so DrawingBuffer needs a reference to that. Currently this is a weak reference. DrawingBuffer has a ::create() method and you pass the GraphicsContext3D to that method. If you destroy the GraphicsContext3D, DrawingBuffer has a stale pointer. If you were to try to destroy the DrawingBuffer it would attempt to use that pointer (to destroy its FBO) and crash or worse. Currently we have an implicit policy that you should never destroy a GraphicsContext3D before its DrawingBuffers are all destroyed. That works fine in the current use case, CanvasRenderingContext2D. And we can follow that same policy when in the future when we use DrawingBuffer in WebGLRenderingContext. My concern is that this sort of implicit policy can lead to errors in the future when other potential clients of these classes use them. So I posted https://bugs.webkit.org/show_bug.cgi?id=47501. In that patch I move the creation of DrawingBuffer to the GraphicsContext3D and keep back pointers to all the DrawingBuffers allocated so they can be cleaned up when GraphicsContext3D is destroyed. In talking to James R. he's concerned this adds unnecessary complexity and would rather stick with the implicit policy. So is this something I should be concerned about, or is an implicit policy sufficient in this case? Before somebody suggests it, I think Chris and I are in agreement that neither GraphicsContext3D nor DrawingBuffer should be RefCounted. They both have clear single-ownership semantics. True, although Simon and I just chatted and he pointed out that Refcounting both classes would solve this problem. The fact that GraphicsContext3D wouldn't need a back pointer to DrawingBuffer means we wouldn't have any circular references. I don't think the overhead of refcounting is an issue here, so maybe that would be a simpler solution? I think having two independent objects that must be deleted in a specific order, or else you crash, is a hazardous design. APIs (even internal APIs) are better when they do not have a way to be used wrong. So, I think this should be changed one way or the other. I have to say that to my taste, refcounting seems like a cleaner solution than ad-hoc weak pointers. I'm skeptical of the claim that refcounting is not good for a heavyweight object. If there's really a time when special resources (VRAM, etc) need to be freed at a known point in program code, then it may be better to have an explicit close type function instead of counting on the destructor. On the other hand, this might end up being roughly equivalent to the clear backpointers solution, but moves the complexity of being in a possibly-invalid state from DrawingBuffer to GraphicsContext3D. Either way, I am pretty confident that a design where objects must be destroyed in a specific order is not the best choice. So it seems like we have two choices: 1) my current patch, which uses backpointers to manage the lifetime of the weak pointers, or 2) refcounting. My current approach has the advantage that the resources are cleared as soon as the DrawingBuffer is destroyed. But it is more complex and therefore more error prone. I think that complexity is manageable so that would be my preferred implementation. But refcounting is simpler and my current patch has a clear() method on DrawingBuffer which gets rid of all the resources. I could leave that method and change to a refcounted model, so the author can control when the resources are destroyed. Another option would be to generalize the WeakPtrT implementation from that patch into a generic class and use that. Then that logic could be implemented, reviewed and tested independently from the graphics code. I know that Maciej has expressed concern about this pattern in the past due to the runtime cost it imposes. Ref counting is a fairly blunt instrument but it would fit in best with the rest of the codepath. - James What do you think, James? - ~Chris cmar...@apple.com ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Protecting against stale pointers in DrawingBuffer and GraphicsContext3D
On Oct 12, 2010, at 12:44 PM, James Robinson wrote: On Tue, Oct 12, 2010 at 9:47 AM, Chris Marrin cmar...@apple.com wrote: On Oct 11, 2010, at 5:15 PM, Maciej Stachowiak wrote: On Oct 11, 2010, at 4:03 PM, Chris Marrin wrote: On Oct 11, 2010, at 3:35 PM, James Robinson wrote: On Mon, Oct 11, 2010 at 3:15 PM, Chris Marrin cmar...@apple.com wrote: For accelerated 2D rendering we created a class called DrawingBuffer. This encapsulates the accelerated drawing surface (usually in the GPU) and the compositing layer used to display that surface on the page. The drawing surface (which is called a Framebuffer Object or FBO) is allocated by GraphicsContext3D, so DrawingBuffer needs a reference to that. Currently this is a weak reference. DrawingBuffer has a ::create() method and you pass the GraphicsContext3D to that method. If you destroy the GraphicsContext3D, DrawingBuffer has a stale pointer. If you were to try to destroy the DrawingBuffer it would attempt to use that pointer (to destroy its FBO) and crash or worse. Currently we have an implicit policy that you should never destroy a GraphicsContext3D before its DrawingBuffers are all destroyed. That works fine in the current use case, CanvasRenderingContext2D. And we can follow that same policy when in the future when we use DrawingBuffer in WebGLRenderingContext. My concern is that this sort of implicit policy can lead to errors in the future when other potential clients of these classes use them. So I posted https://bugs.webkit.org/show_bug.cgi?id=47501. In that patch I move the creation of DrawingBuffer to the GraphicsContext3D and keep back pointers to all the DrawingBuffers allocated so they can be cleaned up when GraphicsContext3D is destroyed. In talking to James R. he's concerned this adds unnecessary complexity and would rather stick with the implicit policy. So is this something I should be concerned about, or is an implicit policy sufficient in this case? Before somebody suggests it, I think Chris and I are in agreement that neither GraphicsContext3D nor DrawingBuffer should be RefCounted. They both have clear single-ownership semantics. True, although Simon and I just chatted and he pointed out that Refcounting both classes would solve this problem. The fact that GraphicsContext3D wouldn't need a back pointer to DrawingBuffer means we wouldn't have any circular references. I don't think the overhead of refcounting is an issue here, so maybe that would be a simpler solution? I think having two independent objects that must be deleted in a specific order, or else you crash, is a hazardous design. APIs (even internal APIs) are better when they do not have a way to be used wrong. So, I think this should be changed one way or the other. I have to say that to my taste, refcounting seems like a cleaner solution than ad-hoc weak pointers. I'm skeptical of the claim that refcounting is not good for a heavyweight object. If there's really a time when special resources (VRAM, etc) need to be freed at a known point in program code, then it may be better to have an explicit close type function instead of counting on the destructor. On the other hand, this might end up being roughly equivalent to the clear backpointers solution, but moves the complexity of being in a possibly-invalid state from DrawingBuffer to GraphicsContext3D. Either way, I am pretty confident that a design where objects must be destroyed in a specific order is not the best choice. So it seems like we have two choices: 1) my current patch, which uses backpointers to manage the lifetime of the weak pointers, or 2) refcounting. My current approach has the advantage that the resources are cleared as soon as the DrawingBuffer is destroyed. But it is more complex and therefore more error prone. I think that complexity is manageable so that would be my preferred implementation. But refcounting is simpler and my current patch has a clear() method on DrawingBuffer which gets rid of all the resources. I could leave that method and change to a refcounted model, so the author can control when the resources are destroyed. Another option would be to generalize the WeakPtrT implementation from that patch into a generic class and use that. Then that logic could be implemented, reviewed and tested independently from the graphics code. I know that Maciej has expressed concern about this pattern in the past due to the runtime cost it imposes. Ref counting is a fairly blunt instrument but it would fit in best with the rest of the codepath. Weak pointers are both more complicated than refcounting and introduce a comparable or possibly even greater level of runtime cost. So if there's ever a problem that can be solved either way, I would tend to prefer refcounting. Regards, Maciej
Re: [webkit-dev] Pixel test experiment
To add a concrete data point, http://trac.webkit.org/changeset/69517 caused a number of SVG tests to fail. It required 14 text rebaselines for Mac and a further two more for Leopard (done by Adam Barth). In order to pass the pixel tests in Chromium, it required 1506 new pixel baselines (checked in by the very brave Albert Wong, http://trac.webkit.org/changeset/69543). None of the rebaselining was done by the patch authors and in general I would not expect a patch author that didn't work in Chromium to be expected to update Chromium-specific baselines. I'm a little skeptical of the claim that all SVG changes are run through the pixel tests given that to date none of the affected platform/mac SVG pixel baselines have been updated. This sort of mass-rebaselining is required fairly regularly for minor changes in SVG and in other parts of the codebase. I'd really like for the bots to run the pixel tests on every run, preferably with 0 tolerance. We catch a lot of regressions by running these tests on the Chromium bots that would probably otherwise go unnoticed. However there is a large maintenance cost associated with this coverage. We normally have two engineers (one in PST, one elsewhere in the world) who watch the Chromium bots to triage, suppress, and rebaseline tests as churn is introduced. Questions: - If the pixel tests were running either with a tolerance of 0 or 0.1, what would the expectation be for a patch like http://trac.webkit.org/changeset/69517 which requires hundreds of pixel rebaselines? Would the patch author be expected to update the baselines for the platform/mac port, or would someone else? Thus far the Chromium folks have been the only ones actively maintaining the pixel baselines - which I think is entirely reasonable since we're the only ones trying to run the pixel tests on bots. - Do we have the tools and infrastructure needed to do mass rebaselines in WebKit currently? We've built a number of tools to deal with the Chromium expectations, but since this has been a need unique to Chromium so far the tools only work for Chromium. - James On Fri, Oct 8, 2010 at 11:18 PM, Nikolas Zimmermann zimmerm...@physik.rwth-aachen.de wrote: Am 08.10.2010 um 20:14 schrieb Jeremy Orlow: I'm not an expert on Pixel tests, but my understanding is that in Chromium (where we've always run with tolerance 0) we've seen real regressions that would have slipped by with something like tolerance 0.1. When you have 0 tolerance, it is more maintenance work, but if we can avoid regressions, it seems worth it. Well, that's why I initially argued for tolerance 0. Especially in SVG we had lots of regressions in the past that were below the 0.1 tolerance. I fully support --tolerance 0 as default. Dirk me are also willing to investigate possible problem sources and minimize them. Reftests as Simon said, are a great thing, but it won't help with official test suites like the W3C one - it would be a huge amount of work to create reftests for all of these... Cheers, Niko ___ 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] Pixel test experiment
On Tue, Oct 12, 2010 at 1:43 PM, James Robinson jam...@google.com wrote: - Do we have the tools and infrastructure needed to do mass rebaselines in WebKit currently? We've built a number of tools to deal with the Chromium expectations, but since this has been a need unique to Chromium so far the tools only work for Chromium. webkit-patch has a very primitive rebaseline command. There are some bugs on file on making it better. Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] ArrayBuffer supprot
On Fri, Oct 8, 2010 at 2:46 PM, Jian Li jia...@chromium.org wrote: Hi, I am looking into hooking up ArrayBuffer support in FileReader and BlobBuilder. It seems that most of the typed array types (ArrayBuffer, ArrayBufferView, Uint8Array, and etc) have already been implemented in WebKit, except TypedArray.get() and DataView. Since most of the other questions have been answered: https://bugs.webkit.org/show_bug.cgi?id=46541 covers implementation of DataView. I will try to take care of this in the next week or two. Let me know if you need it before I've gotten to it. Currently all these typed array supports are put under 3D_CANVAS feature guard. Since they're going to be widely used in other areas, like File API and XHR, should we remove the conditional guard 3D_CANVAS from all the related files? Should we also move these files out of html\canvas, say into html\ or html\typearrays? In addition, get() is not implemented for typed array views. Should we add it? Both the setter and getter in the Web IDL are defined as omittable, which means that in the ECMAScript binding the indexing operator [] is used for both getting and setting values. Both are fully implemented in JSC and V8. At the C++ level, the item and set methods (see IntegralTypedArrayBase and Float32Array for example) are used in certain cases to implement the getter and setter. -Ken Jian ___ 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] ArrayBuffer supprot
On Tue, Oct 12, 2010 at 2:17 PM, Kenneth Russell k...@google.com wrote: On Fri, Oct 8, 2010 at 2:46 PM, Jian Li jia...@chromium.org wrote: Hi, I am looking into hooking up ArrayBuffer support in FileReader and BlobBuilder. It seems that most of the typed array types (ArrayBuffer, ArrayBufferView, Uint8Array, and etc) have already been implemented in WebKit, except TypedArray.get() and DataView. Since most of the other questions have been answered: https://bugs.webkit.org/show_bug.cgi?id=46541 covers implementation of DataView. I will try to take care of this in the next week or two. Let me know if you need it before I've gotten to it. I do not need it in implementing ArrayBuffer support in FileReader and BlobBuilder. Currently I just instantiate a subinstance of ArrayBufferView to get the data for testing purpose. But it would be better if we can have DataView implemented in order to provide a way for the user to read heterogeneous data. Currently all these typed array supports are put under 3D_CANVAS feature guard. Since they're going to be widely used in other areas, like File API and XHR, should we remove the conditional guard 3D_CANVAS from all the related files? Should we also move these files out of html\canvas, say into html\ or html\typearrays? In addition, get() is not implemented for typed array views. Should we add it? Both the setter and getter in the Web IDL are defined as omittable, which means that in the ECMAScript binding the indexing operator [] is used for both getting and setting values. Both are fully implemented in JSC and V8. At the C++ level, the item and set methods (see IntegralTypedArrayBase and Float32Array for example) are used in certain cases to implement the getter and setter. I noticed that and already used it in my test scripts. Thanks for pointing it out. -Ken Jian ___ 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] Protecting against stale pointers in DrawingBuffer and GraphicsContext3D
On Tue, Oct 12, 2010 at 1:20 PM, Maciej Stachowiak m...@apple.com wrote: On Oct 12, 2010, at 12:44 PM, James Robinson wrote: On Tue, Oct 12, 2010 at 9:47 AM, Chris Marrin cmar...@apple.com wrote: On Oct 11, 2010, at 5:15 PM, Maciej Stachowiak wrote: On Oct 11, 2010, at 4:03 PM, Chris Marrin wrote: On Oct 11, 2010, at 3:35 PM, James Robinson wrote: On Mon, Oct 11, 2010 at 3:15 PM, Chris Marrin cmar...@apple.com wrote: For accelerated 2D rendering we created a class called DrawingBuffer. This encapsulates the accelerated drawing surface (usually in the GPU) and the compositing layer used to display that surface on the page. The drawing surface (which is called a Framebuffer Object or FBO) is allocated by GraphicsContext3D, so DrawingBuffer needs a reference to that. Currently this is a weak reference. DrawingBuffer has a ::create() method and you pass the GraphicsContext3D to that method. If you destroy the GraphicsContext3D, DrawingBuffer has a stale pointer. If you were to try to destroy the DrawingBuffer it would attempt to use that pointer (to destroy its FBO) and crash or worse. Currently we have an implicit policy that you should never destroy a GraphicsContext3D before its DrawingBuffers are all destroyed. That works fine in the current use case, CanvasRenderingContext2D. And we can follow that same policy when in the future when we use DrawingBuffer in WebGLRenderingContext. My concern is that this sort of implicit policy can lead to errors in the future when other potential clients of these classes use them. So I posted https://bugs.webkit.org/show_bug.cgi?id=47501. In that patch I move the creation of DrawingBuffer to the GraphicsContext3D and keep back pointers to all the DrawingBuffers allocated so they can be cleaned up when GraphicsContext3D is destroyed. In talking to James R. he's concerned this adds unnecessary complexity and would rather stick with the implicit policy. So is this something I should be concerned about, or is an implicit policy sufficient in this case? Before somebody suggests it, I think Chris and I are in agreement that neither GraphicsContext3D nor DrawingBuffer should be RefCounted. They both have clear single-ownership semantics. True, although Simon and I just chatted and he pointed out that Refcounting both classes would solve this problem. The fact that GraphicsContext3D wouldn't need a back pointer to DrawingBuffer means we wouldn't have any circular references. I don't think the overhead of refcounting is an issue here, so maybe that would be a simpler solution? I think having two independent objects that must be deleted in a specific order, or else you crash, is a hazardous design. APIs (even internal APIs) are better when they do not have a way to be used wrong. So, I think this should be changed one way or the other. I have to say that to my taste, refcounting seems like a cleaner solution than ad-hoc weak pointers. I'm skeptical of the claim that refcounting is not good for a heavyweight object. If there's really a time when special resources (VRAM, etc) need to be freed at a known point in program code, then it may be better to have an explicit close type function instead of counting on the destructor. On the other hand, this might end up being roughly equivalent to the clear backpointers solution, but moves the complexity of being in a possibly-invalid state from DrawingBuffer to GraphicsContext3D. Either way, I am pretty confident that a design where objects must be destroyed in a specific order is not the best choice. So it seems like we have two choices: 1) my current patch, which uses backpointers to manage the lifetime of the weak pointers, or 2) refcounting. My current approach has the advantage that the resources are cleared as soon as the DrawingBuffer is destroyed. But it is more complex and therefore more error prone. I think that complexity is manageable so that would be my preferred implementation. But refcounting is simpler and my current patch has a clear() method on DrawingBuffer which gets rid of all the resources. I could leave that method and change to a refcounted model, so the author can control when the resources are destroyed. Another option would be to generalize the WeakPtrT implementation from that patch into a generic class and use that. Then that logic could be implemented, reviewed and tested independently from the graphics code. I know that Maciej has expressed concern about this pattern in the past due to the runtime cost it imposes. Ref counting is a fairly blunt instrument but it would fit in best with the rest of the codepath. Weak pointers are both more complicated than refcounting and introduce a comparable or possibly even greater level of runtime cost. So if there's ever a problem that can be solved either way, I would tend to prefer refcounting. Regards, Maciej Hmm, I've found weak pointer
Re: [webkit-dev] Pixel test experiment
On Tue, Oct 12, 2010 at 1:43 PM, James Robinson jam...@google.com wrote: To add a concrete data point, http://trac.webkit.org/changeset/69517 caused a number of SVG tests to fail. It required 14 text rebaselines for Mac and a further two more for Leopard (done by Adam Barth). In order to pass the pixel tests in Chromium, it required 1506 new pixel baselines (checked in by the very brave Albert Wong, http://trac.webkit.org/changeset/69543). None of the rebaselining was done by the patch authors and in general I would not expect a patch author that didn't work in Chromium to be expected to update Chromium-specific baselines. I'm a little skeptical of the claim that all SVG changes are run through the pixel tests given that to date none of the affected platform/mac SVG pixel baselines have been updated. This sort of mass-rebaselining is required fairly regularly for minor changes in SVG and in other parts of the codebase. I'd really like for the bots to run the pixel tests on every run, preferably with 0 tolerance. We catch a lot of regressions by running these tests on the Chromium bots that would probably otherwise go unnoticed. However there is a large maintenance cost associated with this coverage. We normally have two engineers (one in PST, one elsewhere in the world) who watch the Chromium bots to triage, suppress, and rebaseline tests as churn is introduced. This isn't to say that it's 2 full time people worth of work to keep them up to date though. Background: Pixel tests and Chromium tests (i.e. tests that touch the full Chromium stack, performance tests, valgrind, etc) both rely on code in Chromium's SVN repo which is why we don't have them on build.webkit.org. One of the major jobs of the WebKit gardener (which James mentioned) is triaging these failures and fixing them, filing upstream bugs, backing out the changes (when they are true regressions and the author is not around and cannot be easily fixed), and/or rebaslining when appropriate. Another major job (and part of why we try to have near 24 hour coverage) is keeping the build from breaking so that our bots don't go blind. So the actual act of rebaslining is only a small component of their job. Questions: - If the pixel tests were running either with a tolerance of 0 or 0.1, what would the expectation be for a patch like http://trac.webkit.org/changeset/69517 which requires hundreds of pixel rebaselines? Would the patch author be expected to update the baselines for the platform/mac port, or would someone else? Thus far the Chromium folks have been the only ones actively maintaining the pixel baselines - which I think is entirely reasonable since we're the only ones trying to run the pixel tests on bots. - Do we have the tools and infrastructure needed to do mass rebaselines in WebKit currently? We've built a number of tools to deal with the Chromium expectations, but since this has been a need unique to Chromium so far the tools only work for Chromium. - James On Fri, Oct 8, 2010 at 11:18 PM, Nikolas Zimmermann zimmerm...@physik.rwth-aachen.de wrote: Am 08.10.2010 um 20:14 schrieb Jeremy Orlow: I'm not an expert on Pixel tests, but my understanding is that in Chromium (where we've always run with tolerance 0) we've seen real regressions that would have slipped by with something like tolerance 0.1. When you have 0 tolerance, it is more maintenance work, but if we can avoid regressions, it seems worth it. Well, that's why I initially argued for tolerance 0. Especially in SVG we had lots of regressions in the past that were below the 0.1 tolerance. I fully support --tolerance 0 as default. Dirk me are also willing to investigate possible problem sources and minimize them. Reftests as Simon said, are a great thing, but it won't help with official test suites like the W3C one - it would be a huge amount of work to create reftests for all of these... Cheers, Niko ___ 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] Protecting against stale pointers in DrawingBuffer and GraphicsContext3D
On Oct 12, 2010, at 2:37 PM, Darin Fisher wrote: ...So it seems like we have two choices: 1) my current patch, which uses backpointers to manage the lifetime of the weak pointers, or 2) refcounting. My current approach has the advantage that the resources are cleared as soon as the DrawingBuffer is destroyed. But it is more complex and therefore more error prone. I think that complexity is manageable so that would be my preferred implementation. But refcounting is simpler and my current patch has a clear() method on DrawingBuffer which gets rid of all the resources. I could leave that method and change to a refcounted model, so the author can control when the resources are destroyed. Another option would be to generalize the WeakPtrT implementation from that patch into a generic class and use that. Then that logic could be implemented, reviewed and tested independently from the graphics code. I know that Maciej has expressed concern about this pattern in the past due to the runtime cost it imposes. Ref counting is a fairly blunt instrument but it would fit in best with the rest of the codepath. Weak pointers are both more complicated than refcounting and introduce a comparable or possibly even greater level of runtime cost. So if there's ever a problem that can be solved either way, I would tend to prefer refcounting. Regards, Maciej Hmm, I've found weak pointer abstractions to be very useful. The issue with reference counting is that it is easy to introduce memory leaks, and has been mentioned, it is sometimes nice to have deterministic object destruction. It is also nice to avoid having to have explicit clear() functions and then add checks to every other method to assert that they are not called after clear(). In the Chromium code base, we have a helper for weak pointers: http://src.chromium.org/viewvc/chrome/trunk/src/base/weak_ptr.h?view=markup We tend to use this in cases in which: 1) there are many consumers interested in holding a back pointer to some shared resource, and 2) we'd like the shared resource to die at some predictable time. Without a utility like this, the alternative is to make the shared resource notify each of the consumers about the impending destruction of the shared resource. It is true that WeakPtrT adds a null check in front of each method call made by the consumers, but that runtime cost is often justified in exchange for reduced code complexity (i.e., eliminating the need to notify consumers when the shared resource dies). In this case I agree with Maciej that the simplest solution is to just use a RefPtr. This is a simple case where a class (DrawingBuffer) must not outlive the GraphicsContext3D used to create it. I have a patch which uses RefPtrs and it simplifies things quite a bit. I'm not too concerned about resource management. I think the typical case will be either that DrawingBuffer and GraphicsContext3D are destroyed at around the same time, or that GraphicsContext3D is persistent and DrawingBuffers come and go. The RefPtr just makes sure that the GraphicsContext3D is never destroyed too early. With that said, there are some places in this area of the code that would benefit from a general WeakPtr pattern. For instance, the WebGLObject set of classes use an ad hoc weak pointer mechanism which would be more readable and reliable with a WeakPtr implementation. I think the accelerated 2D Canvas logic may have some unprotected weak pointers as well (for instance the Shader objects). - ~Chris cmar...@apple.com ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Protecting against stale pointers in DrawingBuffer and GraphicsContext3D
On Oct 12, 2010, at 2:37 PM, Darin Fisher wrote: On Tue, Oct 12, 2010 at 1:20 PM, Maciej Stachowiak m...@apple.com wrote: Hmm, I've found weak pointer abstractions to be very useful. The issue with reference counting is that it is easy to introduce memory leaks, and has been mentioned, it is sometimes nice to have deterministic object destruction. We used to have lots of problems with leaking refcounted objects back when most refcounting was done manually, but this has become much less common as we deployed smart pointers. It is also nice to avoid having to have explicit clear() functions and then add checks to every other method to assert that they are not called after clear(). Well, if you have a weak pointer you have to check weak pointer validity, so you don't save any checks. I don't think explicit close calls are that bad. I think it's actually a better pattern when you hold significant resources other than just some memory (e.g. VRAM, file handles, sockets, limited kernel resources, window server handles...). That way, cleaning up your external resources does not become dependent on your ownership strategy for the object. In the Chromium code base, we have a helper for weak pointers: http://src.chromium.org/viewvc/chrome/trunk/src/base/weak_ptr.h?view=markup We tend to use this in cases in which: 1) there are many consumers interested in holding a back pointer to some shared resource, and 2) we'd like the shared resource to die at some predictable time. Without a utility like this, the alternative is to make the shared resource notify each of the consumers about the impending destruction of the shared resource. It is true that WeakPtrT adds a null check in front of each method call made by the consumers, but that runtime cost is often justified in exchange for reduced code complexity (i.e., eliminating the need to notify consumers when the shared resource dies). The cost isn't just the null checks. You need either storage for all the backpointers, or the smaller storage overhead for a refcounted handle. But then almost by definition you have more overhead than refcounting, since you have a refcounted object plus more work. I do think weak pointers have their uses, but only in relatively unusual cases (e.g. when you'd have a cycle otherwise). It doesn't sound like a huge win in this case. Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Protecting against stale pointers in DrawingBuffer and GraphicsContext3D
On Tue, Oct 12, 2010 at 3:46 PM, Maciej Stachowiak m...@apple.com wrote:
On Oct 12, 2010, at 2:37 PM, Darin Fisher wrote:
On Tue, Oct 12, 2010 at 1:20 PM, Maciej Stachowiak m...@apple.com wrote:
Hmm, I've found weak pointer abstractions to be very useful. The issue
with reference counting is that it is easy to introduce memory leaks, and
has been mentioned, it is sometimes nice to have deterministic object
destruction.
We used to have lots of problems with leaking refcounted objects back when
most refcounting was done manually, but this has become much less common as
we deployed smart pointers.
It is also nice to avoid having to have explicit clear() functions and then
add checks to every other method to assert that they are not called after
clear().
Well, if you have a weak pointer you have to check weak pointer validity,
so you don't save any checks. I don't think explicit close calls are that
bad. I think it's actually a better pattern when you hold significant
resources other than just some memory (e.g. VRAM, file handles, sockets,
limited kernel resources, window server handles...). That way, cleaning up
your external resources does not become dependent on your ownership strategy
for the object.
In the Chromium code base, we have a helper for weak pointers:
http://src.chromium.org/viewvc/chrome/trunk/src/base/weak_ptr.h?view=markup
We tend to use this in cases in which:
1) there are many consumers interested in holding a back pointer to some
shared resource, and
2) we'd like the shared resource to die at some predictable time.
Without a utility like this, the alternative is to make the shared resource
notify each of the consumers about the impending destruction of the shared
resource.
It is true that WeakPtrT adds a null check in front of each method call
made by the consumers, but that runtime cost is often justified in exchange
for reduced code complexity (i.e., eliminating the need to notify consumers
when the shared resource dies).
The cost isn't just the null checks. You need either storage for all the
backpointers, or the smaller storage overhead for a refcounted handle. But
then almost by definition you have more overhead than refcounting, since you
have a refcounted object plus more work.
I do think weak pointers have their uses, but only in relatively unusual
cases (e.g. when you'd have a cycle otherwise). It doesn't sound like a huge
win in this case.
Another other advantage of weak pointers over refcounted pointers is that
with a combination of OwnPtr/WeakPtr it that the ownership is explicit
and easy to examine locally. By this I mean if I have:
class A {
OwnPtrT m_t;
};
class B {
private:
WeakPtrT m_t;
};
it's immediately and locally obvious which object is responsible for the
lifetime of the T. With the alternative:
class A {
RefPtrT m_t;
};
class B {
RefPtrT m_t;
};
there's no way to know which class drives the lifetime of the T without
reading through the definitions of both A and B (and any other class that
holds a RefPtr). This makes it much harder to review changes to A or to B
without carefully reading through many mostly-unrelated classes. This is
especially true when
A concrete example of this is LayerRendererChromium which used to have an
OwnPtrGraphicsContext3D member variable, took a
PassOwnPtrGraphicsContext3D parameter in its create() factory function and
exposed a GraphicsContext3D* getter. The lifetime of the GraphicsContext3D
associated with the compositor was crystal clear just by examining
LayerRendererChromium.h - it took exclusive ownership of the context at
creation time and retained exclusive ownership for the lifetime of the
LayerRendererChromium. Now that GraphicsContext3D is ref counted, it's not
nearly as obvious whether the LayerRendererChromium is supposed to be the
exclusive owner of the underlying context or whether callers might extend
its lifetime (especially since the GraphicsContext3D* getter now allows
callers to grab their own references). Ambiguous and non-local ownership
semantics makes code harder to review and reason about.
- James
Regards,
Maciej
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Protecting against stale pointers in DrawingBuffer and GraphicsContext3D
On Tue, Oct 12, 2010 at 3:46 PM, Maciej Stachowiak m...@apple.com wrote: On Oct 12, 2010, at 2:37 PM, Darin Fisher wrote: On Tue, Oct 12, 2010 at 1:20 PM, Maciej Stachowiak m...@apple.com wrote: Hmm, I've found weak pointer abstractions to be very useful. The issue with reference counting is that it is easy to introduce memory leaks, and has been mentioned, it is sometimes nice to have deterministic object destruction. We used to have lots of problems with leaking refcounted objects back when most refcounting was done manually, but this has become much less common as we deployed smart pointers. I was referring to reference cycles that are just as easily (if not more easily since the reference counting is hidden) created when you use smart pointers for managing references. It is also nice to avoid having to have explicit clear() functions and then add checks to every other method to assert that they are not called after clear(). Well, if you have a weak pointer you have to check weak pointer validity, so you don't save any checks. True. The difference is that the null checks are done at the call site instead. In the abstract sense, what's the difference, right? I agree, but In some cases however I find it nice when writing a class to have fewer states: after construction, you can call methods; post destruction, you cannot call methods. With an intermediate clear() function, you now have an uninitialized state to contend with. This can make some classes a bit more noisy / less readable. I don't think explicit close calls are that bad. I think it's actually a better pattern when you hold significant resources other than just some memory (e.g. VRAM, file handles, sockets, limited kernel resources, window server handles...). That way, cleaning up your external resources does not become dependent on your ownership strategy for the object. I agree. Reference counting has many great use cases. I'm not at all against it. I just find that there are use cases for smart weak pointers sometimes. In the Chromium code base, we have a helper for weak pointers: http://src.chromium.org/viewvc/chrome/trunk/src/base/weak_ptr.h?view=markup We tend to use this in cases in which: 1) there are many consumers interested in holding a back pointer to some shared resource, and 2) we'd like the shared resource to die at some predictable time. Without a utility like this, the alternative is to make the shared resource notify each of the consumers about the impending destruction of the shared resource. It is true that WeakPtrT adds a null check in front of each method call made by the consumers, but that runtime cost is often justified in exchange for reduced code complexity (i.e., eliminating the need to notify consumers when the shared resource dies). The cost isn't just the null checks. You need either storage for all the backpointers, or the smaller storage overhead for a refcounted handle. But then almost by definition you have more overhead than refcounting, since you have a refcounted object plus more work. That's right. I wouldn't use WeakPtr unless that overhead was worth it. It is not that normal for WeakPtr to be used in Chrome. There is one very notable example, which I'll come back to. Note the use case restrictions I sited above (see: We tend to use this in cases in which:). Let me elaborate on that. If there are many consumers interested in a shared resource _and_ if I'd like the shared resource to have deterministic destruction, then enter WeakPtr. Without it, I would need to keep a list of consumers registered on the shared resource, and have some kind of notification from the destructor of the shared resource sent out to each registered consumer. In that notification, the consumers would need to clear their back pointers. This implementation without WeakPtr involves a list of pointers to maintain as well as a loop in the destructor of the shared resource. Each of the consumers still needs to null check the shared resource before dereferencing it. Clearly, WeakPtr is superior in this case. I'm not sure how often the above pattern appears in WebKit. Perhaps it is rare. I do think weak pointers have their uses, but only in relatively unusual cases (e.g. when you'd have a cycle otherwise). It doesn't sound like a huge win in this case. WebCore::Timer is a good example of a smart weak pointer class. It is intended to be allocated as a member variable and hold a back pointer to the containing class. When the containing class object is destroyed, the Timer member is also destroyed, and any outstanding timer event is cancelled. In Chromium, we also have a mechanism to call methods on a class asynchronously. This mechanism builds on top of WeakPtr. There is a templatized factory class that you instantiate as a member of the class you want to call methods on asynchronously. (It is called ScopedRunnableMethodFactory.)
Re: [webkit-dev] Protecting against stale pointers in DrawingBuffer and GraphicsContext3D
On Tue, Oct 12, 2010 at 6:37 PM, James Robinson jam...@google.com wrote:
On Tue, Oct 12, 2010 at 3:46 PM, Maciej Stachowiak m...@apple.com wrote:
On Oct 12, 2010, at 2:37 PM, Darin Fisher wrote:
On Tue, Oct 12, 2010 at 1:20 PM, Maciej Stachowiak m...@apple.com wrote:
Hmm, I've found weak pointer abstractions to be very useful. The issue
with reference counting is that it is easy to introduce memory leaks, and
has been mentioned, it is sometimes nice to have deterministic object
destruction.
We used to have lots of problems with leaking refcounted objects back when
most refcounting was done manually, but this has become much less common as
we deployed smart pointers.
It is also nice to avoid having to have explicit clear() functions and
then add checks to every other method to assert that they are not called
after clear().
Well, if you have a weak pointer you have to check weak pointer validity,
so you don't save any checks. I don't think explicit close calls are that
bad. I think it's actually a better pattern when you hold significant
resources other than just some memory (e.g. VRAM, file handles, sockets,
limited kernel resources, window server handles...). That way, cleaning up
your external resources does not become dependent on your ownership strategy
for the object.
In the Chromium code base, we have a helper for weak pointers:
http://src.chromium.org/viewvc/chrome/trunk/src/base/weak_ptr.h?view=markup
We tend to use this in cases in which:
1) there are many consumers interested in holding a back pointer to some
shared resource, and
2) we'd like the shared resource to die at some predictable time.
Without a utility like this, the alternative is to make the shared
resource notify each of the consumers about the impending destruction of the
shared resource.
It is true that WeakPtrT adds a null check in front of each method call
made by the consumers, but that runtime cost is often justified in exchange
for reduced code complexity (i.e., eliminating the need to notify consumers
when the shared resource dies).
The cost isn't just the null checks. You need either storage for all the
backpointers, or the smaller storage overhead for a refcounted handle. But
then almost by definition you have more overhead than refcounting, since you
have a refcounted object plus more work.
I do think weak pointers have their uses, but only in relatively unusual
cases (e.g. when you'd have a cycle otherwise). It doesn't sound like a huge
win in this case.
Another other advantage of weak pointers over refcounted pointers is that
with a combination of OwnPtr/WeakPtr it that the ownership is explicit
and easy to examine locally. By this I mean if I have:
class A {
OwnPtrT m_t;
};
class B {
private:
WeakPtrT m_t;
};
it's immediately and locally obvious which object is responsible for the
lifetime of the T. With the alternative:
class A {
RefPtrT m_t;
};
class B {
RefPtrT m_t;
};
^^^ Looks like a memory leak too :-)
One then hopes for up-to-date documentation about how the memory leak is
broken.
-Darin
there's no way to know which class drives the lifetime of the T without
reading through the definitions of both A and B (and any other class that
holds a RefPtr). This makes it much harder to review changes to A or to B
without carefully reading through many mostly-unrelated classes. This is
especially true when
A concrete example of this is LayerRendererChromium which used to have an
OwnPtrGraphicsContext3D member variable, took a
PassOwnPtrGraphicsContext3D parameter in its create() factory function and
exposed a GraphicsContext3D* getter. The lifetime of the GraphicsContext3D
associated with the compositor was crystal clear just by examining
LayerRendererChromium.h - it took exclusive ownership of the context at
creation time and retained exclusive ownership for the lifetime of the
LayerRendererChromium. Now that GraphicsContext3D is ref counted, it's not
nearly as obvious whether the LayerRendererChromium is supposed to be the
exclusive owner of the underlying context or whether callers might extend
its lifetime (especially since the GraphicsContext3D* getter now allows
callers to grab their own references). Ambiguous and non-local ownership
semantics makes code harder to review and reason about.
- James
Regards,
Maciej
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Protecting against stale pointers in DrawingBuffer and GraphicsContext3D
Not sure it is relevant to this discussion, but WebKit had a weak pointer class back in 2002 and I removed it. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Protecting against stale pointers in DrawingBuffer and GraphicsContext3D
Caused more harm than good? -Darin On Tue, Oct 12, 2010 at 10:06 PM, Darin Adler da...@apple.com wrote: Not sure it is relevant to this discussion, but WebKit had a weak pointer class back in 2002 and I removed it. -- 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] Pixel test experiment
Does it support pixel test updates? Is it possible to extend this tool if not? This would limit the maintenance cost and every commiter should rebaseline mac if the change is a progression, or the difference is machine dependent (but not OS dependent). Dirk Am 12.10.2010 um 22:49 schrieb Adam Barth: On Tue, Oct 12, 2010 at 1:43 PM, James Robinson jam...@google.com wrote: - Do we have the tools and infrastructure needed to do mass rebaselines in WebKit currently? We've built a number of tools to deal with the Chromium expectations, but since this has been a need unique to Chromium so far the tools only work for Chromium. webkit-patch has a very primitive rebaseline command. There are some bugs on file on making it better. 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] Protecting against stale pointers in DrawingBuffer and GraphicsContext3D
On Oct 12, 2010, at 10:03 PM, Darin Fisher wrote: On Tue, Oct 12, 2010 at 3:46 PM, Maciej Stachowiak m...@apple.com wrote: On Oct 12, 2010, at 2:37 PM, Darin Fisher wrote: On Tue, Oct 12, 2010 at 1:20 PM, Maciej Stachowiak m...@apple.com wrote: Hmm, I've found weak pointer abstractions to be very useful. The issue with reference counting is that it is easy to introduce memory leaks, and has been mentioned, it is sometimes nice to have deterministic object destruction. We used to have lots of problems with leaking refcounted objects back when most refcounting was done manually, but this has become much less common as we deployed smart pointers. I was referring to reference cycles that are just as easily (if not more easily since the reference counting is hidden) created when you use smart pointers for managing references. For whatever reason, we don't seem to have had a lot of leaks from reference cycles. Nearly all the ones I ended up investigating back in the day turned out to be a ref() with no paired deref(). I do believe that it's a real risk, just not one that has hit us a lot. It is also nice to avoid having to have explicit clear() functions and then add checks to every other method to assert that they are not called after clear(). Well, if you have a weak pointer you have to check weak pointer validity, so you don't save any checks. True. The difference is that the null checks are done at the call site instead. In the abstract sense, what's the difference, right? I agree, but In some cases however I find it nice when writing a class to have fewer states: after construction, you can call methods; post destruction, you cannot call methods. With an intermediate clear() function, you now have an uninitialized state to contend with. This can make some classes a bit more noisy / less readable. The my weak pointer has now cleared itself state is exactly the same as the someone explicitly asked me to release some resources state. You deal with it either by making methods no-ops after that point, or making it illegal to call them and enforce it with an assert. It's exactly isomorphic in that regard. The cost isn't just the null checks. You need either storage for all the backpointers, or the smaller storage overhead for a refcounted handle. But then almost by definition you have more overhead than refcounting, since you have a refcounted object plus more work. That's right. I wouldn't use WeakPtr unless that overhead was worth it. It is not that normal for WeakPtr to be used in Chrome. There is one very notable example, which I'll come back to. Note the use case restrictions I sited above (see: We tend to use this in cases in which:). Let me elaborate on that. If there are many consumers interested in a shared resource _and_ if I'd like the shared resource to have deterministic destruction, then enter WeakPtr. Without it, I would need to keep a list of consumers registered on the shared resource, and have some kind of notification from the destructor of the shared resource sent out to each registered consumer. In that notification, the consumers would need to clear their back pointers. This implementation without WeakPtr involves a list of pointers to maintain as well as a loop in the destructor of the shared resource. Each of the consumers still needs to null check the shared resource before dereferencing it. Clearly, WeakPtr is superior in this case. I'm not sure how often the above pattern appears in WebKit. Perhaps it is rare. It's possible it will come up. So far, the use cases we have had for clearing backpointers have usually involved a unique client that the object which may get destroyed first already has a pointer to. If broadcast death notification was a more common pattern, I would be all in favor of WeakPtr. As things stand, I'd worry that people would use it for unicast death notifaction, which would be a little wasteful. I do think weak pointers have their uses, but only in relatively unusual cases (e.g. when you'd have a cycle otherwise). It doesn't sound like a huge win in this case. WebCore::Timer is a good example of a smart weak pointer class. It is intended to be allocated as a member variable and hold a back pointer to the containing class. When the containing class object is destroyed, the Timer member is also destroyed, and any outstanding timer event is cancelled. In Chromium, we also have a mechanism to call methods on a class asynchronously. This mechanism builds on top of WeakPtr. There is a templatized factory class that you instantiate as a member of the class you want to call methods on asynchronously. (It is called ScopedRunnableMethodFactory.) When you want to schedule an asynchronous method call back on yourself, you ask the factory member to create a closure object, which when run
Re: [webkit-dev] Protecting against stale pointers in DrawingBuffer and GraphicsContext3D
I don't know if this is what Darin (Adler) is referring to, but at one point I think there was a special weak pointer for Nodes to point back to their Document. This turned out to be the wrong design since for Web-compatible behavior you really want the document to stay alive while the node is alive. - Maciej On Oct 12, 2010, at 10:12 PM, Darin Fisher wrote: Caused more harm than good? -Darin On Tue, Oct 12, 2010 at 10:06 PM, Darin Adler da...@apple.com wrote: Not sure it is relevant to this discussion, but WebKit had a weak pointer class back in 2002 and I removed it. -- 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
