Fix some unexpected cases of image maps.

Fix for buganizer issue 1800613: Image map not accessible.  We were attempting to cache associtations of <area> elements to their RenderImages so we would not have to search for them.  However, we were storing them in CacheBuilder.  When we search for them, we search in the <area> element's frame, even though when we built the cache, we stored it in the main frame.  Another issue arose on a page where <area> elements show up before their RenderImages.  In this case, we were storing an empty rectangle for the bounds, and caching the associations afterwards, when it was too late.  In order to clean up/simplify things, while fixing both bugs, I have changed validNode into a static function which takes the starting Frame as a parameter.  Any time we need to find the rectangle of an <area> element, we have to traverse the DOM to find its associated RenderImage.  This may be slower, but it will always get the correct answer.  In the future, we may need to investigate a universal location for caching the associations for better performance.

3 files changed, 44 insertions, 105 deletions

diff --git a/WebKit/android/jni/WebViewCore.cpp b/WebKit/android/jni/WebViewCore.cpp
index 596ec3f..ba07c52 100644
--- a/WebKit/android/jni/WebViewCore.cpp
+++ b/WebKit/android/jni/WebViewCore.cpp
@@ -974,8 +974,7 @@ void WebViewCore::dumpNavTree()
 
 WebCore::String WebViewCore::retrieveHref(WebCore::Frame* frame, WebCore::Node* node)
 {
-    CacheBuilder& builder = FrameLoaderClientAndroid::get(m_mainFrame)->getCacheBuilder();
-    if (!builder.validNode(frame, node))
+    if (!CacheBuilder::validNode(m_mainFrame, frame, node))
         return WebCore::String();
     if (!node->hasTagName(WebCore::HTMLNames::aTag))
         return WebCore::String();
@@ -1221,7 +1220,8 @@ bool WebViewCore::commonKitFocus(int generation, int buildGeneration,
     }
     // if the nav cache has been rebuilt since this focus request was generated,
     // send a request back to the UI side to recompute the kit-side focus
-    if (m_buildGeneration > buildGeneration || (node && !FrameLoaderClientAndroid::get(m_mainFrame)->getCacheBuilder().validNode(frame, node))) {
+    if (m_buildGeneration > buildGeneration
+            || (node && !CacheBuilder::validNode(m_mainFrame, frame, node))) {
         DBG_NAV_LOGD("m_buildGeneration=%d > buildGeneration=%d",
            m_buildGeneration, buildGeneration);
         gRecomputeFocusMutex.lock();
@@ -1252,7 +1252,7 @@ bool WebViewCore::finalKitFocus(WebCore::Frame* frame, WebCore::Node* node,
         donotChangeDOMFocus ? "true" : "false");
     CacheBuilder& builder = FrameLoaderClientAndroid::
         get(m_mainFrame)->getCacheBuilder();
-    if (!frame || builder.validNode(frame, NULL) == false)
+    if (!frame || CacheBuilder::validNode(m_mainFrame, frame, NULL) == false)
         frame = m_mainFrame;
     WebCore::Node* oldFocusNode = builder.currentFocus();
     // mouse event expects the position in the window coordinate
@@ -1263,7 +1263,7 @@ bool WebViewCore::finalKitFocus(WebCore::Frame* frame, WebCore::Node* node,
         WebCore::NoButton, WebCore::MouseEventMoved, 1, false, false, false,
         false, WTF::currentTime());
     frame->eventHandler()->handleMouseMoveEvent(mouseEvent);
-    bool valid = builder.validNode(frame, node);
+    bool valid = CacheBuilder::validNode(m_mainFrame, frame, node);
     if (!donotChangeDOMFocus) {
         WebCore::Document* oldDoc = oldFocusNode ? oldFocusNode->document() : 0;
         if (!node) {
@@ -1605,8 +1605,7 @@ void WebViewCore::setFocusControllerActive(bool active)
 
 void WebViewCore::saveDocumentState(WebCore::Frame* frame)
 {
-    if (!FrameLoaderClientAndroid::get(m_mainFrame)->getCacheBuilder()
-            .validNode(frame, 0))
+    if (!CacheBuilder::validNode(m_mainFrame, frame, 0))
         frame = m_mainFrame;
     WebCore::HistoryItem *item = frame->loader()->currentHistoryItem();
 
@@ -1649,9 +1648,8 @@ public:
         }
         // If the select element no longer exists, due to a page change, etc,
         // silently return.
-        if (!m_select ||
-                !FrameLoaderClientAndroid::get(m_viewImpl->m_mainFrame)
-                        ->getCacheBuilder().validNode(m_frame, m_select))
+        if (!m_select || !CacheBuilder::validNode(m_viewImpl->m_mainFrame,
+                m_frame, m_select))
             return;
         int optionIndex = m_select->listToOptionIndex(index);
         m_select->setSelectedIndex(optionIndex, true, false);
@@ -1665,9 +1663,8 @@ public:
     {
         // If the select element no longer exists, due to a page change, etc,
         // silently return.
-        if (!m_select ||
-                !FrameLoaderClientAndroid::get(m_viewImpl->m_mainFrame)
-                        ->getCacheBuilder().validNode(m_frame, m_select))
+        if (!m_select || !CacheBuilder::validNode(m_viewImpl->m_mainFrame,
+                m_frame, m_select))
             return;
 
         // If count is 1 or 0, use replyInt.
@@ -1859,7 +1856,7 @@ void WebViewCore::touchUp(int touchGeneration, int buildGeneration,
     // so just leave the function now.
     if (!isClick)
         return;
-    if (frame && FrameLoaderClientAndroid::get(m_mainFrame)->getCacheBuilder().validNode(frame, 0)) {
+    if (frame && CacheBuilder::validNode(m_mainFrame, frame, 0)) {
         frame->loader()->resetMultipleFormSubmissionProtection();
     }
     EditorClientAndroid* client = static_cast<EditorClientAndroid*>(m_mainFrame->editor()->client());
@@ -1872,8 +1869,8 @@ void WebViewCore::touchUp(int touchGeneration, int buildGeneration,
 
 bool WebViewCore::handleMouseClick(WebCore::Frame* framePtr, WebCore::Node* nodePtr)
 {
-    bool valid = framePtr == NULL || FrameLoaderClientAndroid::get(
-            m_mainFrame)->getCacheBuilder().validNode(framePtr, nodePtr);
+    bool valid = framePtr == NULL
+            || CacheBuilder::validNode(m_mainFrame, framePtr, nodePtr);
     WebFrame* webFrame = WebFrame::getWebFrame(m_mainFrame);
     if (valid && nodePtr) {
     // Need to special case area tags because an image map could have an area element in the middle
diff --git a/WebKit/android/nav/CacheBuilder.cpp b/WebKit/android/nav/CacheBuilder.cpp
index bf10a4d..02f4605 100644
--- a/WebKit/android/nav/CacheBuilder.cpp
+++ b/WebKit/android/nav/CacheBuilder.cpp
@@ -390,7 +390,7 @@ void CacheBuilder::Debug::groups() {
                 properties.truncate(properties.length() - 3);
             IntRect rect = node->getRect();
             if (node->hasTagName(HTMLNames::areaTag))
-                rect = Builder(frame)->getAreaRect(static_cast<HTMLAreaElement*>(node));
+                rect = getAreaRect(static_cast<HTMLAreaElement*>(node));
             char buffer[DEBUG_BUFFER_SIZE];
             memset(buffer, 0, sizeof(buffer));
             mBuffer = buffer;
@@ -822,7 +822,6 @@ void CacheBuilder::buildCache(CachedRoot* root)
 {
     Frame* frame = FrameAnd(this);
     mLastKnownFocus = NULL;
-    m_areaBoundsMap.clear();
     BuildFrame(frame, frame, root, (CachedFrame*) root);
     root->finishInit(); // set up frame parent pointers, child pointers
     setData((CachedFrame*) root);
@@ -980,20 +979,6 @@ void CacheBuilder::BuildFrame(Frame* root, Frame* frame,
             RenderStyle* style = nodeRenderer->style();
             if (style->visibility() == HIDDEN)
                 continue;
-            if (nodeRenderer->isImage()) { // set all the area elements to have a link to their images
-                RenderImage* image = static_cast<RenderImage*>(nodeRenderer);
-                HTMLMapElement* map = image->imageMap();
-                if (map) {
-                    Node* node;
-                    for (node = map->firstChild(); node; 
-                            node = node->traverseNextNode(map)) {
-                        if (!node->hasTagName(HTMLNames::areaTag))
-                            continue;
-                        HTMLAreaElement* area = static_cast<HTMLAreaElement*>(node);
-                        m_areaBoundsMap.set(area, image);
-                    }
-                }
-            }
             isTransparent = style->hasBackground() == false;
 #ifdef ANDROID_CSS_TAP_HIGHLIGHT_COLOR
             hasFocusRing = style->tapHighlightColor().alpha() > 0;
@@ -2347,14 +2332,28 @@ void CacheBuilder::FindResetNumber(FindState* state)
     state->mStorePtr = state->mStore;
 }
 
-IntRect CacheBuilder::getAreaRect(const HTMLAreaElement* area) const
+IntRect CacheBuilder::getAreaRect(const HTMLAreaElement* area)
 {
-    RenderImage* map = m_areaBoundsMap.get(area);
-    if (!map)
-        return IntRect();
-    if (area->isDefault())
-        return map->absoluteBoundingBoxRect();
-    return area->getRect(map);
+    Node* node = area->document();
+    while ((node = node->traverseNextNode()) != NULL) {
+        RenderObject* renderer = node->renderer();
+        if (renderer && renderer->isRenderImage()) {
+            RenderImage* image = static_cast<RenderImage*>(renderer);
+            HTMLMapElement* map = image->imageMap();
+            if (map) {
+                Node* n;
+                for (n = map->firstChild(); n;
+                        n = n->traverseNextNode(map)) {
+                    if (n == area) {
+                        if (area->isDefault())
+                            return image->absoluteBoundingBoxRect();
+                        return area->getRect(image);
+                    }
+                }
+            }
+        }
+    }
+    return IntRect();
 }
 
 void CacheBuilder::GetGlobalOffset(Node* node, int* x, int * y) 
@@ -2432,59 +2431,6 @@ bool CacheBuilder::IsDomainChar(UChar ch)
     return (body[ch >> 5] & 1 << (ch & 0x1f)) != 0;
 }
 
-// does not find text to keep it fast
-// (this assume text nodes are more rarely moved than other nodes)
-Node* CacheBuilder::findByCenter(int x, int y) const
-{
-    DBG_NAV_LOGD("x=%d y=%d\n", x, y);
-    Frame* frame = FrameAnd(this);
-    Node* node = frame->document();
-    ASSERT(node != NULL);
-    int globalOffsetX, globalOffsetY;
-    GetGlobalOffset(frame, &globalOffsetX, &globalOffsetY);
-    while ((node = node->traverseNextNode()) != NULL) {
-        Frame* child = HasFrame(node);
-        if (child != NULL) {
-            if (child->document() == NULL)
-                continue;
-            CacheBuilder* cacheBuilder = Builder(child);
-     //       if (cacheBuilder->mViewBounds.isEmpty())
-     //           continue;
-            Node* result = cacheBuilder->findByCenter(x, y);
-            if (result != NULL)
-                return result;
-        }
-        if (node->isTextNode())
-            continue;
-        IntRect bounds;
-        if (node->hasTagName(HTMLNames::areaTag)) {
-            HTMLAreaElement* area = static_cast<HTMLAreaElement*>(node);
-            bounds = getAreaRect(area);
-            bounds.move(globalOffsetX, globalOffsetY);
-        } else
-            bounds = node->getRect();
-        if (bounds.isEmpty())
-            continue;
-        bounds.move(globalOffsetX, globalOffsetY);
-        if (x != bounds.x() + (bounds.width() >> 1))
-            continue;
-        if (y != bounds.y() + (bounds.height() >> 1))
-            continue;
-        if (node->isKeyboardFocusable(NULL))
-            return node;
-        if (node->isMouseFocusable())
-            return node;
-        if (node->isFocusable())
-            return node;
-        if (AnyIsClick(node))
-            continue;
-        if (HasTriggerEvent(node) == false)
-            continue;
-        return node;
-    }
-    return NULL;
-}
-
 bool CacheBuilder::isFocusableText(NodeWalk* walk, bool more, Node* node, 
     CachedNodeType* type, String* exported) const
 {
@@ -2760,13 +2706,13 @@ bool CacheBuilder::setData(CachedFrame* cachedFrame)
     return true;
 }
 
-bool CacheBuilder::validNode(void* matchFrame, void* matchNode) const
+bool CacheBuilder::validNode(Frame* startFrame, void* matchFrame,
+        void* matchNode)
 {
-    Frame* frame = FrameAnd(this);
-    if (matchFrame == frame) {
+    if (matchFrame == startFrame) {
         if (matchNode == NULL)
             return true;
-        Node* node = frame->document();
+        Node* node = startFrame->document();
         while (node != NULL) {
             if (node == matchNode) {
                 const IntRect& rect = node->hasTagName(HTMLNames::areaTag) ? 
@@ -2782,15 +2728,15 @@ bool CacheBuilder::validNode(void* matchFrame, void* matchNode) const
         DBG_NAV_LOGD("frame=%p valid node=%p invalid\n", matchFrame, matchNode);
         return false;
     }
-    Frame* child = frame->tree()->firstChild();
+    Frame* child = startFrame->tree()->firstChild();
     while (child) {
-        bool result = Builder(child)->validNode(matchFrame, matchNode);
+        bool result = validNode(child, matchFrame, matchNode);
         if (result)
             return result;
         child = child->tree()->nextSibling();
     }
 #if DEBUG_NAV_UI
-    if (frame->tree()->parent() == NULL)
+    if (startFrame->tree()->parent() == NULL)
         DBG_NAV_LOGD("frame=%p node=%p false\n", matchFrame, matchNode);
 #endif
     return false;
diff --git a/WebKit/android/nav/CacheBuilder.h b/WebKit/android/nav/CacheBuilder.h
index 8152fa2..667f149 100644
--- a/WebKit/android/nav/CacheBuilder.h
+++ b/WebKit/android/nav/CacheBuilder.h
@@ -31,7 +31,6 @@
 #include "IntRect.h"
 #include "PlatformString.h"
 #include "TextDirection.h"
-#include "wtf/HashMap.h"
 #include "wtf/Vector.h"
 
 #define NAVIGATION_MAX_PHONE_LENGTH 14
@@ -48,7 +47,6 @@ class InlineTextBox;
 class Node;
 class PlatformGraphicsContext;
 class RenderFlow;
-class RenderImage;
 class RenderObject;
 class RenderLayer;
 class Text;
@@ -91,12 +89,11 @@ public:
     void disallowPhoneDetection() { mAllowableTypes = (CachedNodeType) (
         mAllowableTypes & ~PHONE_CACHEDNODETYPE); }
     static FoundState FindAddress(const UChar* , unsigned length, int* start, int* end);
-    Node* findByCenter(int x, int y) const;
     static void GetGlobalOffset(Frame* , int* x, int * y);
     static void GetGlobalOffset(Node* , int* x, int * y);
     bool outOfDate();
     void setLastFocus(Node* );
-    bool validNode(void* framePtr, void* nodePtr) const;
+    static bool validNode(Frame* startFrame, void* framePtr, void* nodePtr);
 private:
     enum AddressProgress {
         NO_ADDRESS,
@@ -215,7 +212,7 @@ private:
     static Frame* FrameAnd(CacheBuilder* focusNav);
     static Frame* FrameAnd(const CacheBuilder* focusNav);
     static CacheBuilder* Builder(Frame* );
-    IntRect getAreaRect(const HTMLAreaElement* area) const;
+    static IntRect getAreaRect(const HTMLAreaElement* area);
     static Frame* HasFrame(Node* );
     static bool HasOverOrOut(Node* );
     static bool HasTriggerEvent(Node* );
@@ -231,7 +228,6 @@ private:
     Node* mLastKnownFocus;
     IntRect mLastKnownFocusBounds;
     CachedNodeType mAllowableTypes;
-    WTF::HashMap<const HTMLAreaElement* , RenderImage* > m_areaBoundsMap;
 #if DUMP_NAV_CACHE
 public:
     class Debug {