Use Chrome facilities for omnibox state save and restore on Mac.

TabContents has a facility for storing a bag of stuff across
current-tab changes.  Wire up AutocompleteEditViewMac to use that
facility.  Unfork some code in Browser::TabSelectedAt() so that the
new code gets used, and straighten up the Mac code along the code path
between there and AutocompleteEditViewMac.

This overall change also exposed a couple bugs/mis-features in the
AutocompleteEditViewMac code.

TEST=Text field maintains contents and selection across tab changes, even when edited.

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

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

8 files changed, 142 insertions, 51 deletions

diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_mac.h b/chrome/browser/autocomplete/autocomplete_edit_view_mac.h
index 521f590..a693841 100644
--- a/chrome/browser/autocomplete/autocomplete_edit_view_mac.h
+++ b/chrome/browser/autocomplete/autocomplete_edit_view_mac.h
@@ -43,9 +43,7 @@ class AutocompleteEditViewMac : public AutocompleteEditView {
   virtual const AutocompleteEditModel* model() const { return model_.get(); }
 
   virtual void SaveStateToTab(TabContents* tab);
-  virtual void Update(const TabContents* tab_for_state_restoring) {
-    NOTIMPLEMENTED();
-  }
+  virtual void Update(const TabContents* tab_for_state_restoring);
 
   virtual void OpenURL(const GURL& url,
                        WindowOpenDisposition disposition,
@@ -55,10 +53,12 @@ class AutocompleteEditViewMac : public AutocompleteEditView {
                        const std::wstring& keyword);
 
   virtual std::wstring GetText() const;
-  virtual void SetUserText(const std::wstring& text) { NOTIMPLEMENTED(); }
+  virtual void SetUserText(const std::wstring& text) {
+    SetUserText(text, text, true);
+  }
   virtual void SetUserText(const std::wstring& text,
                            const std::wstring& display_text,
-                           bool update_popup) { NOTIMPLEMENTED(); }
+                           bool update_popup);
 
   virtual void SetWindowTextAndCaretPos(const std::wstring& text,
                                         size_t caret_pos);
diff --git a/chrome/browser/autocomplete/autocomplete_edit_view_mac.mm b/chrome/browser/autocomplete/autocomplete_edit_view_mac.mm
index c9ca5fc..e1a2a53 100644
--- a/chrome/browser/autocomplete/autocomplete_edit_view_mac.mm
+++ b/chrome/browser/autocomplete/autocomplete_edit_view_mac.mm
@@ -8,6 +8,41 @@
 #include "chrome/browser/autocomplete/autocomplete_edit.h"
 #include "chrome/browser/autocomplete/autocomplete_popup_model.h"
 #include "chrome/browser/autocomplete/autocomplete_popup_view_mac.h"
+#include "chrome/browser/tab_contents/tab_contents.h"
+
+namespace {
+
+// Store's the model and view state across tab switches.
+struct AutocompleteEditViewMacState {
+  AutocompleteEditViewMacState(const AutocompleteEditModel::State model_state,
+                               const bool has_focus, const NSRange& selection)
+      : model_state(model_state),
+        has_focus(has_focus),
+        selection(selection) {
+  }
+
+  const AutocompleteEditModel::State model_state;
+  const bool has_focus;
+  const NSRange selection;
+};
+
+// Returns a lazily initialized property bag accessor for saving our
+// state in a TabContents.
+PropertyAccessor<AutocompleteEditViewMacState>* GetStateAccessor() {
+  static PropertyAccessor<AutocompleteEditViewMacState> state;
+  return &state;
+}
+
+// Accessors for storing and getting the state from the tab.
+void StoreStateToTab(TabContents* tab,
+                     const AutocompleteEditViewMacState& state) {
+  GetStateAccessor()->SetProperty(tab->property_bag(), state);
+}
+const AutocompleteEditViewMacState* GetStateFromTab(const TabContents* tab) {
+  return GetStateAccessor()->GetProperty(tab->property_bag());
+}
+
+}  // namespace
 
 // Thin Obj-C bridge class that is the delegate of the omnibox field.
 // It intercepts various control delegate methods and vectors them to
@@ -57,19 +92,68 @@ AutocompleteEditViewMac::~AutocompleteEditViewMac() {
   [field_ setDelegate:nil];
 }
 
-// TODO(shess): This is the minimal change which seems to unblock
-// getting the minimal Omnibox code checked in without making the
-// world worse.  Browser::TabSelectedAt() calls this when the tab
-// changes, but that is only wired up for Windows.  I do not yet
-// understand that code well enough to go for it.  Once wired up, then
-// code can be removed at:
-// [TabContentsController defocusLocationBar]
-// [TabStripController selectTabWithContents:...]
 void AutocompleteEditViewMac::SaveStateToTab(TabContents* tab) {
-  // TODO(shess): Actually save the state to the tab area.
+  DCHECK(tab);
+
+  NSRange range;
+  if (model_->has_focus()) {
+    range = GetSelectedRange();
+  } else {
+    // If we are not focussed, there is no selection.  Manufacture
+    // something reasonable in case it starts to matter in the future.
+    range = NSMakeRange(0, [[field_ stringValue] length]);
+  }
 
-  // Drop the popup before we change to another tab.
-  ClosePopup();
+  AutocompleteEditViewMacState state(model_->GetStateForTabSwitch(),
+                                     model_->has_focus(), range);
+  StoreStateToTab(tab, state);
+}
+
+void AutocompleteEditViewMac::Update(
+    const TabContents* tab_for_state_restoring) {
+  // TODO(shess): It seems like if the tab is non-NULL, then this code
+  // shouldn't need to be called at all.  When coded that way, I find
+  // that the field isn't always updated correctly.  Figure out why
+  // this is.  Maybe this method should be refactored into more
+  // specific cases.
+  const std::wstring text = toolbar_model_->GetText();
+  const bool user_visible = model_->UpdatePermanentText(text);
+
+  if (tab_for_state_restoring) {
+    RevertAll();
+
+    const AutocompleteEditViewMacState* state = 
+        GetStateFromTab(tab_for_state_restoring);
+    if (state) {
+      // Should restore the user's text via SetUserText().
+      model_->RestoreState(state->model_state);
+
+      // Restore user's selection.
+      // TODO(shess): The model_ does not restore the focus state.  If
+      // field_ was in focus when we switched away, I presume it
+      // should be in focus when we switch back.  Figure out if model_
+      // not restoring focus is an oversight, or intentional for some
+      // subtle reason.
+      if (state->has_focus) {
+        FocusLocation();
+        DCHECK([field_ currentEditor]);
+        [[field_ currentEditor] setSelectedRange:state->selection];
+      }
+    }
+  } else if (user_visible) {
+    // Restore everything to the baseline look.
+    RevertAll();
+    // TODO(shess): Figure out how this case is used, to make sure
+    // we're getting the selection and popup right.
+
+  } else {
+    // TODO(shess): Figure out how this case is used, to make sure
+    // we're getting the selection and popup right.
+    // UpdateAndStyleText() approximates the inner part of Revertall()
+    // which under GTK is called EmphasizeURLComponents(), and is
+    // expected to change when I start feeding in the styling code.
+    UpdateAndStyleText(text, 0);
+  }
 }
 
 void AutocompleteEditViewMac::OpenURL(const GURL& url,
@@ -97,6 +181,16 @@ std::wstring AutocompleteEditViewMac::GetText() const {
   return base::SysNSStringToWide([field_ stringValue]);
 }
 
+void AutocompleteEditViewMac::SetUserText(const std::wstring& text,
+                                          const std::wstring& display_text,
+                                          bool update_popup) {
+  model_->SetUserText(text);
+  UpdateAndStyleText(display_text, display_text.size());
+  if (update_popup) {
+    UpdatePopup();
+  }
+}
+
 NSRange AutocompleteEditViewMac::GetSelectedRange() const {
   DCHECK([field_ currentEditor]);
   return [[field_ currentEditor] selectedRange];
@@ -119,7 +213,7 @@ void AutocompleteEditViewMac::RevertAll() {
   model_->Revert();
 
   std::wstring tt = GetText();
-  UpdateAndStyleText(tt, tt.size());
+  UpdateAndStyleText(tt, 0);
   controller_->OnChanged();
 }
 
@@ -156,7 +250,7 @@ void AutocompleteEditViewMac::UpdateAndStyleText(
     [as addAttribute:NSForegroundColorAttributeName
                value:[NSColor greenColor]
                range:NSMakeRange((NSInteger)parts.host.begin,
-                                 (NSInteger)parts.host.end())];
+                                 (NSInteger)parts.host.len)];
   }
 
   // TODO(shess): GTK has this as a member var, figure out why.
@@ -175,7 +269,7 @@ void AutocompleteEditViewMac::UpdateAndStyleText(
     }
     [as addAttribute:NSForegroundColorAttributeName value:color
                range:NSMakeRange((NSInteger)parts.scheme.begin,
-                                 (NSInteger)parts.scheme.end())];
+                                 (NSInteger)parts.scheme.len)];
   }
 
   // TODO(shess): Check that this updates the model's sense of focus
