From 08c1068d9fd18289b9812c3d1fbcfccb8dfa3e60 Mon Sep 17 00:00:00 2001 From: Steve Block Date: Thu, 12 Aug 2010 12:11:09 +0100 Subject: [PATCH] Use upstream version of ANDROID_HITTEST_WITHSIZE This change removes the last occurrences of ANDROID_HITTEST_WITHSIZE and updates WebKit to use the upstream version of this logic. This follows on from https://android-git.corp.google.com/g/#change,60166 which merged the upstream logic into the Android tree but maintained the existing behaviour for Android, guarded with ANDROID_HITTEST_WITHSIZE. This change introduces the following functional changes ... - HitTestResult::addNodeToRectBasedTestResult() The Android version proceeded with the containment test if the node is null. We now early out and continue the hit test. We also record the node's shadowAncestorNode, rather than the node itself. - RenderLayer::hitTestList() The android version always updates the result, but only updates the result layer if the result is not a rect based result. We now always update result layer, but only update the result if the result is not a rect based result. - RenderSVGRoot::nodeAtPoint() The Android version returned true only if it's not a rect based hit test. Now we return true always. - HitTestResult::m_isRectBased In the Android version, a padding must have positive width and height. Now we allow one of the two to be zero, provided the other is positive All other changes are non-functional and should probably have been done as part of https://android-git.corp.google.com/g/#change,60166 Note that the change to RenderBlock::nodeAtPoint() is a non-functional change because we don't pass a rect to addNodeToRectBasedTestResult(), so the default zero rect is used and the hit test always fails. Change-Id: I4163fddad59ad6485df40cd48794aa262f76ced7 --- WebCore/config.h | 4 ---- WebCore/rendering/HitTestResult.cpp | 17 ----------------- WebCore/rendering/RenderBlock.cpp | 9 --------- WebCore/rendering/RenderBox.cpp | 5 ++--- WebCore/rendering/RenderLayer.cpp | 6 ------ WebCore/rendering/RenderLineBoxList.cpp | 3 --- WebCore/rendering/RenderSVGRoot.cpp | 7 ------- WebKit/android/jni/WebViewCore.cpp | 22 ++++++++++------------ 8 files changed, 12 insertions(+), 61 deletions(-) diff --git a/WebCore/config.h b/WebCore/config.h index 057542268..188f21219 100644 --- a/WebCore/config.h +++ b/WebCore/config.h @@ -200,9 +200,6 @@ // Enable prefetching when specified via the rel element of elements. #define ENABLE_LINK_PREFETCH 1 -// Enable hit test with point plus a size -#define ANDROID_HITTEST_WITHSIZE - // Enable scrollable divs in separate layers. This might be upstreamed to // webkit.org but for now, it is just an Android feature. #define ENABLE_ANDROID_OVERFLOW_SCROLL 1 @@ -318,4 +315,3 @@ typedef float CGFloat; #if PLATFORM(WIN) && PLATFORM(CG) #define WTF_USE_SAFARI_THEME 1 #endif - diff --git a/WebCore/rendering/HitTestResult.cpp b/WebCore/rendering/HitTestResult.cpp index 8a986d65e..dd96e0ea1 100644 --- a/WebCore/rendering/HitTestResult.cpp +++ b/WebCore/rendering/HitTestResult.cpp @@ -57,13 +57,9 @@ HitTestResult::HitTestResult(const IntPoint& centerPoint, const IntSize& padding : m_point(centerPoint) , m_isOverWidget(false) { -#ifdef ANDROID_HITTEST_WITHSIZE - m_isRectBased = !padding.isEmpty(); -#else // If a zero padding is passed in or either width or height is negative, then it // is not a valid padding and hence not a rect based hit test. m_isRectBased = !(padding.isZero() || (padding.width() < 0 || padding.height() < 0)); -#endif m_padding = m_isRectBased ? padding : IntSize(); } @@ -76,16 +72,12 @@ HitTestResult::HitTestResult(const HitTestResult& other) , m_scrollbar(other.scrollbar()) , m_isOverWidget(other.isOverWidget()) { -#ifndef ANDROID_HITTEST_WITHSIZE // Only copy the padding and ListHashSet in case of rect hit test. // Copying the later is rather expensive. if ((m_isRectBased = other.isRectBasedTest())) { -#endif m_padding = other.padding(); m_rectBasedTestResult = other.rectBasedTestResult(); -#ifndef ANDROID_HITTEST_WITHSIZE } -#endif } HitTestResult::~HitTestResult() @@ -101,16 +93,12 @@ HitTestResult& HitTestResult::operator=(const HitTestResult& other) m_innerURLElement = other.URLElement(); m_scrollbar = other.scrollbar(); m_isOverWidget = other.isOverWidget(); -#ifndef ANDROID_HITTEST_WITHSIZE // Only copy the padding and ListHashSet in case of rect hit test. // Copying the later is rather expensive. if ((m_isRectBased = other.isRectBasedTest())) { -#endif m_padding = other.padding(); m_rectBasedTestResult = other.rectBasedTestResult(); -#ifndef ANDROID_HITTEST_WITHSIZE } -#endif return *this; } @@ -404,17 +392,12 @@ bool HitTestResult::addNodeToRectBasedTestResult(Node* node, int x, int y, const if (!isRectBasedTest()) return false; -#ifdef ANDROID_HITTEST_WITHSIZE - if (node) - m_rectBasedTestResult.add(node); -#else // If node is null, return true so the hit test can continue. if (!node) return true; node = node->shadowAncestorNode(); m_rectBasedTestResult.add(node); -#endif return !rect.contains(rectFromPoint(x, y)); } diff --git a/WebCore/rendering/RenderBlock.cpp b/WebCore/rendering/RenderBlock.cpp index 574d38954..02b00795d 100644 --- a/WebCore/rendering/RenderBlock.cpp +++ b/WebCore/rendering/RenderBlock.cpp @@ -3774,17 +3774,8 @@ bool RenderBlock::nodeAtPoint(const HitTestRequest& request, HitTestResult& resu if ((hitTestAction == HitTestBlockBackground || hitTestAction == HitTestChildBlockBackground) && isPointInOverflowControl(result, _x, _y, tx, ty)) { updateHitTestResult(result, IntPoint(_x - tx, _y - ty)); // FIXME: isPointInOverflowControl() doesn't handle rect-based tests yet. -#ifdef ANDROID_HITTEST_WITHSIZE - // FIXME: This looks wrong - the return value does not depend on the hit test result. - result.addNodeToRectBasedTestResult(node(), _x, _y); - if (result.isRectBasedTest()) - ASSERT(node() || isAnonymous()); - else - return true; -#else if (!result.addNodeToRectBasedTestResult(node(), _x, _y)) return true; -#endif } // If we have clipping, then we can't have any spillout. diff --git a/WebCore/rendering/RenderBox.cpp b/WebCore/rendering/RenderBox.cpp index 8a3ea8e26..d61408149 100644 --- a/WebCore/rendering/RenderBox.cpp +++ b/WebCore/rendering/RenderBox.cpp @@ -35,9 +35,6 @@ #include "htmlediting.h" #include "HTMLElement.h" #include "HTMLNames.h" -#ifdef ANDROID_HITTEST_WITHSIZE -#include "HitTestResult.h" -#endif #include "ImageBuffer.h" #include "FloatQuad.h" #include "Frame.h" @@ -563,6 +560,8 @@ bool RenderBox::nodeAtPoint(const HitTestRequest& request, HitTestResult& result } } + // Check our bounds next. For this purpose always assume that we can only be hit in the + // foreground phase (which is true for replaced elements like images). IntRect boundsRect = IntRect(tx, ty, width(), height()); if (visibleToHitTesting() && action == HitTestForeground && boundsRect.intersects(result.rectFromPoint(xPos, yPos))) { updateHitTestResult(result, IntPoint(xPos - tx, yPos - ty)); diff --git a/WebCore/rendering/RenderLayer.cpp b/WebCore/rendering/RenderLayer.cpp index 7c63af9ac..df80a7cb7 100644 --- a/WebCore/rendering/RenderLayer.cpp +++ b/WebCore/rendering/RenderLayer.cpp @@ -2997,15 +2997,9 @@ RenderLayer* RenderLayer::hitTestList(Vector* list, RenderLayer* r result.append(tempResult); if (isHitCandidate(hitLayer, depthSortDescendants, zOffset, unflattenedTransformState)) { -#ifdef ANDROID_HITTEST_WITHSIZE - if (!result.isRectBasedTest()) - resultLayer = hitLayer; - result = tempResult; -#else resultLayer = hitLayer; if (!result.isRectBasedTest()) result = tempResult; -#endif if (!depthSortDescendants) break; } diff --git a/WebCore/rendering/RenderLineBoxList.cpp b/WebCore/rendering/RenderLineBoxList.cpp index d8b4e754b..4d0dcd6b0 100644 --- a/WebCore/rendering/RenderLineBoxList.cpp +++ b/WebCore/rendering/RenderLineBoxList.cpp @@ -31,9 +31,6 @@ #include "HitTestResult.h" #include "InlineTextBox.h" -#ifdef ANDROID_HITTEST_WITHSIZE -#include "HitTestResult.h" -#endif #include "RenderArena.h" #include "RenderInline.h" #include "RenderView.h" diff --git a/WebCore/rendering/RenderSVGRoot.cpp b/WebCore/rendering/RenderSVGRoot.cpp index 75e2bd278..b66a870ae 100644 --- a/WebCore/rendering/RenderSVGRoot.cpp +++ b/WebCore/rendering/RenderSVGRoot.cpp @@ -334,14 +334,7 @@ bool RenderSVGRoot::nodeAtPoint(const HitTestRequest& request, HitTestResult& re updateHitTestResult(result, pointInBorderBox); // FIXME: nodeAtFloatPoint() doesn't handle rect-based hit tests yet. result.addNodeToRectBasedTestResult(child->node(), _x, _y); -#ifdef ANDROID_HITTEST_WITHSIZE - if (result.isRectBasedTest()) - ASSERT(node() || isAnonymous()); - else - return true; -#else return true; -#endif } } diff --git a/WebKit/android/jni/WebViewCore.cpp b/WebKit/android/jni/WebViewCore.cpp index e81e335b2..42aa7181f 100644 --- a/WebKit/android/jni/WebViewCore.cpp +++ b/WebKit/android/jni/WebViewCore.cpp @@ -1403,7 +1403,6 @@ Vector WebViewCore::getTouchHighlightRects(int x, int y, int slop) { Vector rects; m_mousePos = IntPoint(x - m_scrollOffsetX, y - m_scrollOffsetY); -#ifdef ANDROID_HITTEST_WITHSIZE HitTestResult hitTestResult = m_mainFrame->eventHandler()->hitTestResultAtPoint(IntPoint(x, y), false, false, DontHitTestScrollbars, HitTestRequest::Active | HitTestRequest::ReadOnly, IntSize(slop, slop)); if (!hitTestResult.innerNode() || !hitTestResult.innerNode()->inDocument()) { @@ -1437,7 +1436,7 @@ Vector WebViewCore::getTouchHighlightRects(int x, int y, int slop) found = true; break; } - // the nodes in the rawNodeList() are ordered based on z-index during hit testing. + // the nodes in the rectBasedTestResult() are ordered based on z-index during hit testing. // so do not search for the eventNode across explicit z-index border. // TODO: this is a hard one to call. z-index is quite complicated as its value only // matters when you compare two RenderLayer in the same hierarchy level. e.g. in @@ -1618,7 +1617,6 @@ Vector WebViewCore::getTouchHighlightRects(int x, int y, int slop) m_scrollOffsetX, m_scrollOffsetY); } } -#endif return rects; } @@ -2340,15 +2338,15 @@ void WebViewCore::touchUp(int touchGeneration, frame = 0; DBG_NAV_LOGD("touch up on (%d, %d), scrollOffset is (%d, %d), node:%p, frame:%p", m_mousePos.x() + m_scrollOffsetX, m_mousePos.y() + m_scrollOffsetY, m_scrollOffsetX, m_scrollOffsetY, node, frame); } else { - if (m_touchGeneration > touchGeneration) { - DBG_NAV_LOGD("m_touchGeneration=%d > touchGeneration=%d" - " x=%d y=%d", m_touchGeneration, touchGeneration, x, y); - return; // short circuit if a newer touch has been generated - } - // This moves m_mousePos to the correct place, and handleMouseClick uses - // m_mousePos to determine where the click happens. - moveMouse(frame, x, y); - m_lastGeneration = touchGeneration; + if (m_touchGeneration > touchGeneration) { + DBG_NAV_LOGD("m_touchGeneration=%d > touchGeneration=%d" + " x=%d y=%d", m_touchGeneration, touchGeneration, x, y); + return; // short circuit if a newer touch has been generated + } + // This moves m_mousePos to the correct place, and handleMouseClick uses + // m_mousePos to determine where the click happens. + moveMouse(frame, x, y); + m_lastGeneration = touchGeneration; } if (frame && CacheBuilder::validNode(m_mainFrame, frame, 0)) { frame->loader()->resetMultipleFormSubmissionProtection(); -- 2.11.0