| Summary: | RenderLayer::hitTestList could mutate the list of candidate layers. | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | zalan <zalan> | ||||||||||||
| Component: | Layout and Rendering | Assignee: | zalan <zalan> | ||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||
| Severity: | Normal | CC: | commit-queue, esprehn+autocc, glenn, kondapallykalyan, simon.fraser, webkit-bug-importer, WebkitBugTracker | ||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||
| Hardware: | Unspecified | ||||||||||||||
| OS: | Unspecified | ||||||||||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=163877 | ||||||||||||||
| Attachments: |
|
||||||||||||||
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.
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.
Created attachment 278953 [details]
WIP patch
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. 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
Created attachment 278965 [details]
Patch
Created attachment 279039 [details]
Patch
Comment on attachment 279039 [details] Patch Clearing flags on attachment: 279039 Committed r200971: <http://trac.webkit.org/changeset/200971> All reviewed patches have been landed. Closing bug. 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. |
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. }