Bug 103914 - Add support for tracking hit test rectangles to enable fast event rejection in the compositor
: Add support for tracking hit test rectangles to enable fast event rejection i...
Status: RESOLVED FIXED
: WebKit
Event Handling
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To:
: https://code.google.com/p/chromium/is...
:
: 105048
:
  Show dependency treegraph
 
Reported: 2012-12-03 11:45 PST by
Modified: 2012-12-17 13:43 PST (History)


Attachments
Patch (47.31 KB, patch)
2012-12-03 12:08 PST, Levi Weintraub
no flags Review Patch | Details | Formatted Diff | Diff
Patch (48.36 KB, patch)
2012-12-03 18:10 PST, Levi Weintraub
no flags Review Patch | Details | Formatted Diff | Diff
Patch (48.35 KB, patch)
2012-12-04 10:33 PST, Levi Weintraub
no flags Review Patch | Details | Formatted Diff | Diff
Patch (48.33 KB, patch)
2012-12-04 14:44 PST, Levi Weintraub
no flags Review Patch | Details | Formatted Diff | Diff
Patch (48.36 KB, patch)
2012-12-04 17:22 PST, Levi Weintraub
no flags Review Patch | Details | Formatted Diff | Diff
Patch (50.82 KB, patch)
2012-12-07 12:35 PST, Levi Weintraub
no flags Review Patch | Details | Formatted Diff | Diff
Patch (50.93 KB, patch)
2012-12-07 14:09 PST, Levi Weintraub
no flags Review Patch | Details | Formatted Diff | Diff
Patch (50.91 KB, patch)
2012-12-07 14:41 PST, Levi Weintraub
no flags Review Patch | Details | Formatted Diff | Diff
Patch (50.94 KB, patch)
2012-12-10 10:02 PST, Levi Weintraub
no flags Review Patch | Details | Formatted Diff | Diff
Simplified solution (33.71 KB, patch)
2012-12-11 12:16 PST, Levi Weintraub
no flags Review Patch | Details | Formatted Diff | Diff
Patch (32.82 KB, patch)
2012-12-11 14:19 PST, Levi Weintraub
no flags Review Patch | Details | Formatted Diff | Diff
Patch for landing (28.66 KB, patch)
2012-12-12 16:15 PST, Levi Weintraub
no flags Review Patch | Details | Formatted Diff | Diff
Patch for landing (36.46 KB, patch)
2012-12-12 16:20 PST, Levi Weintraub
no flags Review Patch | Details | Formatted Diff | Diff
Patch for landing (36.44 KB, patch)
2012-12-12 16:45 PST, Levi Weintraub
no flags Review Patch | Details | Formatted Diff | Diff
Patch (40.85 KB, patch)
2012-12-14 17:30 PST, Levi Weintraub
webkit.review.bot: commit‑queue-
Review Patch | Details | Formatted Diff | Diff
Patch (39.54 KB, patch)
2012-12-16 19:08 PST, Levi Weintraub
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2012-12-03 11:45:51 PST
When the WebKit thread is in heavy use, it can be faster to perform a first-pass hit test in the compositor before handing the event to WebKit. This change adds support for tracking the rects associated with an event target with an event handler, and begins doing so for Touch Events on Chromium.
------- Comment #1 From 2012-12-03 12:08:13 PST -------
Created an attachment (id=177303) [details]
Patch
------- Comment #2 From 2012-12-03 12:22:55 PST -------
Do we know how iOS survives w/o this? :)
------- Comment #3 From 2012-12-03 12:38:21 PST -------
(From update of attachment 177303 [details])
Attachment 177303 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15104615
------- Comment #4 From 2012-12-03 12:47:38 PST -------
(From update of attachment 177303 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=177303&action=review

> Source/WebCore/rendering/RenderInline.cpp:1437
> +    Vector<IntRect> result;

here and elsewhere - can you use a Region for this? It would accumulate overlapping / enclosed rects in a slightly nicer way
------- Comment #5 From 2012-12-03 12:59:45 PST -------
(From update of attachment 177303 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=177303&action=review

> Source/WebCore/rendering/RenderBlock.cpp:6931
> +void RenderBlock::addLayerHitTestRects(LayerHitTestRects& layerRects, const RenderLayer* currentCompositedLayer, const LayoutPoint& layerOffset) const

I don't understand why you're accumulating these rects relative to some RenderLayer. Shouldn't they all just be root-relative?

Also the layerOffset stuff won't work with software-rendered transforms.

> Source/WebCore/rendering/RenderBlock.cpp:6960
> +    if (width() && height())
> +        result.append(pixelSnappedIntRect(adjustedLayerOffset, size()));
> +
> +    if (hasHorizontalLayoutOverflow() || hasVerticalLayoutOverflow()) {
> +        for (RootInlineBox* curr = firstRootBox(); curr; curr = curr->nextRootBox()) {
> +            LayoutUnit top = max<LayoutUnit>(curr->lineTop(), curr->top());
> +            LayoutUnit bottom = min<LayoutUnit>(curr->lineBottom(), curr->top() + curr->height());
> +            LayoutRect rect(adjustedLayerOffset.x() + curr->x(), adjustedLayerOffset.y() + top, curr->width(), bottom - top);
> +            if (!rect.isEmpty())
> +                result.append(pixelSnappedIntRect(rect));
> +        }
> +    }
> +
> +    for (RenderObject* curr = firstChild(); curr; curr = curr->nextSibling()) {
> +        if (!curr->isText() && !curr->isListMarker() && curr->isBox()) {
> +            RenderBox* box = toRenderBox(curr);
> +            box->addLayerHitTestRects(layerRects, currentCompositedLayer,  adjustedLayerOffset + box->locationOffset());
> +        }
> +    }

Rather than have all this code here, can we make a generic RenderObject method for computing the hit-test region for an arbitrary element?

> Source/WebCore/rendering/RenderObject.cpp:629
> +        // Don't apply transforms as we should never pass through one.
> +        mapLocalToContainer((*enclosingCompositedLayer)->renderer(), transformState, ApplyContainerFlip | SnapOffsetForTransforms);

Sure you will; 2D transforms don't always make compositing layers.
------- Comment #6 From 2012-12-03 13:23:43 PST -------
(From update of attachment 177303 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=177303&action=review

>> Source/WebCore/rendering/RenderBlock.cpp:6931
>> +void RenderBlock::addLayerHitTestRects(LayerHitTestRects& layerRects, const RenderLayer* currentCompositedLayer, const LayoutPoint& layerOffset) const
> 
> I don't understand why you're accumulating these rects relative to some RenderLayer. Shouldn't they all just be root-relative?
> 
> Also the layerOffset stuff won't work with software-rendered transforms.

Accumulating them relative to their enclosing composited layer allows us to hit test those layers in the compositor properly when 3d transforms are involved, ie as opposed to taking an enclosing bounding box in the root's space.

>> Source/WebCore/rendering/RenderBlock.cpp:6960
>> +    }
> 
> Rather than have all this code here, can we make a generic RenderObject method for computing the hit-test region for an arbitrary element?

With the differing implementations for boxes, blocks, and inlines, it seems uglier to me to probe for types, but it's certainly do-able.

>> Source/WebCore/rendering/RenderInline.cpp:1437
>> +    Vector<IntRect> result;
> 
> here and elsewhere - can you use a Region for this? It would accumulate overlapping / enclosed rects in a slightly nicer way

Sure.

>> Source/WebCore/rendering/RenderObject.cpp:629
>> +        mapLocalToContainer((*enclosingCompositedLayer)->renderer(), transformState, ApplyContainerFlip | SnapOffsetForTransforms);
> 
> Sure you will; 2D transforms don't always make compositing layers.

Good catch, thanks!
------- Comment #7 From 2012-12-03 13:27:01 PST -------
(From update of attachment 177303 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=177303&action=review

>>> Source/WebCore/rendering/RenderInline.cpp:1437
>>> +    Vector<IntRect> result;
>> 
>> here and elsewhere - can you use a Region for this? It would accumulate overlapping / enclosed rects in a slightly nicer way
> 
> Sure.

Very very slowly. :)  Unioning N rects is N^2 with Region. :(  See bug 98800
------- Comment #8 From 2012-12-03 13:33:02 PST -------
(In reply to comment #6)
> (From update of attachment 177303 [details] [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=177303&action=review
> 
> >> Source/WebCore/rendering/RenderBlock.cpp:6931
> >> +void RenderBlock::addLayerHitTestRects(LayerHitTestRects& layerRects, const RenderLayer* currentCompositedLayer, const LayoutPoint& layerOffset) const
> > 
> > I don't understand why you're accumulating these rects relative to some RenderLayer. Shouldn't they all just be root-relative?
> > 
> > Also the layerOffset stuff won't work with software-rendered transforms.
> 
> Accumulating them relative to their enclosing composited layer allows us to hit test those layers in the compositor properly when 3d transforms are involved, ie as opposed to taking an enclosing bounding box in the root's space.

I see. That does seem rather platform-specific, though. You could track root-relative quads.

On iOS, we track them all root-relative (as bounding boxes). It would be nice to be able to share code here.

> >> Source/WebCore/rendering/RenderBlock.cpp:6960
> >> +    }
> > 
> > Rather than have all this code here, can we make a generic RenderObject method for computing the hit-test region for an arbitrary element?
> 
> With the differing implementations for boxes, blocks, and inlines, it seems uglier to me to probe for types, but it's certainly do-able.

Use virtual methods? It would be akin to absoluteQuads().
------- Comment #9 From 2012-12-03 13:33:56 PST -------
Also, do you track rects per-frame, or just on the root including all subframes?
------- Comment #10 From 2012-12-03 14:09:15 PST -------
(From update of attachment 177303 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=177303&action=review

>>>> Source/WebCore/rendering/RenderBlock.cpp:6931
>>>> +void RenderBlock::addLayerHitTestRects(LayerHitTestRects& layerRects, const RenderLayer* currentCompositedLayer, const LayoutPoint& layerOffset) const
>>> 
>>> I don't understand why you're accumulating these rects relative to some RenderLayer. Shouldn't they all just be root-relative?
>>> 
>>> Also the layerOffset stuff won't work with software-rendered transforms.
>> 
>> Accumulating them relative to their enclosing composited layer allows us to hit test those layers in the compositor properly when 3d transforms are involved, ie as opposed to taking an enclosing bounding box in the root's space.
> 
> I see. That does seem rather platform-specific, though. You could track root-relative quads.
> 
> On iOS, we track them all root-relative (as bounding boxes). It would be nice to be able to share code here.

Root-relative boxes or quads would be possible, it just requires a lot more layer walking.

I'd agree it'd be nice to share code. Is the code used to generate these bounding boxes on iOS available to share? Obviously, the standard bounding box code doesn't actually mirror hit testing, but if there's something that does this already I'd love to re-use it.

>>>> Source/WebCore/rendering/RenderBlock.cpp:6960
>>>> +    }
>>> 
>>> Rather than have all this code here, can we make a generic RenderObject method for computing the hit-test region for an arbitrary element?
>> 
>> With the differing implementations for boxes, blocks, and inlines, it seems uglier to me to probe for types, but it's certainly do-able.
> 
> Use virtual methods? It would be akin to absoluteQuads().

If our goal is to generate all the absolute bounding boxes or quads, we can simplify the virtual implementations.

>>>> Source/WebCore/rendering/RenderInline.cpp:1437
>>>> +    Vector<IntRect> result;
>>> 
>>> here and elsewhere - can you use a Region for this? It would accumulate overlapping / enclosed rects in a slightly nicer way
>> 
>> Sure.
> 
> Very very slowly. :)  Unioning N rects is N^2 with Region. :(  See bug 98800

I originally implemented it with Regions, but after looking at the accumulation code there I changed my path.
------- Comment #11 From 2012-12-03 16:43:02 PST -------
Doing everything viewport-relative would meet our short-term needs, but doing hit testing on child layers is better long term (since it's more precise) and something I would expect the ScrollingTree logic to be able to handle eventually even on iOS.
------- Comment #12 From 2012-12-03 17:12:01 PST -------
The scrolling tree isn't about layers; it's about "islands of scrollability".
------- Comment #13 From 2012-12-03 17:28:38 PST -------
OK(In reply to comment #12)
> The scrolling tree isn't about layers; it's about "islands of scrollability".

OK, well I think the points still stands - you'd want to be able to hit test on each "island" separately since they will move around relative to each other.  I don't really know what the point of trying to abstract here is since the only useful islands of scrollability are ones you can move (i.e. layers)
------- Comment #14 From 2012-12-03 18:10:34 PST -------
Created an attachment (id=177386) [details]
Patch
------- Comment #15 From 2012-12-03 18:14:09 PST -------
(In reply to comment #14)
> Created an attachment (id=177386) [details] [details]
> Patch

This patch switches to using repaint containers, but it doesn't drastically re-architect the method for accumulating the rects. I found trying to do the tree walk in a separate non-virtual function to be quite ugly.
------- Comment #16 From 2012-12-03 18:16:22 PST -------
(In reply to comment #4)
> (From update of attachment 177303 [details] [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=177303&action=review
> 
> > Source/WebCore/rendering/RenderInline.cpp:1437
> > +    Vector<IntRect> result;
> 
> here and elsewhere - can you use a Region for this? It would accumulate overlapping / enclosed rects in a slightly nicer way

If this gets traction, I'll file another bug to track switching to Regions when performance allows.
------- Comment #17 From 2012-12-03 18:24:25 PST -------
(In reply to comment #16)
> (In reply to comment #4)
> > (From update of attachment 177303 [details] [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=177303&action=review
> > 
> > > Source/WebCore/rendering/RenderInline.cpp:1437
> > > +    Vector<IntRect> result;
> > 
> > here and elsewhere - can you use a Region for this? It would accumulate overlapping / enclosed rects in a slightly nicer way
> 
> If this gets traction, I'll file another bug to track switching to Regions when performance allows.

(For the record, the bug tracking the N^2 issue is https://bugs.webkit.org/show_bug.cgi?id=100814)
------- Comment #18 From 2012-12-03 18:40:18 PST -------
(From update of attachment 177386 [details])
Attachment 177386 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15101793
------- Comment #19 From 2012-12-04 10:33:58 PST -------
Created an attachment (id=177507) [details]
Patch
------- Comment #20 From 2012-12-04 11:22:59 PST -------
(From update of attachment 177507 [details])
Attachment 177507 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15154039
------- Comment #21 From 2012-12-04 14:44:20 PST -------
Created an attachment (id=177569) [details]
Patch
------- Comment #22 From 2012-12-04 16:00:46 PST -------
(From update of attachment 177569 [details])
Attachment 177569 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15152139
------- Comment #23 From 2012-12-04 17:22:24 PST -------
Created an attachment (id=177615) [details]
Patch
------- Comment #24 From 2012-12-05 09:51:48 PST -------
(In reply to comment #23)
> Created an attachment (id=177615) [details] [details]
> Patch

smfr: see if this addresses your concerns.
------- Comment #25 From 2012-12-05 14:15:15 PST -------
(In reply to comment #24)
> (In reply to comment #23)
> > Created an attachment (id=177615) [details] [details] [details]
> > Patch
> 
> smfr: see if this addresses your concerns.

Ping?
------- Comment #26 From 2012-12-06 15:50:09 PST -------
(From update of attachment 177615 [details])
R=me
------- Comment #27 From 2012-12-06 15:59:17 PST -------
(From update of attachment 177615 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=177615&action=review

> Source/WebCore/rendering/RenderBox.cpp:483
> +void RenderBox::addRepaintContainerHitTestRects(RepaintContainerHitTestRects& repaintContainerRects, const RenderLayerModelObject* currentRepaintContainer, const LayoutPoint& repaintOffset) const

What does "current" add to repaintContainer?

> Source/WebCore/rendering/RenderBox.cpp:485
> +    LayoutPoint adjustedRepaintOffset = repaintOffset;

It's odd that this is called a repaintOffset when it has nothing to do with (re)painting. It should just be offsetFromContainer.

> Source/WebCore/rendering/RenderBox.cpp:493
> +    if (width() && height()) {
> +        IntRect result(pixelSnappedIntRect(adjustedRepaintOffset, size()));

I'd prefer this to be a call to something like borderBoxRect(). Just using size() doesn't indicate that you've thought about which rect makes sense for hit testing.

> Source/WebCore/rendering/RenderObject.cpp:622
> +    // Self-painting layers subtract the renderer's location. Otherwise, we need to map our location to container space.

self-painting layer means something different in RL speak.

> Source/WebCore/rendering/RenderObject.cpp:629
> +        TransformState transformState(TransformState::ApplyTransformDirection, FloatPoint());
> +        mapLocalToContainer(repaintContainer, transformState, ApplyContainerFlip | UseTransforms | SnapOffsetForTransforms);
> +        transformState.flatten();
> +        offset = LayoutPoint(transformState.lastPlanarPoint());

See comment below.

> Source/WebCore/rendering/RenderObject.h:965
> +    void repaintContainerAndOffset(const RenderLayerModelObject*&, LayoutPoint&) const;

r- for this method. This encourages incorrect usage, where this is a software-rendered transform between you and your repaint container. Just offsetting a rect by that offset will do the wrong thing there.
------- Comment #28 From 2012-12-06 17:55:27 PST -------
(From update of attachment 177615 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=177615&action=review

>> Source/WebCore/rendering/RenderBox.cpp:483
>> +void RenderBox::addRepaintContainerHitTestRects(RepaintContainerHitTestRects& repaintContainerRects, const RenderLayerModelObject* currentRepaintContainer, const LayoutPoint& repaintOffset) const
> 
> What does "current" add to repaintContainer?

The idea is that it can be updated within the method itself, so what's passed in is not necessarily the repaint container.

>> Source/WebCore/rendering/RenderBox.cpp:485
>> +    LayoutPoint adjustedRepaintOffset = repaintOffset;
> 
> It's odd that this is called a repaintOffset when it has nothing to do with (re)painting. It should just be offsetFromContainer.

No problem.

>> Source/WebCore/rendering/RenderBox.cpp:493
>> +        IntRect result(pixelSnappedIntRect(adjustedRepaintOffset, size()));
> 
> I'd prefer this to be a call to something like borderBoxRect(). Just using size() doesn't indicate that you've thought about which rect makes sense for hit testing.

Sure.

>> Source/WebCore/rendering/RenderObject.h:965
>> +    void repaintContainerAndOffset(const RenderLayerModelObject*&, LayoutPoint&) const;
> 
> r- for this method. This encourages incorrect usage, where this is a software-rendered transform between you and your repaint container. Just offsetting a rect by that offset will do the wrong thing there.

Any recommendations for the name then?
------- Comment #29 From 2012-12-07 12:35:14 PST -------
Created an attachment (id=178256) [details]
Patch
------- Comment #30 From 2012-12-07 13:35:52 PST -------
(From update of attachment 178256 [details])
Attachment 178256 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15175807
------- Comment #31 From 2012-12-07 14:01:58 PST -------
(From update of attachment 178256 [details])
Attachment 178256 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15182656

New failing tests:
inspector-protocol/debugger-terminate-dedicated-worker-while-paused.html
------- Comment #32 From 2012-12-07 14:09:00 PST -------
Created an attachment (id=178272) [details]
Patch
------- Comment #33 From 2012-12-07 14:19:17 PST -------
(From update of attachment 178272 [details])
Attachment 178272 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/15186524
------- Comment #34 From 2012-12-07 14:23:40 PST -------
(From update of attachment 178272 [details])
Attachment 178272 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/15181648
------- Comment #35 From 2012-12-07 14:40:36 PST -------
(From update of attachment 178272 [details])
Attachment 178272 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/15212265
------- Comment #36 From 2012-12-07 14:41:05 PST -------
Created an attachment (id=178281) [details]
Patch
------- Comment #37 From 2012-12-08 03:46:09 PST -------
(From update of attachment 178281 [details])
Attachment 178281 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15219101
------- Comment #38 From 2012-12-10 10:02:14 PST -------
Created an attachment (id=178580) [details]
Patch
------- Comment #39 From 2012-12-11 12:16:04 PST -------
Created an attachment (id=178845) [details]
Simplified solution

It's important to our schedule to get some flavor of this in soon, so in the interest of moving forward, this is a more simple solution that uses clippedOverflowRectForRepaint, mirroring my understanding of what iOS does.
------- Comment #40 From 2012-12-11 13:50:10 PST -------
(From update of attachment 178845 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=178845&action=review

> Source/WebCore/ChangeLog:50
> +        (WebCore):

Please remove all the redundant lines like this one. Per-function comments are useful to explain particular changes that are not covered in the summary.

> Source/WebCore/page/scrolling/ScrollingCoordinator.cpp:192
> +    rects.append(enclosingIntRect(renderer->clippedOverflowRectForRepaint(0)));
> +    if (renderer->isRenderBlock()) {
> +        const RenderBlock* block = toRenderBlock(renderer);
> +        for (RenderObject* child = block->firstChild(); child; child = child->nextSibling())
> +            accumulateRendererTouchEventTargetRects(rects, child);

This is a tree walk which is O(N^2) on tree depth (since clippedOverflowRectForRepaint(0) walks up the root for each renderer). Making a version of clippedOverflowRectForRepaint() which takes a RenderGeometryMap would alleviate that.

> Source/WebCore/page/scrolling/ScrollingCoordinator.cpp:204
> +        if (iter->key == document) {

I'd prefer iter->key be pulled out into a local variable that makes it clear what its type is.

> Source/WebCore/page/scrolling/ScrollingCoordinator.h:156
> +    void computeAbsoluteTouchEventTargetRects(const Document*, Vector<IntRect>&);

Doesn't this need to be inside a #if ENABLE(TOUCH_EVENT_TRACKING)?
------- Comment #41 From 2012-12-11 13:56:49 PST -------
(From update of attachment 178845 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=178845&action=review

>> Source/WebCore/ChangeLog:50
>> +        (WebCore):
> 
> Please remove all the redundant lines like this one. Per-function comments are useful to explain particular changes that are not covered in the summary.

Okay.

>> Source/WebCore/page/scrolling/ScrollingCoordinator.cpp:192
>> +            accumulateRendererTouchEventTargetRects(rects, child);
> 
> This is a tree walk which is O(N^2) on tree depth (since clippedOverflowRectForRepaint(0) walks up the root for each renderer). Making a version of clippedOverflowRectForRepaint() which takes a RenderGeometryMap would alleviate that.

Agreed. I'm happy to revisit this in a future patch.

>> Source/WebCore/page/scrolling/ScrollingCoordinator.cpp:204
>> +        if (iter->key == document) {
> 
> I'd prefer iter->key be pulled out into a local variable that makes it clear what its type is.

Good idea.
------- Comment #42 From 2012-12-11 14:19:25 PST -------
Created an attachment (id=178874) [details]
Patch
------- Comment #43 From 2012-12-12 10:26:33 PST -------
smfr, can I get you to take another look?
------- Comment #44 From 2012-12-12 15:36:27 PST -------
(From update of attachment 178874 [details])
Thank you for your help Simon.
------- Comment #45 From 2012-12-12 15:37:05 PST -------
(In reply to comment #44)
> (From update of attachment 178874 [details] [details])
> Thank you for your help Simon.

+1! I'll file a tracking bug for the n^2 issue.
------- Comment #46 From 2012-12-12 15:39:20 PST -------
(From update of attachment 178874 [details])
Rejecting attachment 178874 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
atching file Source/WebKit/chromium/src/WebPluginContainerImpl.cpp
patching file LayoutTests/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file LayoutTests/platform/chromium/fast/events/touch/compositor-touch-hit-rects-expected.txt
patching file LayoutTests/platform/chromium/fast/events/touch/compositor-touch-hit-rects.html

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Simon Fras..." exit_code: 1 cwd: /mnt/git/webkit-commit-queue

Full output: http://queues.webkit.org/results/15304166
------- Comment #47 From 2012-12-12 15:42:56 PST -------
(From update of attachment 178874 [details])
I'm not sure what broke there. Giving it a kick.
------- Comment #48 From 2012-12-12 15:45:06 PST -------
(From update of attachment 178874 [details])
Rejecting attachment 178874 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
atching file Source/WebKit/chromium/src/WebPluginContainerImpl.cpp
patching file LayoutTests/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file LayoutTests/platform/chromium/fast/events/touch/compositor-touch-hit-rects-expected.txt
patching file LayoutTests/platform/chromium/fast/events/touch/compositor-touch-hit-rects.html

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--force', u'--reviewer', u'Simon Fras..." exit_code: 1 cwd: /mnt/git/webkit-commit-queue

Full output: http://queues.webkit.org/results/15272948
------- Comment #49 From 2012-12-12 16:15:54 PST -------
Created an attachment (id=179143) [details]
Patch for landing
------- Comment #50 From 2012-12-12 16:19:09 PST -------
(From update of attachment 179143 [details])
Missing the tests...
------- Comment #51 From 2012-12-12 16:20:19 PST -------
Created an attachment (id=179146) [details]
Patch for landing
------- Comment #52 From 2012-12-12 16:24:01 PST -------
Attachment 179146 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/plat..." exit_code: 1
Source/WebCore/page/scrolling/ScrollingCoordinator.h:155:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Total errors found: 1 in 22 files


If any of these errors are false positives, please file a bug against check-webkit-style.
------- Comment #53 From 2012-12-12 16:45:53 PST -------
Created an attachment (id=179150) [details]
Patch for landing
------- Comment #54 From 2012-12-12 17:36:05 PST -------
(From update of attachment 179150 [details])
Rejecting attachment 179150 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1

/mnt/git/webkit-commit-queue/Source/WebCore/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://queues.webkit.org/results/15312043
------- Comment #55 From 2012-12-14 17:30:36 PST -------
Created an attachment (id=179565) [details]
Patch
------- Comment #56 From 2012-12-14 22:39:39 PST -------
(From update of attachment 179565 [details])
Attachment 179565 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/15317906

New failing tests:
http/tests/inspector/appcache/appcache-manifest-with-non-existing-file.html
http/tests/inspector/network/har-content.html
http/tests/inspector/network/network-disable-cache-xhrs.html
http/tests/inspector/extensions-headers.html
http/tests/inspector-enabled/injected-script-discard.html
http/tests/inspector/console-cross-origin-iframe-logging.html
accessibility/contenteditable-table-check-causes-crash.html
http/tests/inspector/filesystem/delete-entry.html
http/tests/inspector/extensions-useragent.html
http/tests/inspector-enabled/dynamic-scripts.html
http/tests/inspector/indexeddb/resources-panel.html
compositing/visibility/hidden-iframe.html
http/tests/inspector/network/network-cachedresources-with-same-urls.html
http/tests/inspector/console-cd-completions.html
accessibility/notification-listeners.html
http/tests/inspector/compiler-script-mapping.html
http/tests/cache/cancel-during-failure-crash.html
http/tests/inspector/filesystem/request-directory-content.html
http/tests/inspector/indexeddb/database-names.html
http/tests/inspector/web-socket-frame-error.html
accessibility/duplicate-child-nodes.html
http/tests/inspector/network/cached-resource-destroyed-moved-to-storage.html
http/tests/inspector/console-xhr-logging-async.html
http/tests/inspector/network/network-empty-xhr.html
http/tests/inspector/network/network-cyrillic-xhr.html
http/tests/inspector/resource-har-headers.html
http/tests/inspector/network-preflight-options.html
http/tests/inspector/inspect-iframe-from-different-domain.html
http/tests/inspector/filesystem/request-filesystem-root.html
http/tests/inspector-enabled/database-open.html
------- Comment #57 From 2012-12-15 16:32:24 PST -------
Crashes seem related - here's one from the unit tests:

[ RUN      ] WebPageSerializerTest.MultipleFrames
Received signal 11
    base::debug::StackTrace::StackTrace() [0x1b993ee]
    base::debug::(anonymous namespace)::StackDumpSignalHandler() [0x1b994f5]
    <unknown> [0x7f8e29ac2af0]
    WebCore::Document::didRemoveEventTargetNode() [0x88efe4]
    WebCore::DOMWindow::~DOMWindow() [0x12158dc]
    WebCore::Document::~Document() [0x89e553]
    WebCore::HTMLDocument::~HTMLDocument() [0xcdeb9d]
    WebCore::Node::~Node() [0x8d8adb]
    WebCore::HTMLSpanElement::~HTMLSpanElement() [0x1fe57d7]
    v8::internal::GlobalHandles::PostGarbageCollectionProcessing() [0xa1457b]
    v8::internal::Heap::PerformGarbageCollection() [0xa335d3]
    v8::internal::Heap::CollectGarbage() [0xa33e28]
    v8::internal::Map::UpdateCodeCache() [0xb0b6c3]
    v8::internal::StubCache::ComputeKeyedLoadOrStoreElement() [0xbacb66]
    v8::internal::KeyedIC::ComputeStub() [0xa79369]
    v8::internal::KeyedStoreIC::Store() [0xa799cd]
    v8::internal::KeyedStoreIC_Miss() [0xa79eda]
    <unknown> [0x3bc14b206362]
------- Comment #58 From 2012-12-15 19:18:21 PST -------
Definitely crashing - not every time, but here's one example stack from fast/events/touch/

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0xbbadbf17
0x031588e7 in WTF::IdentityHashTranslator<WTF::PtrHash<WebCore::Node*> >::equal<WebCore::Node*> (a=@0xbbadbf17, b=@0xbfff90f8) at HashTable.h:300
300            template<typename T> static bool equal(const T& a, const T& b) { return HashFunctions::equal(a, b); }

#0  0x031588e7 in WTF::IdentityHashTranslator<WTF::PtrHash<WebCore::Node*> >::equal<WebCore::Node*> (a=@0xbbadbf17, b=@0xbfff90f8) at HashTable.h:300
#1  0x0237537e in WTF::HashTable<WebCore::Node*, WTF::KeyValuePair<WebCore::Node*, unsigned int>, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WebCore::Node*, unsigned int> >, WTF::PtrHash<WebCore::Node*>, WTF::HashMapValueTraits<WTF::HashTraits<WebCore::Node*>, WTF::HashTraits<unsigned int> >, WTF::HashTraits<WebCore::Node*> >::lookup<WTF::IdentityHashTranslator<WTF::PtrHash<WebCore::Node*> >, WebCore::Node*> (this=0xe236760, key=@0xbfff90f8) at HashTable.h:628
#2  0x023751ec in WTF::HashTable<WebCore::Node*, WTF::KeyValuePair<WebCore::Node*, unsigned int>, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WebCore::Node*, unsigned int> >, WTF::PtrHash<WebCore::Node*>, WTF::HashMapValueTraits<WTF::HashTraits<WebCore::Node*>, WTF::HashTraits<unsigned int> >, WTF::HashTraits<WebCore::Node*> >::find<WTF::IdentityHashTranslator<WTF::PtrHash<WebCore::Node*> >, WebCore::Node*> (this=0xe236760, key=@0xbfff90f8) at HashTable.h:971
#3  0x02375151 in WTF::HashTable<WebCore::Node*, WTF::KeyValuePair<WebCore::Node*, unsigned int>, WTF::KeyValuePairKeyExtractor<WTF::KeyValuePair<WebCore::Node*, unsigned int> >, WTF::PtrHash<WebCore::Node*>, WTF::HashMapValueTraits<WTF::HashTraits<WebCore::Node*>, WTF::HashTraits<unsigned int> >, WTF::HashTraits<WebCore::Node*> >::find (this=0xe236760, key=@0xbfff90f8) at HashTable.h:401
#4  0x02374b17 in WTF::HashMap<WebCore::Node*, unsigned int, WTF::PtrHash<WebCore::Node*>, WTF::HashTraits<WebCore::Node*>, WTF::HashTraits<unsigned int> >::find (this=0xe236760, key=@0xbfff90f8) at HashMap.h:291
#5  0x03048411 in WTF::HashCountedSet<WebCore::Node*, WTF::PtrHash<WebCore::Node*>, WTF::HashTraits<WebCore::Node*> >::find (this=0xe236760, value=@0xbfff90f8) at HashCountedSet.h:132
#6  0x0302a263 in WTF::HashCountedSet<WebCore::Node*, WTF::PtrHash<WebCore::Node*>, WTF::HashTraits<WebCore::Node*> >::removeAll (this=0xe236760, value=@0xbfff90f8) at HashCountedSet.h:188
#7  0x0301afb7 in WebCore::Document::didRemoveEventTargetNode (this=0xd91d000, handler=0xd91d000) at ../../third_party/WebKit/Source/WebCore/dom/Document.cpp:5657
#8  0x02d4e73f in WebCore::DOMWindow::~DOMWindow (this=0xe23ff20) at ../../third_party/WebKit/Source/WebCore/page/DOMWindow.cpp:447
#9  0x02d4de3b in WebCore::DOMWindow::~DOMWindow (this=0xe23ff20) at ../../third_party/WebKit/Source/WebCore/page/DOMWindow.cpp:409
#10 0x02d4ddde in WebCore::DOMWindow::~DOMWindow (this=0xe23ff20) at ../../third_party/WebKit/Source/WebCore/page/DOMWindow.cpp:409
#11 0x0316bcce in WTF::RefCounted<WebCore::DOMWindow>::deref (this=0xe23ff24) at RefCounted.h:202
#12 0x0316bc4d in WTF::derefIfNotNull<WebCore::DOMWindow> (ptr=0xe23ff20) at PassRefPtr.h:53
#13 0x030ec55d in WTF::RefPtr<WebCore::DOMWindow>::~RefPtr (this=0xd91d16c) at RefPtr.h:56
#14 0x030ec50b in WTF::RefPtr<WebCore::DOMWindow>::~RefPtr (this=0xd91d16c) at RefPtr.h:56
#15 0x02fff5f2 in WebCore::Document::~Document (this=0xd91d000) at ../../third_party/WebKit/Source/WebCore/dom/Document.cpp:672
#16 0x015bf046 in WebCore::HTMLDocument::~HTMLDocument (this=0xd91d000) at ../../third_party/WebKit/Source/WebCore/html/HTMLDocument.cpp:91
#17 0x015bee6b in WebCore::HTMLDocument::~HTMLDocument (this=0xd91d000) at ../../third_party/WebKit/Source/WebCore/html/HTMLDocument.cpp:90
#18 0x015bee0e in WebCore::HTMLDocument::~HTMLDocument (this=0xd91d000) at ../../third_party/WebKit/Source/WebCore/html/HTMLDocument.cpp:90
#19 0x03144cc5 in WebCore::Document::guardDeref (this=0xd91d000) at Document.h:248
#20 0x031368bc in WebCore::Node::~Node (this=0xe236a00) at ../../third_party/WebKit/Source/WebCore/dom/Node.cpp:436
#21 0x02fd6484 in WebCore::ContainerNode::~ContainerNode (this=0xe236a00) at ../../third_party/WebKit/Source/WebCore/dom/ContainerNode.cpp:126
#22 0x030b2b18 in WebCore::Element::~Element (this=0xe236a00) at ../../third_party/WebKit/Source/WebCore/dom/Element.cpp:205
#23 0x031ebab2 in WebCore::StyledElement::~StyledElement (this=0xe236a00) at ../../third_party/WebKit/Source/WebCore/dom/StyledElement.cpp:142
#24 0x0165e84b in WebCore::HTMLElement::~HTMLElement (this=0xe236a00) at HTMLElement.h:45
#25 0x015a384b in WebCore::HTMLBodyElement::~HTMLBodyElement (this=0xe236a00) at ../../third_party/WebKit/Source/WebCore/html/HTMLBodyElement.cpp:62
#26 0x015a37fb in WebCore::HTMLBodyElement::~HTMLBodyElement (this=0xe236a00) at ../../third_party/WebKit/Source/WebCore/html/HTMLBodyElement.cpp:61
#27 0x015a379e in WebCore::HTMLBodyElement::~HTMLBodyElement (this=0xe236a00) at ../../third_party/WebKit/Source/WebCore/html/HTMLBodyElement.cpp:61
#28 0x03143358 in WebCore::Node::removedLastRef (this=0xe236a00) at ../../third_party/WebKit/Source/WebCore/dom/Node.cpp:2602
#29 0x0112d188 in WebCore::TreeShared<WebCore::Node, WebCore::ContainerNode>::deref (this=0xe236a08) at TreeShared.h:81
#30 0x03138cd1 in WebCore::Node::derefEventTarget (this=0xe236a00) at ../../third_party/WebKit/Source/WebCore/dom/Node.cpp:852
#31 0x03116a71 in WebCore::EventTarget::deref (this=0xe236a00) at EventTarget.h:106
#32 0x03116a18 in WTF::derefIfNotNull<WebCore::EventTarget> (ptr=0xe236a00) at PassRefPtr.h:53
#33 0x031169ad in WTF::RefPtr<WebCore::EventTarget>::~RefPtr (this=0xe21f1ac) at RefPtr.h:56
#34 0x0311572b in WTF::RefPtr<WebCore::EventTarget>::~RefPtr (this=0xe21f1ac) at RefPtr.h:56
#35 0x02d92576 in WebCore::Touch::~Touch (this=0xe21f190) at Touch.h:41
#36 0x02d924ab in WebCore::Touch::~Touch (this=0xe21f190) at Touch.h:41
#37 0x02d92449 in WTF::RefCounted<WebCore::Touch>::deref (this=0xe21f190) at RefCounted.h:202
#38 0x039cd31b in WebCore::V8Touch::derefObject (object=0xe21f190) at V8Touch.cpp:223
#39 0x023d0f38 in WebCore::WrapperTypeInfo::derefObject (this=0x643fa84, object=0xe21f190) at WrapperTypeInfo.h:90
#40 0x023d0817 in WebCore::DOMWrapperMap<void>::defaultWeakCallback (value={<v8::Handle<v8::Value>> = {val_ = 0xd9039f0}, <No data fields>}, context=0xe37b234) at DOMWrapperMap.h:103
#41 0x04a07e51 in v8::internal::GlobalHandles::Node::PostGarbageCollectionProcessing (this=0xd9039f0, isolate=0xf08a200, global_handles=0xe06c700) at ../../v8/src/global-handles.cc:244
#42 0x04a05622 in v8::internal::GlobalHandles::PostGarbageCollectionProcessing (this=0xe06c700, collector=v8::internal::MARK_COMPACTOR, tracer=0xbfff99e8) at ../../v8/src/global-handles.cc:634
#43 0x04a1f79f in v8::internal::Heap::PerformGarbageCollection (this=0xf08a208, collector=v8::internal::MARK_COMPACTOR, tracer=0xbfff99e8) at ../../v8/src/heap.cc:968
#44 0x04a1eec9 in v8::internal::Heap::CollectGarbage (this=0xf08a208, space=v8::internal::NEW_SPACE, collector=v8::internal::MARK_COMPACTOR, gc_reason=0x57ba825 "Runtime::PerformGC", collector_reason=0x5794fd5 "promotion limit reached") at ../../v8/src/heap.cc:651
#45 0x04c03e6b in v8::internal::Heap::CollectGarbage (this=0xf08a208, space=v8::internal::NEW_SPACE, gc_reason=0x57ba825 "Runtime::PerformGC") at heap-inl.h:436
#46 0x04d5ca81 in v8::internal::Runtime::PerformGC (result=0xbad0003) at ../../v8/src/runtime.cc:13501
#47 0x2250a45f in ?? ()
#48 0x225379f9 in ?? ()
#49 0x225377e4 in ?? ()
#50 0x2253751b in ?? ()
#51 0x22520afb in ?? ()
#52 0x2251db95 in ?? ()
#53 0x22527005 in ?? ()
#54 0x22526d11 in ?? ()
#55 0x22527841 in ?? ()
#56 0x2252729d in ?? ()
#57 0x22526d11 in ?? ()
#58 0x2250e7e1 in ?? ()
#59 0x225223b2 in ?? ()
#60 0x225134ea in ?? ()
#61 0x049b31bc in v8::internal::Invoke (is_construct=false, function={location_ = 0xd0b1474}, receiver={location_ = 0xd0b147c}, argc=1, args=0xbfff9fd0, has_pending_exception=0xbfffa07f) at ../../v8/src/execution.cc:118
#62 0x049b2ae9 in v8::internal::Execution::Call (callable={location_ = 0xd0b1474}, receiver={location_ = 0xd0b1478}, argc=1, argv=0xbfff9fd0, pending_exception=0xbfffa07f, convert_receiver=false) at ../../v8/src/execution.cc:179
#63 0x049b6003 in v8::internal::Execution::InstantiateFunction (data={location_ = 0xd0b2db0}, exc=0xbfffa07f) at ../../v8/src/execution.cc:719
#64 0x048b5347 in v8::FunctionTemplate::GetFunction (this=0xd0b2db0) at ../../v8/src/api.cc:4796
#65 0x0248da62 in WebCore::V8PerContextData::constructorForTypeSlowCase (this=0xe24c3f0, type=0x643e22c) at ../../third_party/WebKit/Source/WebCore/bindings/v8/V8PerContextData.cpp:120
#66 0x035dedcc in WebCore::V8PerContextData::constructorForType (this=0xe24c3f0, type=0x643e22c) at V8PerContextData.h:88
#67 0x0248d6b2 in WebCore::V8PerContextData::createWrapperFromCacheSlowCase (this=0xe24c3f0, type=0x643e22c) at ../../third_party/WebKit/Source/WebCore/bindings/v8/V8PerContextData.cpp:102
#68 0x0246d081 in WebCore::V8PerContextData::createWrapperFromCache (this=0xe24c3f0, type=0x643e22c) at V8PerContextData.h:80
#69 0x0246c4b1 in WebCore::V8DOMWrapper::createWrapper (creationContext={val_ = 0xbfffbd54}, type=0x643e22c, impl=0xe210520) at ../../third_party/WebKit/Source/WebCore/bindings/v8/V8DOMWrapper.cpp:112
#70 0x0379762b in WebCore::V8HTMLParagraphElement::createWrapper (impl=@0xbfffa5f8, creationContext={val_ = 0xbfffbd54}, isolate=0xf08a200) at V8HTMLParagraphElement.cpp:119
#71 0x03a8bbc6 in WebCore::wrap (impl=0xe210520, creationContext={val_ = 0xbfffbd54}, isolate=0xf08a200) at V8HTMLParagraphElement.h:60
#72 0x03a8514b in WebCore::createHTMLParagraphElementWrapper (element=0xe210520, creationContext={val_ = 0xbfffbd54}, isolate=0xf08a200) at gen/webkit/V8HTMLElementWrapperFactory.cpp:583
#73 0x03a83536 in WebCore::createV8HTMLWrapper (element=0xe210520, creationContext={val_ = 0xbfffbd54}, isolate=0xf08a200) at gen/webkit/V8HTMLElementWrapperFactory.cpp:901
#74 0x024eb8a4 in WebCore::wrap (impl=0xe210520, creationContext={val_ = 0xbfffbd54}, isolate=0xf08a200) at ../../third_party/WebKit/Source/WebCore/bindings/v8/custom/V8HTMLElementCustom.cpp:46
#75 0x024d52c4 in WebCore::wrap (impl=0xe210520, creationContext={val_ = 0xbfffbd54}, isolate=0xf08a200) at ../../third_party/WebKit/Source/WebCore/bindings/v8/custom/V8ElementCustom.cpp:48
#76 0x038eae92 in WebCore::toV8Fast<v8::Arguments, WebCore::Document> (impl=0xe210520, container=@0xbfffbd38, wrappable=0xd906c00) at V8Element.h:78
#77 0x03889394 in WebCore::DocumentV8Internal::getElementByIdCallback (args=@0xbfffbd38) at V8Document.cpp:1439
#78 0x04933986 in v8::internal::HandleApiCallHelper<false> (args={<v8::internal::Arguments> = {<v8::internal::Embedded> = {<No data fields>}, length_ = 3, arguments_ = 0xbfffbe94}, <No data fields>}, isolate=0xf08a200) at ../../v8/src/builtins.cc:1372
#79 0x0493342a in v8::internal::Builtin_Impl_HandleApiCall (args={<v8::internal::Arguments> = {<v8::internal::Embedded> = {<No data fields>}, length_ = 3, arguments_ = 0xbfffbe94}, <No data fields>}, isolate=0xf08a200) at ../../v8/src/builtins.cc:1390
#80 0x049284cc in v8::internal::Builtin_HandleApiCall (args={<v8::internal::Arguments> = {<v8::internal::Embedded> = {<No data fields>}, length_ = 3, arguments_ = 0xbfffbe94}, <No data fields>}, isolate=0xf08a200) at ../../v8/src/builtins.cc:1389
#81 0x2250a3f6 in ?? ()
#82 0x22539e35 in ?? ()
#83 0x22539ca6 in ?? ()
#84 0x2250e7e1 in ?? ()
#85 0x45c71ebf in ?? ()
#86 0x45c71c57 in ?? ()
#87 0x225223b9 in ?? ()
#88 0x225134ea in ?? ()
#89 0x049b31bc in v8::internal::Invoke (is_construct=false, function={location_ = 0xd0b144c}, receiver={location_ = 0xd0b145c}, argc=1, args=0xbfffc408, has_pending_exception=0xbfffc1bb) at ../../v8/src/execution.cc:118
#90 0x049b2ae9 in v8::internal::Execution::Call (callable={location_ = 0xd0b144c}, receiver={location_ = 0xd0b145c}, argc=1, argv=0xbfffc408, pending_exception=0xbfffc1bb, convert_receiver=false) at ../../v8/src/execution.cc:179
#91 0x048afcf9 in v8::Function::Call (this=0xd0b144c, recv={val_ = 0xd0b145c}, argc=1, argv=0xbfffc408) at ../../v8/src/api.cc:3785
#92 0x024055d1 in WebCore::ScriptController::callFunctionWithInstrumentation (context=0xd906c54, function={val_ = 0xd0b144c}, receiver={val_ = 0xd0b145c}, argc=1, args=0xbfffc408) at ../../third_party/WebKit/Source/WebCore/bindings/v8/ScriptController.cpp:233
#93 0x02405272 in WebCore::ScriptController::callFunction (this=0xf09887c, function={val_ = 0xd0b144c}, receiver={val_ = 0xd0b145c}, argc=1, args=0xbfffc408) at ../../third_party/WebKit/Source/WebCore/bindings/v8/ScriptController.cpp:186
#94 0x02479334 in WebCore::V8LazyEventListener::callListenerFunction (this=0xe20f110, context=0xd906c54, jsEvent={val_ = 0xd0b1434}, event=0xe2210b0) at ../../third_party/WebKit/Source/WebCore/bindings/v8/V8LazyEventListener.cpp:103
#95 0x0245e050 in WebCore::V8AbstractEventListener::invokeEventHandler (this=0xe20f110, context=0xd906c54, event=0xe2210b0, jsEvent={val_ = 0xd0b1434}) at ../../third_party/WebKit/Source/WebCore/bindings/v8/V8AbstractEventListener.cpp:142
#96 0x0245dcd7 in WebCore::V8AbstractEventListener::handleEvent (this=0xe20f110, context=0xd906c54, event=0xe2210b0) at ../../third_party/WebKit/Source/WebCore/bindings/v8/V8AbstractEventListener.cpp:102
#97 0x030f669e in WebCore::EventTarget::fireEventListeners (this=0xe2160f0, event=0xe2210b0, d=0xe216190, entry=@0xe232450) at ../../third_party/WebKit/Source/WebCore/dom/EventTarget.cpp:210
#98 0x030f640d in WebCore::EventTarget::fireEventListeners (this=0xe2160f0, event=0xe2210b0) at ../../third_party/WebKit/Source/WebCore/dom/EventTarget.cpp:175
#99 0x02d4cb0f in WebCore::DOMWindow::dispatchEvent (this=0xe2160f0, prpEvent=@0xbfffc9b0, prpTarget=@0xbfffc9a8) at ../../third_party/WebKit/Source/WebCore/page/DOMWindow.cpp:1675
#100 0x02d557a7 in WebCore::DOMWindow::dispatchLoadEvent (this=0xe2160f0) at ../../third_party/WebKit/Source/WebCore/page/DOMWindow.cpp:1649
#101 0x0300b23b in WebCore::Document::dispatchWindowLoadEvent (this=0xd906c00) at ../../third_party/WebKit/Source/WebCore/dom/Document.cpp:3658
#102 0x03008233 in WebCore::Document::implicitClose (this=0xd906c00) at ../../third_party/WebKit/Source/WebCore/dom/Document.cpp:2422
#103 0x02c274c2 in WebCore::FrameLoader::checkCallImplicitClose (this=0xf098460) at ../../third_party/WebKit/Source/WebCore/loader/FrameLoader.cpp:833
#104 0x02c2703e in WebCore::FrameLoader::checkCompleted (this=0xf098460) at ../../third_party/WebKit/Source/WebCore/loader/FrameLoader.cpp:776
#105 0x02c2727b in WebCore::FrameLoader::loadDone (this=0xf098460) at ../../third_party/WebKit/Source/WebCore/loader/FrameLoader.cpp:722
#106 0x02ce162c in WebCore::CachedResourceLoader::loadDone (this=0xe21bd80, resource=0xd8eee00) at ../../third_party/WebKit/Source/WebCore/loader/cache/CachedResourceLoader.cpp:721
#107 0x02c8c565 in WebCore::SubresourceLoader::releaseResources (this=0xd8fa200) at ../../third_party/WebKit/Source/WebCore/loader/SubresourceLoader.cpp:318
#108 0x02c838f3 in WebCore::ResourceLoader::didFinishLoading (this=0xd8fa200, finishTime=0) at ../../third_party/WebKit/Source/WebCore/loader/ResourceLoader.cpp:317
#109 0x02c8bffc in WebCore::SubresourceLoader::didFinishLoading (this=0xd8fa200, finishTime=0) at ../../third_party/WebKit/Source/WebCore/loader/SubresourceLoader.cpp:277
#110 0x02c8435f in WebCore::ResourceLoader::didFinishLoading (this=0xd8fa200, finishTime=0) at ../../third_party/WebKit/Source/WebCore/loader/ResourceLoader.cpp:456
#111 0x0209331a in WebCore::ResourceHandleInternal::didFinishLoading (this=0xe229fc0, finishTime=0) at ../../third_party/WebKit/Source/WebCore/platform/network/chromium/ResourceHandle.cpp:156
#112 0x0196ce2e in webkit_glue::WebURLLoaderImpl::Context::OnCompletedRequest (this=0xe220a90, error_code=0, was_ignored_by_handler=false, security_info=@0xc489d1c, completion_time=@0xc489d20) at ../../webkit/glue/weburlloader_impl.cc:673
#113 0x02f8f02d in (anonymous namespace)::RequestProxy::NotifyCompletedRequest (this=0xe221310, error_code=0, security_info=@0xc489d1c, complete_time=@0xc489d20) at ../../webkit/tools/test_shell/simple_resource_loader_bridge.cc:404
#114 0x02f8f847 in base::internal::RunnableAdapter<void ((anonymous namespace)::RequestProxy::*)(int, std::string const&, base::TimeTicks const&)>::Run (this=0xbfffd080, object=0xe221310, a1=@0xc489d18, a2=@0xc489d1c, a3=@0xc489d20) at bind_internal.h:311
#115 0x02f8f746 in base::internal::InvokeHelper<false, void, base::internal::RunnableAdapter<void ((anonymous namespace)::RequestProxy::*)(int, std::string const&, base::TimeTicks const&)>, void ()((anonymous namespace)::RequestProxy* const&, int const&, std::string const&, base::TimeTicks const&)>::MakeItSo (runnable={method_ = {ptr = 49868720, ptr = 0}}, a1=@0xc489d14, a2=@0xc489d18, a3=@0xc489d1c, a4=@0xc489d20) at bind_internal.h:960
#116 0x02f8f685 in base::internal::Invoker<4, base::internal::BindState<base::internal::RunnableAdapter<void ((anonymous namespace)::RequestProxy::*)(int, std::string const&, base::TimeTicks const&)>, void ()((anonymous namespace)::RequestProxy*, int, std::string const&, base::TimeTicks const&), void ()((anonymous namespace)::RequestProxy*, int, std::string, base::TimeTicks)>, void ()((anonymous namespace)::RequestProxy*, int, std::string const&, base::TimeTicks const&)>::Run (base=0xc489d00) at bind_internal.h:1572
#117 0x012b261b in base::Callback<void ()()>::Run (this=0xbfffd354) at callback.h:396
#118 0x012afb5b in MessageLoop::RunTask (this=0xe06a620, pending_task=@0xbfffd340) at ../../base/message_loop.cc:473
#119 0x012b0022 in MessageLoop::DeferOrRunPendingTask (this=0xe06a620, pending_task=@0xbfffd340) at ../../base/message_loop.cc:485
#120 0x012b0222 in MessageLoop::DoWork (this=0xe06a620) at ../../base/message_loop.cc:668
#121 0x01218dcb in base::MessagePumpCFRunLoopBase::RunWork (this=0xe069c90) at ../../base/message_pump_mac.mm:250
#122 0x01218582 in base::MessagePumpCFRunLoopBase::RunWorkSource (info=0xe069c90) at ../../base/message_pump_mac.mm:228
#123 0x982cc66f in __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ ()
#124 0x982cc099 in __CFRunLoopDoSources0 ()
#125 0x982f1e46 in __CFRunLoopRun ()
#126 0x982f163a in CFRunLoopRunSpecific ()
#127 0x982f14ab in CFRunLoopRunInMode ()
#128 0x9a39115a in RunCurrentEventLoopInMode ()
#129 0x9a390ec9 in ReceiveNextEventCommon ()
#130 0x9a390d44 in BlockUntilNextEventMatchingListInMode ()
#131 0x96c4da3a in _DPSNextEvent ()
#132 0x96c4d26c in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] ()
#133 0x96c436cc in -[NSApplication run] ()
#134 0x01219cfe in base::MessagePumpNSApplication::DoRun (this=0xe069c90, delegate=0xe06a620) at ../../base/message_pump_mac.mm:574
#135 0x01218b38 in base::MessagePumpCFRunLoopBase::Run (this=0xe069c90, delegate=0xe06a620) at ../../base/message_pump_mac.mm:169
#136 0x012af3a2 in MessageLoop::RunInternal (this=0xe06a620) at ../../base/message_loop.cc:430
#137 0x012af25b in MessageLoop::RunHandler (this=0xe06a620) at ../../base/message_loop.cc:403
#138 0x0130c6c8 in base::RunLoop::Run (this=0xbfffecf8) at ../../base/run_loop.cc:45
#139 0x012ae656 in MessageLoop::Run (this=0xe06a620) at ../../base/message_loop.cc:310
#140 0x046ad407 in webkit_support::RunMessageLoop () at ../../webkit/support/webkit_support.cc:553
#141 0x00062f39 in TestShell::waitTestFinished (this=0xbffff798) at ../../third_party/WebKit/Tools/DumpRenderTree/chromium/TestShellMac.mm:122
#142 0x00050949 in TestShell::runFileTest (this=0xbffff798, params=@0xbffff9f0, shouldDumpPixels=false) at ../../third_party/WebKit/Tools/DumpRenderTree/chromium/TestShell.cpp:294
#143 0x0000700f in runTest (shell=@0xbffff798, params=@0xbffff9f0, inputLine=@0xbfffef88, forceDumpPixels=false) at ../../third_party/WebKit/Tools/DumpRenderTree/chromium/DumpRenderTree.cpp:115
#144 0x00006b07 in main (argc=2, argv=0xbffffa90) at ../../third_party/WebKit/Tools/DumpRenderTree/chromium/DumpRenderTree.cpp:268
------- Comment #59 From 2012-12-15 19:20:47 PST -------
(In reply to comment #58)
> Definitely crashing - not every time, but here's one example stack from fast/events/touch/

Thanks for the bt. I'm taking a look.
------- Comment #60 From 2012-12-16 19:08:10 PST -------
Created an attachment (id=179682) [details]
Patch
------- Comment #61 From 2012-12-16 23:28:54 PST -------
(From update of attachment 179682 [details])
If I'm following the diffs correctly, this moved some cleanup logic from ~DOMWindow() to ~Document() since in ~DOMWindow() it ran as Document's members were being destroyed and the touch set was destroyed before ~DOMWindow() ran - is that correct?

Why do we add a document to its parent's touch set at all when handlers are registered inside it?  It looks like accumulateDocumentEventTargetRects() will recursively check child documents.
------- Comment #62 From 2012-12-17 01:13:43 PST -------
(In reply to comment #61)
> (From update of attachment 179682 [details] [details])
> If I'm following the diffs correctly, this moved some cleanup logic from ~DOMWindow() to ~Document() since in ~DOMWindow() it ran as Document's members were being destroyed and the touch set was destroyed before ~DOMWindow() ran - is that correct?

That is correct.

> 
> Why do we add a document to its parent's touch set at all when handlers are registered inside it?  It looks like accumulateDocumentEventTargetRects() will recursively check child documents.

We add the child Document to the parent Document's list so the main frame's Document can ultimately track the list, as that Document is tied to the Page and ScrollingCoordinator, which generates the rects.
------- Comment #63 From 2012-12-17 09:11:04 PST -------
(In reply to comment #62)
> (In reply to comment #61)
> > Why do we add a document to its parent's touch set at all when handlers are registered inside it?  It looks like accumulateDocumentEventTargetRects() will recursively check child documents.
> 
> We add the child Document to the parent Document's list so the main frame's Document can ultimately track the list, as that Document is tied to the Page and ScrollingCoordinator, which generates the rects.

We only check child documents that are in the parent's TouchEventTarget set.
------- Comment #64 From 2012-12-17 13:16:15 PST -------
(From update of attachment 179682 [details])
R=me
------- Comment #65 From 2012-12-17 13:17:51 PST -------
(From update of attachment 179682 [details])
Thanks for taking a look!
------- Comment #66 From 2012-12-17 13:43:33 PST -------
(From update of attachment 179682 [details])
Clearing flags on attachment: 179682

Committed r137939: <http://trac.webkit.org/changeset/137939>
------- Comment #67 From 2012-12-17 13:43:43 PST -------
All reviewed patches have been landed.  Closing bug.