@@ -183,7 +277,7 @@ void AutocompleteEditViewMac::UpdateAndStyleText(
   [field_ setObjectValue:as];
   if (![field_ currentEditor]) {
     [field_ becomeFirstResponder];
-    DCHECK_EQ(field_, [[field_ window] firstResponder]);
+    DCHECK_EQ([field_ currentEditor], [[field_ window] firstResponder]);
   }
 
   NSRange selected_range = NSMakeRange(user_text_length, [as length]);
@@ -281,7 +375,12 @@ void AutocompleteEditViewMac::AcceptInput(
 }
 
 void AutocompleteEditViewMac::FocusLocation() {
-  [[field_ window] makeFirstResponder:field_];
+  // -makeFirstResponder: will select the entire field_.  If we're
+  // already firstResponder, it's likely that we want to retain the
+  // current selection.
+  if (![field_ currentEditor]) {
+    [[field_ window] makeFirstResponder:field_];
+  }
 }
 
 @implementation AutocompleteFieldDelegate
diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc
index 955a3af..79130d5 100644
--- a/chrome/browser/browser.cc
+++ b/chrome/browser/browser.cc
@@ -1590,13 +1590,9 @@ void Browser::TabSelectedAt(TabContents* old_contents,
     ProcessPendingUIUpdates();
 
   if (old_contents) {
-#if defined(OS_WIN)
     // Save what the user's currently typing, so it can be restored when we
     // switch back to this tab.
     window_->GetLocationBar()->SaveStateToContents(old_contents);
-#else
-    NOTIMPLEMENTED();
-#endif
   }
 
   // Propagate the profile to the location bar.
diff --git a/chrome/browser/cocoa/browser_window_controller.mm b/chrome/browser/cocoa/browser_window_controller.mm
index 08250f1..c0150d5f 100644
--- a/chrome/browser/cocoa/browser_window_controller.mm
+++ b/chrome/browser/cocoa/browser_window_controller.mm
@@ -290,7 +290,8 @@ willPositionSheet:(NSWindow *)sheet
 
 - (void)updateToolbarWithContents:(TabContents*)tab
                shouldRestoreState:(BOOL)shouldRestore {
-  [toolbarController_ updateToolbarWithContents:shouldRestore ? tab : NULL];
+  [toolbarController_ updateToolbarWithContents:tab
+                             shouldRestoreState:shouldRestore];
 }
 
 - (void)setStarredState:(BOOL)isStarred {
diff --git a/chrome/browser/cocoa/location_bar_view_mac.h b/chrome/browser/cocoa/location_bar_view_mac.h
index ee9cf26..12f32fa 100644
--- a/chrome/browser/cocoa/location_bar_view_mac.h
+++ b/chrome/browser/cocoa/location_bar_view_mac.h
@@ -46,6 +46,11 @@ class LocationBarViewMac : public AutocompleteEditController,
     return edit_view_.get();
   }
 
+  // Updates the location bar.  Resets the bar's permanent text and
+  // security style, and if |should_restore_state| is true, restores
+  // saved state from the tab (for tab switching).
+  void Update(const TabContents* tab, bool should_restore_state);
+
   virtual void OnAutocompleteAccept(const GURL& url,
       WindowOpenDisposition disposition,
       PageTransition::Type transition,
diff --git a/chrome/browser/cocoa/location_bar_view_mac.mm b/chrome/browser/cocoa/location_bar_view_mac.mm
index b8c521f..138a567 100644
--- a/chrome/browser/cocoa/location_bar_view_mac.mm
+++ b/chrome/browser/cocoa/location_bar_view_mac.mm
@@ -56,6 +56,12 @@ void LocationBarViewMac::SaveStateToContents(TabContents* contents) {
   edit_view_->SaveStateToTab(contents);
 }
 
+void LocationBarViewMac::Update(const TabContents* contents,
+                                bool should_restore_state) {
+  // AutocompleteEditView restores state if the tab is non-NULL.
+  edit_view_->Update(should_restore_state ? contents : NULL);
+}
+
 void LocationBarViewMac::OnAutocompleteAccept(const GURL& url,
                                               WindowOpenDisposition disposition,
                                               PageTransition::Type transition,
diff --git a/chrome/browser/cocoa/toolbar_controller.h b/chrome/browser/cocoa/toolbar_controller.h
index 8967aee..1909800 100644
--- a/chrome/browser/cocoa/toolbar_controller.h
+++ b/chrome/browser/cocoa/toolbar_controller.h
@@ -52,9 +52,12 @@ class ToolbarView;
 // Make the location bar the first responder, if possible.
 - (void)focusLocationBar;
 
-// Called when any url bar state changes. If |tabForRestoring| is non-NULL,
-// it points to a TabContents whose state we should restore.
-- (void)updateToolbarWithContents:(TabContents*)tabForRestoring;
+// Updates the toolbar (and transitively the location bar) with the states of
+// the specified |tab|.  If |shouldRestore| is true, we're switching
+// (back?) to this tab and should restore any previous location bar state
+// (such as user editing) as well.
+- (void)updateToolbarWithContents:(TabContents*)tabForRestoring
+               shouldRestoreState:(BOOL)shouldRestore;
 
 // Sets whether or not the current page in the frontmost tab is bookmarked.
 - (void)setStarredState:(BOOL)isStarred;
diff --git a/chrome/browser/cocoa/toolbar_controller.mm b/chrome/browser/cocoa/toolbar_controller.mm
index d2a986e..0860a0b 100644
--- a/chrome/browser/cocoa/toolbar_controller.mm
+++ b/chrome/browser/cocoa/toolbar_controller.mm
@@ -48,7 +48,6 @@ static NSString* const kStarredImageName = @"starred";
   [self initCommandStatus:commands_];
   locationBarView_.reset(new LocationBarViewMac(locationBar_, commands_,
                                                 toolbarModel_, profile_));
-  [locationBar_ setStringValue:@"http://dev.chromium.org"];
 }
 
 - (void)dealloc {
@@ -98,27 +97,9 @@ static NSString* const kStarredImageName = @"starred";
   [starButton_ setEnabled:commands->IsCommandEnabled(IDC_STAR) ? YES : NO];
 }
 
-- (void)updateToolbarWithContents:(TabContents*)tab {
-  // TODO(pinkerton): there's a lot of ui code in autocomplete_edit.cc
-  // that we'll want to duplicate. For now, just handle setting the text.
-
-  // TODO(shess): This is the start of what pinkerton refers to.
-  // Unfortunately, I'm going to need to spend some time wiring things
-  // up.  This call should suffice to close any open autocomplete
-  // pulldown.  It should also be the right thing to do to save and
-  // restore state, but at this time it's not clear that this is the
-  // right place, and tab is not the right parameter.
-  if (locationBarView_.get()) {
-    locationBarView_->SaveStateToContents(NULL);
-  }
-
-  // TODO(pinkerton): update the security lock icon and background color
-
-  // TODO(shess): Determine whether this should happen via
-  // locationBarView_, instead, in which case this class can
-  // potentially lose the locationBar_ reference.
-  NSString* urlString = base::SysWideToNSString(toolbarModel_->GetText());
-  [locationBar_ setStringValue:urlString];
+- (void)updateToolbarWithContents:(TabContents*)tab
+               shouldRestoreState:(BOOL)shouldRestore {
+  locationBarView_->Update(tab, shouldRestore ? true : false);
 }
 
 - (void)setStarredState:(BOOL)isStarred {