nextTextField may walk off the end or try invalid frames

Rewrote nextTextField() to check range and frame, and
to more resemble other node walkers.

Caller no longer passes uninitialized frame in focused
case, and looks at parent frames after the target node.

Change-Id: I7ea9dffb75d28bdd9d71d83921058feca6baf928
http://b/2607250

5 files changed, 44 insertions, 34 deletions

diff --git a/WebKit/android/nav/CachedFrame.cpp b/WebKit/android/nav/CachedFrame.cpp
index 21a4115..ce5600b 100644
--- a/WebKit/android/nav/CachedFrame.cpp
+++ b/WebKit/android/nav/CachedFrame.cpp
@@ -928,28 +928,27 @@ int CachedFrame::maxWorkingVertical() const
 }
 
 const CachedNode* CachedFrame::nextTextField(const CachedNode* start,
-        const CachedFrame** framePtr) const
+        const CachedFrame** framePtr, bool* startFound) const
 {
-    CachedNode* test;
-    if (start) {
-        test = const_cast<CachedNode*>(start);
-        test++;
-    } else {
-        test = const_cast<CachedNode*>(mCachedNodes.begin());
-    }
-    while (test != mCachedNodes.end()) {
-        CachedFrame* frame = const_cast<CachedFrame*>(hasFrame(test));
+    const CachedNode* test = mCachedNodes.begin();
+    while ((test = test->traverseNextNode())) {
+        const CachedFrame* frame = hasFrame(test);
         if (frame) {
+            if (!frame->validDocument())
+                continue;
             const CachedNode* node
-                    = frame->nextTextField(0, framePtr);
+                    = frame->nextTextField(start, framePtr, startFound);
             if (node)
                 return node;
         } else if (test->isTextInput()) {
-            if (framePtr)
-                *framePtr = this;
-            return test;
+            if (test == start)
+                *startFound = true;
+            else if (*startFound) {
+                if (framePtr)
+                    *framePtr = this;
+                return test;
+            }
         }
-        test++;
     }
     return 0;
 }
diff --git a/WebKit/android/nav/CachedFrame.h b/WebKit/android/nav/CachedFrame.h
index ed76583..9334707 100644
--- a/WebKit/android/nav/CachedFrame.h
+++ b/WebKit/android/nav/CachedFrame.h
@@ -120,16 +120,6 @@ public:
 #endif
     WebCore::IntRect localBounds(const CachedNode* ,
         const WebCore::IntRect& ) const;
-    /**
-     * Find the next textfield/textarea
-     * @param start Must be a CachedNode in this CachedFrame's tree, or
-     *              null, in which case we start from the beginning.
-     * @param framePtr  If not null, and a textfield/textarea is found, its
-     *                  CachedFrame will be pointed to by this pointer.
-     * @return CachedNode* Next textfield (or area)
-     */
-    const CachedNode* nextTextField(const CachedNode* start,
-        const CachedFrame** framePtr) const;
     const CachedFrame* parent() const { return mParent; }
     CachedFrame* parent() { return mParent; }
     SkPicture* picture(const CachedNode* ) const;
@@ -152,6 +142,8 @@ public:
     }
     const CachedNode* validDocument() const;
 protected:
+    const CachedNode* nextTextField(const CachedNode* start,
+        const CachedFrame** framePtr, bool* found) const;
     struct BestData {
         int mDistance;
         int mSideDistance;
diff --git a/WebKit/android/nav/CachedRoot.cpp b/WebKit/android/nav/CachedRoot.cpp
index 71c0993..115a0f9 100644
--- a/WebKit/android/nav/CachedRoot.cpp
+++ b/WebKit/android/nav/CachedRoot.cpp
@@ -1288,6 +1288,13 @@ const CachedNode* CachedRoot::moveCursor(Direction direction, const CachedFrame*
     return const_cast<CachedNode*>(bestData.mNode);
 }
 
+const CachedNode* CachedRoot::nextTextField(const CachedNode* start,
+        const CachedFrame** framePtr) const
+{
+    bool startFound = false;
+    return CachedFrame::nextTextField(start, framePtr, &startFound);
+}
+
 SkPicture* CachedRoot::pictureAt(int x, int y) const
 {
 #if USE(ACCELERATED_COMPOSITING)
diff --git a/WebKit/android/nav/CachedRoot.h b/WebKit/android/nav/CachedRoot.h
index 735f23b..6e9fff0 100644
--- a/WebKit/android/nav/CachedRoot.h
+++ b/WebKit/android/nav/CachedRoot.h
@@ -75,6 +75,14 @@ public:
     WebCore::String imageURI(int x, int y) const;
     bool maskIfHidden(BestData* ) const;
     const CachedNode* moveCursor(Direction , const CachedFrame** , WebCore::IntPoint* scroll);
+    /**
+     * Find the next textfield/textarea
+     * @param start         The textfield/textarea to search from.
+     * @param framePtr      If non-zero, returns CachedFrame* containing result.
+     * @return CachedNode*  Next textfield/textarea or null (0) if none.
+     */
+    const CachedNode* nextTextField(const CachedNode* start,
+        const CachedFrame** framePtr) const;
     SkPicture* pictureAt(int x, int y) const;
     void reset();
     CachedHistory* rootHistory() const { return mHistory; }
diff --git a/WebKit/android/nav/WebView.cpp b/WebKit/android/nav/WebView.cpp
index 0ac6bd6..bdcafbf 100644
--- a/WebKit/android/nav/WebView.cpp
+++ b/WebKit/android/nav/WebView.cpp
@@ -1223,10 +1223,15 @@ static const CachedNode* getFocusCandidate(JNIEnv *env, jobject obj,
 
 static bool focusCandidateHasNextTextfield(JNIEnv *env, jobject obj)
 {
-    const CachedFrame* frame;
-    const CachedNode* cursor = getFocusCandidate(env, obj, &frame);
+    WebView* view = GET_NATIVE_VIEW(env, obj);
+    CachedRoot* root = view->getFrameCache(WebView::DontAllowNewer);
+    if (!root)
+        return false;
+    const CachedNode* cursor = root->currentCursor();
+    if (!cursor || !cursor->isTextInput())
+        cursor = root->currentFocus();
     if (!cursor || !cursor->isTextInput()) return false;
-    return frame->nextTextField(cursor, 0);
+    return root->nextTextField(cursor, 0);
 }
 
 static const CachedNode* getFocusNode(JNIEnv *env, jobject obj)
@@ -1760,14 +1765,13 @@ static bool nativeMoveCursorToNextTextInput(JNIEnv *env, jobject obj)
     CachedRoot* root = view->getFrameCache(WebView::DontAllowNewer);
     if (!root)
         return false;
-    const CachedFrame* containingFrame;
-    const CachedNode* current = root->currentCursor(&containingFrame);
-    if (!current)
-        current = root->currentFocus(&containingFrame);
-    if (!current)
+    const CachedNode* current = root->currentCursor();
+    if (!current || !current->isTextInput())
+        current = root->currentFocus();
+    if (!current || !current->isTextInput())
         return false;
     const CachedFrame* frame;
-    const CachedNode* next = containingFrame->nextTextField(current, &frame);
+    const CachedNode* next = root->nextTextField(current, &frame);
     if (!next)
         return false;
     const WebCore::IntRect& bounds = next->bounds(frame);