We used to store/restore the frame and node that were focused last when a age would lose/gain focus.

This patches lets WebKit takes care of it by using the new FocusController::setFocused method.

BUG=http://crbug.com/15777
TEST=Open www.google.com, the focus should be on the text-field (focus ring + blinking caret). Activate another window. The page should not show the text-field with the focus. Switch tabs, make sure the focus is remembered correctly. Do the same tests in a page with frames. Run the layout tests.


Review URL: http://codereview.chromium.org/151195

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@19899 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 11 insertions, 146 deletions

diff --git a/chrome/renderer/render_view.cc b/chrome/renderer/render_view.cc
index 0d65aac..5b98519 100644
--- a/chrome/renderer/render_view.cc
+++ b/chrome/renderer/render_view.cc
@@ -2144,21 +2144,9 @@ void RenderView::OnFind(int request_id,
       }
     }
 
-    // TODO(jcampan): http://b/issue?id=1157486 Remove StoreForFocus call once
-    //                we have the fix for 792423.
-    search_frame->GetView()->StoreFocusForFrame(search_frame);
     webview()->SetFocusedFrame(search_frame);
   } while (!result && search_frame != focused_frame);
 
-  // Make sure we don't leave any frame focused or the focus won't be restored
-  // properly in WebViewImpl::SetFocus().  Note that we are talking here about
-  // focused on the SelectionController, not FocusController.
-  // webview()->GetFocusedFrame() will still return the last focused frame (as
-  // it queries the FocusController).
-  // TODO(jcampan): http://b/issue?id=1157486 Remove next line once we have the
-  //                fix for 792423.
-  webview()->SetFocusedFrame(NULL);
-
   if (options.findNext) {
     // Force the main_frame to report the actual count.
     main_frame->IncreaseMatchCount(0, request_id);
diff --git a/webkit/glue/webframe_impl.cc b/webkit/glue/webframe_impl.cc
index 3f26032..20f4b2a 100644
--- a/webkit/glue/webframe_impl.cc
+++ b/webkit/glue/webframe_impl.cc
@@ -1263,10 +1263,6 @@ void WebFrameImpl::SetFindEndstateFocusAndSelection() {
     if (!selection.isNone())
       return;
 
-    // We will be setting focus ourselves, so we want the view to forget its
-    // stored focus node so that it won't change it after we are done.
-    static_cast<WebViewImpl*>(GetView())->ReleaseFocusReferences();
-
     // Try to find the first focusable node up the chain, which will, for
     // example, focus links if we have found text within the link.
     Node* node = active_match_->firstNode();
diff --git a/webkit/glue/webframeloaderclient_impl.cc b/webkit/glue/webframeloaderclient_impl.cc
index 58fe059..3a65c91 100644
--- a/webkit/glue/webframeloaderclient_impl.cc
+++ b/webkit/glue/webframeloaderclient_impl.cc
@@ -644,10 +644,6 @@ void WebFrameLoaderClient::dispatchDidChangeLocationWithinPage() {
 
 void WebFrameLoaderClient::dispatchWillClose() {
   WebViewImpl* webview = webframe_->GetWebViewImpl();
-  // Make sure WebViewImpl releases the references it uses to restore focus.
-  // If we didn't do this, WebViewImpl might try to restore focus to an invalid
-  // element.
-  webview->ReleaseFocusReferences();
   WebViewDelegate* d = webview->delegate();
   if (d)
     d->WillCloseFrame(webview, webframe_);
diff --git a/webkit/glue/webview.h b/webkit/glue/webview.h
index deb3903..08a9042 100644
--- a/webkit/glue/webview.h
+++ b/webkit/glue/webview.h
@@ -112,22 +112,9 @@ class WebView : public WebWidget {
 
   // ---- TODO(darin): remove to here ----
 
-  // Restores focus to the previously focused element.
-  // This method is invoked when the webview is shown after being
-  // hidden, and focus is to be restored. When WebView loses focus, it remembers
-  // the frame/element that had focus, so that when this method is invoked
-  // focus is then restored.
-  virtual void RestoreFocus() = 0;
-
   // Focus the first (last if reverse is true) focusable node.
   virtual void SetInitialFocus(bool reverse) = 0;
 
-  // Stores the focused node and clears it if |frame| is the focused frame.
-  // TODO(jcampan): http://b/issue?id=1157486 this is needed to work-around
-  // issues caused by the fix for bug #792423 and should be removed when that
-  // bug is fixed.
-  virtual void StoreFocusForFrame(WebFrame* frame) = 0;
-
   // Clears the focused node (and selection if a text field is focused) to
   // ensure that a text field on the page is not eating keystrokes we send it.
   virtual void ClearFocusedNode() = 0;
diff --git a/webkit/glue/webview_impl.cc b/webkit/glue/webview_impl.cc
index ea18c4a..03cf13e 100644
--- a/webkit/glue/webview_impl.cc
+++ b/webkit/glue/webview_impl.cc
@@ -379,7 +379,6 @@ WebViewImpl::WebViewImpl()
 
 WebViewImpl::~WebViewImpl() {
   DCHECK(page_ == NULL);
-  ReleaseFocusReferences();
   for (std::set<ImageResourceFetcher*>::iterator i = image_fetchers_.begin();
        i != image_fetchers_.end(); ++i) {
     delete *i;
@@ -1097,10 +1096,10 @@ void WebViewImpl::SetBackForwardListSize(int size) {
 }
 
 void WebViewImpl::ClearFocusedNode() {
-  // Get the last focused frame or the mainframe.
-  RefPtr<Frame> frame = last_focused_frame_;
-  if (!frame.get() && page_.get())
-    frame = page_->mainFrame();
+  if (!page_.get())
+    return;
+
+  RefPtr<Frame> frame = page_->mainFrame();
   if (!frame.get())
     return;
 
@@ -1130,29 +1129,15 @@ void WebViewImpl::ClearFocusedNode() {
 }
 
 void WebViewImpl::SetFocus(bool enable) {
+  page_->focusController()->setFocused(enable);
   if (enable) {
-    // Getting the focused frame will have the side-effect of setting the main
-    // frame as the focused frame if it is not already focused.  Otherwise, if
-    // there is already a focused frame, then this does nothing.
-    GetFocusedFrame();
-    if (page_.get() && page_->mainFrame()) {
-      Frame* frame = page_->mainFrame();
-      if (!frame->selection()->isFocusedAndActive()) {
-        // No one has focus yet, try to restore focus.
-        RestoreFocus();
-        page_->focusController()->setActive(true);
-      }
-      Frame* focused_frame = page_->focusController()->focusedOrMainFrame();
-      frame->selection()->setFocused(frame == focused_frame);
-    }
+    // Note that we don't call setActive() when disabled as this cause extra
+    // focus/blur events to be dispatched.
+    page_->focusController()->setActive(true);
     ime_accept_events_ = true;
   } else {
     HideAutoCompletePopup();
 
-    // Clear out who last had focus. If someone has focus, the refs will be
-    // updated below.
-    ReleaseFocusReferences();
-
     // Clear focus on the currently focused frame if any.
     if (!page_.get())
       return;
@@ -1161,61 +1146,14 @@ void WebViewImpl::SetFocus(bool enable) {
     if (!frame)
       return;
 
-    RefPtr<Frame> focused = page_->focusController()->focusedFrame();
-    if (focused.get()) {
-      // Update the focus refs, this way we can give focus back appropriately.
-      // It's entirely possible to have a focused document, but not a focused
-      // node.
-      RefPtr<Document> document = focused->document();
-      last_focused_frame_ = focused;
-      if (document.get()) {
-        RefPtr<Node> focused_node = document->focusedNode();
-        if (focused_node.get()) {
-          // To workaround bug #792423, we do not blur the focused node.  This
-          // should be reenabled when we merge a WebKit that has the fix for
-          // http://bugs.webkit.org/show_bug.cgi?id=16928.
-          // last_focused_node_ = focused_node;
-          // document->setFocusedNode(NULL);
-        }
-      }
-      page_->focusController()->setFocusedFrame(0);
-
+    RefPtr<Frame> focused_frame = page_->focusController()->focusedFrame();
+    if (focused_frame.get()) {
       // Finish an ongoing composition to delete the composition node.
-      Editor* editor = focused->editor();
+      Editor* editor = focused_frame->editor();
       if (editor && editor->hasComposition())
         editor->confirmComposition();
       ime_accept_events_ = false;
     }
-    // Make sure the main frame doesn't think it has focus.
-    if (frame != focused.get())
-      frame->selection()->setFocused(false);
-  }
-}
-
-// TODO(jcampan): http://b/issue?id=1157486 this is needed to work-around
-// issues caused by the fix for bug #792423 and should be removed when that
-// bug is fixed.
-void WebViewImpl::StoreFocusForFrame(WebFrame* frame) {
-  DCHECK(frame);
-
-  // We only want to store focus info if we are the focused frame and if we have
-  // not stored it already.
-  WebCore::Frame* webcore_frame = static_cast<WebFrameImpl*>(frame)->frame();
-  if (last_focused_frame_.get() != webcore_frame || last_focused_node_.get())
-    return;
-
-  // Clear out who last had focus. If someone has focus, the refs will be
-  // updated below.
-  ReleaseFocusReferences();
-
-  last_focused_frame_ = webcore_frame;
-  RefPtr<Document> document = last_focused_frame_->document();
-  if (document.get()) {
-    RefPtr<Node> focused_node = document->focusedNode();
-    if (focused_node.get()) {
-      last_focused_node_ = focused_node;
-      document->setFocusedNode(NULL);
-    }
   }
 }
 
@@ -1369,29 +1307,8 @@ void WebViewImpl::SetTextDirection(WebTextDirection direction) {
   }
 }
 
-void WebViewImpl::RestoreFocus() {
-  if (last_focused_frame_.get()) {
-    if (last_focused_frame_->page()) {
-      // last_focused_frame_ can be detached from the frame tree, thus,
-      // its page can be null.
-      last_focused_frame_->page()->focusController()->setFocusedFrame(
-        last_focused_frame_.get());
-    }
-    if (last_focused_node_.get()) {
-      // last_focused_node_ may be null, make sure it's valid before trying to
-      // focus it.
-      static_cast<Element*>(last_focused_node_.get())->focus();
-    }
-    // And clear out the refs.
-    ReleaseFocusReferences();
-  }
-}
-
 void WebViewImpl::SetInitialFocus(bool reverse) {
   if (page_.get()) {
-    // So RestoreFocus does not focus anything when it is called.
-    ReleaseFocusReferences();
-
     // Since we don't have a keyboard event, we'll create one.
     WebKeyboardEvent keyboard_event;
     keyboard_event.type = WebInputEvent::RawKeyDown;
@@ -1409,14 +1326,6 @@ void WebViewImpl::SetInitialFocus(bool reverse) {
   }
 }
 
-// Releases references used to restore focus.
-void WebViewImpl::ReleaseFocusReferences() {
-  if (last_focused_frame_.get()) {
-    last_focused_frame_.release();
-    last_focused_node_.release();
-  }
-}
-
 bool WebViewImpl::DownloadImage(int id, const GURL& image_url, int image_size) {
   if (!page_.get())
     return false;
diff --git a/webkit/glue/webview_impl.h b/webkit/glue/webview_impl.h
index 7635dbf..c87e0ba 100644
--- a/webkit/glue/webview_impl.h
+++ b/webkit/glue/webview_impl.h
@@ -73,7 +73,6 @@ class WebViewImpl : public WebView, public base::RefCounted<WebViewImpl> {
   virtual void MouseCaptureLost();
   virtual void SetFocus(bool enable);
   virtual void ClearFocusedNode();
-  virtual void StoreFocusForFrame(WebFrame* frame);
   virtual bool ImeSetComposition(int string_type,
                                  int cursor_position,
                                  int target_start,
@@ -84,7 +83,6 @@ class WebViewImpl : public WebView, public base::RefCounted<WebViewImpl> {
   virtual void SetTextDirection(WebTextDirection direction);
   virtual void StopLoading();
   virtual void SetBackForwardListSize(int size);
-  virtual void RestoreFocus();
   virtual void SetInitialFocus(bool reverse);
   virtual bool DownloadImage(int id, const GURL& image_url, int image_size);
   virtual void SetPreferences(const WebPreferences& preferences);
@@ -188,9 +186,6 @@ class WebViewImpl : public WebView, public base::RefCounted<WebViewImpl> {
   // Mouse button down event.
   bool SendContextMenuEvent(const WebKit::WebKeyboardEvent& event);
 
-  // Releases references used to restore focus.
-  void ReleaseFocusReferences();
-
   // Notifies the WebView that a load has been committed.
   // is_new_navigation will be true if a new session history item should be
   // created for that load.
@@ -238,12 +233,6 @@ class WebViewImpl : public WebView, public base::RefCounted<WebViewImpl> {
   WebKit::WebSize size_;
 
   WebKit::WebPoint last_mouse_position_;
-  // Reference to the Frame that last had focus. This is set once when
-  // we lose focus, and used when focus is gained to reinstall focus to
-  // the correct element.
-  RefPtr<WebCore::Frame> last_focused_frame_;
-  // Reference to the node that last had focus.
-  RefPtr<WebCore::Node> last_focused_node_;
   scoped_ptr<WebCore::Page> page_;
 
   webkit_glue::BackForwardListClientImpl back_forward_list_client_impl_;