Bug 157718 – RenderLayer::hitTestList could mutate the list of candidate layers.

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[zalan]

while looping through the list of layers.
RenderLayer::hitTestList()
...
    for (size_t i = list->size(); i > 0; --i) {
        RenderLayer* childLayer = list->at(i - 1);
        hitLayer = childLayer->hitTestLayer(rootLayer, this, request, tempResult, hitTestRect, hitTestLocation, false, transformState, zOffsetForDescendants);
    !!! list items could be invalid at this point.
   }

[zalan]

rdar://problem/22556046

[zalan]

Created attachment 278952 [details]
WIP patch

Not sure whether we should return nullptr or resultLayer. and whether the mutate check should happen at the end of the loop so that we could still return the current layer if it is the hittest candidate even if while hit testing it, the list got mutated.

[WebKit Commit Bot]

Attachment 278952 [details] did not pass style-queue:


ERROR: Source/WebCore/rendering/RenderLayer.cpp:5390:  A case label should not be indented, but line up with its switch statement.  [whitespace/indent] [4]
ERROR: Source/WebCore/rendering/RenderLayer.cpp:5389:  Missing space before ( in switch(  [whitespace/parens] [5]
ERROR: Source/WebCore/rendering/RenderLayer.h:823:  The parameter name "request" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/rendering/RenderLayer.h:823:  The parameter name "result" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 4 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[zalan]

Created attachment 278953 [details]
WIP patch

[Build Bot]

Comment on attachment 278953 [details]
WIP patch

Attachment 278953 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/1323073

Number of test failures exceeded the failure limit.

[Build Bot]

Created attachment 278954 [details]
Archive of layout-test-results from ews113 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[zalan]

Created attachment 278965 [details]
Patch

[zalan]

Created attachment 279039 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 279039 [details]
Patch

Clearing flags on attachment: 279039

Committed r200971: <http://trac.webkit.org/changeset/200971>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

Comment on attachment 279039 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=279039&action=review

> Source/WebCore/page/EventHandler.cpp:1141
> +    // We should always start hittesting a clean tree.
> +    renderView->document().updateLayoutIgnorePendingStylesheets();

Seems safer to do this earlier, before calling contentRenderer(). Probably even before making the HitTestResult.

Also, “hit testing” is two words.