Change the SearchBox API from using the start/end margins of the location bar to using the start margin and width. This change is necessary to avoid accounting for the presence of the scrollbar in the search page JavaScript.

BUG=153403

Review URL: https://chromiumcodereview.appspot.com/12047107

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

12 files changed, 75 insertions, 71 deletions

diff --git a/chrome/browser/instant/instant_controller.cc b/chrome/browser/instant/instant_controller.cc
index 9725d5e..624e081 100644
--- a/chrome/browser/instant/instant_controller.cc
+++ b/chrome/browser/instant/instant_controller.cc
@@ -203,8 +203,6 @@ InstantController::InstantController(chrome::BrowserInstantController* browser,
       last_transition_type_(content::PAGE_TRANSITION_LINK),
       last_match_was_search_(false),
       omnibox_focus_state_(OMNIBOX_FOCUS_NONE),
-      start_margin_(0),
-      end_margin_(0),
       allow_preview_to_show_search_suggestions_(false),
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)) {
 }
@@ -467,18 +465,17 @@ void InstantController::SetPopupBounds(const gfx::Rect& bounds) {
   }
 }
 
-void InstantController::SetMarginSize(int start, int end) {
-  if (!extended_enabled_ || (start_margin_ == start && end_margin_ == end))
+void InstantController::SetOmniboxBounds(const gfx::Rect& bounds) {
+  if (!extended_enabled_ || omnibox_bounds_ == bounds)
     return;
 
-  start_margin_ = start;
-  end_margin_ = end;
+  omnibox_bounds_ = bounds;
   if (overlay_)
-    overlay_->SetMarginSize(start_margin_, end_margin_);
+    overlay_->SetOmniboxBounds(omnibox_bounds_);
   if (ntp_)
-    ntp_->SetMarginSize(start_margin_, end_margin_);
+    ntp_->SetOmniboxBounds(omnibox_bounds_);
   if (instant_tab_)
-    instant_tab_->SetMarginSize(start_margin_, end_margin_);
+    instant_tab_->SetOmniboxBounds(omnibox_bounds_);
 }
 
 void InstantController::HandleAutocompleteResults(
@@ -896,11 +893,11 @@ void InstantController::InstantPageRenderViewCreated(
     overlay_->SetDisplayInstantResults(instant_enabled_);
     overlay_->KeyCaptureChanged(
         omnibox_focus_state_ == OMNIBOX_FOCUS_INVISIBLE);
-    overlay_->SetMarginSize(start_margin_, end_margin_);
+    overlay_->SetOmniboxBounds(omnibox_bounds_);
     overlay_->InitializeFonts();
   } else if (IsContentsFrom(ntp(), contents)) {
     ntp_->SetDisplayInstantResults(instant_enabled_);
-    ntp_->SetMarginSize(start_margin_, end_margin_);
+    ntp_->SetOmniboxBounds(omnibox_bounds_);
     ntp_->InitializeFonts();
   } else {
     NOTREACHED();
@@ -1230,7 +1227,7 @@ void InstantController::ResetInstantTab() {
       // Update theme info for this tab.
       browser_->UpdateThemeInfo(false);
       instant_tab_->SetDisplayInstantResults(instant_enabled_);
-      instant_tab_->SetMarginSize(start_margin_, end_margin_);
+      instant_tab_->SetOmniboxBounds(omnibox_bounds_);
       instant_tab_->InitializeFonts();
       StartListeningToMostVisitedChanges();
       instant_tab_->KeyCaptureChanged(
diff --git a/chrome/browser/instant/instant_controller.h b/chrome/browser/instant/instant_controller.h
index def4880..e9e1d5f 100644
--- a/chrome/browser/instant/instant_controller.h
+++ b/chrome/browser/instant/instant_controller.h
@@ -101,8 +101,8 @@ class InstantController : public InstantPage::Delegate,
   // Sets the bounds of the omnibox popup, in screen coordinates.
   void SetPopupBounds(const gfx::Rect& bounds);
 
-  // Sets the start and end margins of the omnibox text area.
-  void SetMarginSize(int start, int end);
+  // Sets the stored start-edge margin and width of the omnibox.
+  void SetOmniboxBounds(const gfx::Rect& bounds);
 
   // Send autocomplete results from |providers| to the preview page.
   void HandleAutocompleteResults(
@@ -401,11 +401,9 @@ class InstantController : public InstantPage::Delegate,
   // Last popup bounds passed to the page.
   gfx::Rect last_popup_bounds_;
 
-  // Size of the start-edge omnibox text area margin.
-  int start_margin_;
-
-  // Size of the end-edge omnibox text area margin.
-  int end_margin_;
+  // The start-edge margin and width of the omnibox, used by the page to align
+  // its suggestions with the omnibox.
+  gfx::Rect omnibox_bounds_;
 
   // Timer used to update the bounds of the omnibox popup.
   base::OneShotTimer<InstantController> update_bounds_timer_;
diff --git a/chrome/browser/instant/instant_extended_browsertest.cc b/chrome/browser/instant/instant_extended_browsertest.cc
index f0fc514..9e03d54 100644
--- a/chrome/browser/instant/instant_extended_browsertest.cc
+++ b/chrome/browser/instant/instant_extended_browsertest.cc
@@ -4,6 +4,7 @@
 
 #include <sstream>
 
+#include "base/prefs/pref_service.h"
 #include "chrome/browser/favicon/favicon_tab_helper.h"
 #include "chrome/browser/instant/instant_commit_type.h"
 #include "chrome/browser/instant/instant_ntp.h"
@@ -12,9 +13,11 @@
 #include "chrome/browser/instant/instant_service_factory.h"
 #include "chrome/browser/instant/instant_tab.h"
 #include "chrome/browser/instant/instant_test_utils.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/search/search.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/chrome_notification_types.h"
+#include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/test/base/interactive_test_utils.h"
 #include "chrome/test/base/ui_test_utils.h"
@@ -718,3 +721,38 @@ IN_PROC_BROWSER_TEST_F(InstantExtendedTest, MostVisited) {
   // Make sure we have the same number of items as before.
   EXPECT_EQ(most_visited_items_count_, old_most_visited_items_count);
 }
+
+// Only implemented in Views currently: http://crbug.com/164723
+#if defined(OS_WIN) || defined(OS_CHROMEOS)
+#define MAYBE_HomeButtonAffectsMargin HomeButtonAffectsMargin
+#else
+#define MAYBE_HomeButtonAffectsMargin DISABLED_HomeButtonAffectsMargin
+#endif
+// Check that toggling the state of the home button changes the start-edge
+// margin and width.
+IN_PROC_BROWSER_TEST_F(InstantExtendedTest, MAYBE_HomeButtonAffectsMargin) {
+  ASSERT_NO_FATAL_FAILURE(SetupInstant());
+
+  // Get the current value of the start-edge margin and width.
+  int start_margin;
+  int width;
+  content::WebContents* preview_tab = instant()->GetPreviewContents();
+  EXPECT_TRUE(GetIntFromJS(preview_tab, "chrome.searchBox.startMargin",
+      &start_margin));
+  EXPECT_TRUE(GetIntFromJS(preview_tab, "chrome.searchBox.width", &width));
+
+  // Toggle the home button visibility pref.
+  PrefService* profile_prefs = browser()->profile()->GetPrefs();
+  bool show_home = profile_prefs->GetBoolean(prefs::kShowHomeButton);
+  profile_prefs->SetBoolean(prefs::kShowHomeButton, !show_home);
+
+  // Make sure the margin and width changed.
+  int new_start_margin;
+  int new_width;
+  EXPECT_TRUE(GetIntFromJS(preview_tab, "chrome.searchBox.startMargin",
+      &new_start_margin));
+  EXPECT_TRUE(GetIntFromJS(preview_tab, "chrome.searchBox.width", &new_width));
+  EXPECT_NE(start_margin, new_start_margin);
+  EXPECT_NE(width, new_width);
+  EXPECT_EQ(new_width - width, start_margin - new_start_margin);
+}
diff --git a/chrome/browser/instant/instant_page.cc b/chrome/browser/instant/instant_page.cc
index 85f5699..ba79353 100644
--- a/chrome/browser/instant/instant_page.cc
+++ b/chrome/browser/instant/instant_page.cc
@@ -36,8 +36,9 @@ void InstantPage::SetPopupBounds(const gfx::Rect& bounds) {
   Send(new ChromeViewMsg_SearchBoxPopupResize(routing_id(), bounds));
 }
 
-void InstantPage::SetMarginSize(const int start, const int end) {
-  Send(new ChromeViewMsg_SearchBoxMarginChange(routing_id(), start, end));
+void InstantPage::SetOmniboxBounds(const gfx::Rect& bounds) {
+  Send(new ChromeViewMsg_SearchBoxMarginChange(
+      routing_id(), bounds.x(), bounds.width()));
 }
 
 void InstantPage::InitializeFonts() {
diff --git a/chrome/browser/instant/instant_page.h b/chrome/browser/instant/instant_page.h
index f0167c2..1260beb1 100644
--- a/chrome/browser/instant/instant_page.h
+++ b/chrome/browser/instant/instant_page.h
@@ -131,10 +131,9 @@ class InstantPage : public content::WebContentsObserver {
   // by the omnibox dropdown.
   void SetPopupBounds(const gfx::Rect& bounds);
 
-  // Tells the page the start and end margins of the omnibox (in screen
-  // coordinates). This is used by the page to align text or assets properly
-  // with the omnibox.
-  void SetMarginSize(int start, int end);
+  // Tells the page the bounds of the omnibox (in screen coordinates). This is
+  // used by the page to align text or assets properly with the omnibox.
+  void SetOmniboxBounds(const gfx::Rect& bounds);
 
   // Tells the page about the font information.
   void InitializeFonts();
diff --git a/chrome/browser/ui/browser_instant_controller.cc b/chrome/browser/ui/browser_instant_controller.cc
index 1027ece..2d17390 100644
--- a/chrome/browser/ui/browser_instant_controller.cc
+++ b/chrome/browser/ui/browser_instant_controller.cc
@@ -248,8 +248,8 @@ void BrowserInstantController::OpenURL(
                                            false));
 }
 
-void BrowserInstantController::SetMarginSize(int start, int end) {
-  instant_.SetMarginSize(start, end);
+void BrowserInstantController::SetOmniboxBounds(const gfx::Rect& bounds) {
+  instant_.SetOmniboxBounds(bounds);
 }
 
 void BrowserInstantController::ResetInstant() {
diff --git a/chrome/browser/ui/browser_instant_controller.h b/chrome/browser/ui/browser_instant_controller.h
index 965fe43..80cfde0 100644
--- a/chrome/browser/ui/browser_instant_controller.h
+++ b/chrome/browser/ui/browser_instant_controller.h
@@ -109,8 +109,8 @@ class BrowserInstantController : public content::NotificationObserver,
                content::PageTransition transition,
                WindowOpenDisposition disposition);
 
-  // Sets the start and end margins of the omnibox text area.
-  void SetMarginSize(int start, int end);
+  // Sets the stored omnibox bounds.
+  void SetOmniboxBounds(const gfx::Rect& bounds);
 
  private:
   // Sets the value of |instant_| based on value from profile. Invoked
diff --git a/chrome/browser/ui/views/location_bar/location_bar_view.cc b/chrome/browser/ui/views/location_bar/location_bar_view.cc
index e8a8d68..68306ee 100644
--- a/chrome/browser/ui/views/location_bar/location_bar_view.cc
+++ b/chrome/browser/ui/views/location_bar/location_bar_view.cc
@@ -1275,14 +1275,7 @@ bool LocationBarView::HasFocus() const {
 
 void LocationBarView::OnBoundsChanged(const gfx::Rect& previous_bounds) {
   if (browser_ && browser_->instant_controller() && parent()) {
-    // Pass the side margins of the location bar to the Instant Controller.
-    const gfx::Rect bounds = GetBoundsInScreen();
-    const gfx::Rect parent_bounds = parent()->GetBoundsInScreen();
-    int start = bounds.x() - parent_bounds.x();
-    int end = parent_bounds.right() - bounds.right();
-    if (base::i18n::IsRTL())
-      std::swap(start, end);
-    browser_->instant_controller()->SetMarginSize(start, end);
+    browser_->instant_controller()->SetOmniboxBounds(bounds());
   }
 }
 
diff --git a/chrome/renderer/resources/extensions/searchbox_api.js b/chrome/renderer/resources/extensions/searchbox_api.js
index becc8ff..820aa63 100644
--- a/chrome/renderer/resources/extensions/searchbox_api.js
+++ b/chrome/renderer/resources/extensions/searchbox_api.js
@@ -85,7 +85,6 @@ if (!chrome.embeddedSearch) {
       native function GetSelectionStart();
       native function GetSelectionEnd();
       native function GetStartMargin();
-      native function GetEndMargin();
       native function GetRightToLeft();
       native function GetAutocompleteResults();
       native function GetDisplayInstantResults();
@@ -223,7 +222,6 @@ if (!chrome.embeddedSearch) {
       this.__defineGetter__('selectionStart', GetSelectionStart);
       this.__defineGetter__('selectionEnd', GetSelectionEnd);
       this.__defineGetter__('startMargin', GetStartMargin);
-      this.__defineGetter__('endMargin', GetEndMargin);
       this.__defineGetter__('rtl', GetRightToLeft);
       this.__defineGetter__('nativeSuggestions', GetAutocompleteResultsWrapper);
       this.__defineGetter__('isKeyCaptureEnabled', IsKeyCaptureEnabled);
diff --git a/chrome/renderer/searchbox/searchbox.cc b/chrome/renderer/searchbox/searchbox.cc
index 5e76b79..3631412 100644
--- a/chrome/renderer/searchbox/searchbox.cc
+++ b/chrome/renderer/searchbox/searchbox.cc
@@ -28,7 +28,6 @@ SearchBox::SearchBox(content::RenderView* render_view)
       selection_end_(0),
       results_base_(0),
       start_margin_(0),
-      end_margin_(0),
       last_results_base_(0),
       is_key_capture_enabled_(false),
       display_instant_results_(false),
@@ -99,10 +98,6 @@ int SearchBox::GetStartMargin() const {
   return static_cast<int>(start_margin_ / GetZoom());
 }
 
-int SearchBox::GetEndMargin() const {
-  return static_cast<int>(end_margin_ / GetZoom());
-}
-
 gfx::Rect SearchBox::GetPopupBounds() const {
   double zoom = GetZoom();
   return gfx::Rect(static_cast<int>(popup_bounds_.x() / zoom),
@@ -216,9 +211,12 @@ void SearchBox::OnPopupResize(const gfx::Rect& bounds) {
   }
 }
 
-void SearchBox::OnMarginChange(int start, int end) {
-  start_margin_ = start;
-  end_margin_ = end;
+void SearchBox::OnMarginChange(int margin, int width) {
+  start_margin_ = margin;
+
+  // Override only the width parameter of the popup bounds.
+  popup_bounds_.set_width(width);
+
   if (render_view()->GetWebView() && render_view()->GetWebView()->mainFrame()) {
     extensions_v8::SearchBoxExtension::DispatchMarginChange(
         render_view()->GetWebView()->mainFrame());
@@ -312,7 +310,6 @@ void SearchBox::Reset() {
   results_base_ = 0;
   popup_bounds_ = gfx::Rect();
   start_margin_ = 0;
-  end_margin_ = 0;
   autocomplete_results_.clear();
   is_key_capture_enabled_ = false;
   theme_info_ = ThemeBackgroundInfo();
diff --git a/chrome/renderer/searchbox/searchbox.h b/chrome/renderer/searchbox/searchbox.h
index 6b5d6d4..445d4dc 100644
--- a/chrome/renderer/searchbox/searchbox.h
+++ b/chrome/renderer/searchbox/searchbox.h
@@ -65,10 +65,9 @@ class SearchBox : public content::RenderViewObserver,
   const string16& omnibox_font() const { return omnibox_font_; }
   size_t omnibox_font_size() const { return omnibox_font_size_; }
 
-  // These functions return the start/end margins of the page text area,
-  // adjusted for the page zoom.
+  // In extended Instant, returns the start-edge margin of the location bar in
+  // screen pixels.
   int GetStartMargin() const;
-  int GetEndMargin() const;
 
   // Returns the bounds of the omnibox popup in screen coordinates.
   gfx::Rect GetPopupBounds() const;
@@ -100,7 +99,7 @@ class SearchBox : public content::RenderViewObserver,
   void OnSubmit(const string16& query);
   void OnCancel(const string16& query);
   void OnPopupResize(const gfx::Rect& bounds);
-  void OnMarginChange(int start, int end);
+  void OnMarginChange(int margin, int width);
   void OnDetermineIfPageSupportsInstant();
   void OnAutocompleteResults(
       const std::vector<InstantAutocompleteResult>& results);
@@ -126,7 +125,6 @@ class SearchBox : public content::RenderViewObserver,
   size_t selection_end_;
   size_t results_base_;
   int start_margin_;
-  int end_margin_;
   gfx::Rect popup_bounds_;
   std::vector<InstantAutocompleteResult> autocomplete_results_;
   size_t last_results_base_;
diff --git a/chrome/renderer/searchbox/searchbox_extension.cc b/chrome/renderer/searchbox/searchbox_extension.cc
index 65bf71e..cb74530 100644
--- a/chrome/renderer/searchbox/searchbox_extension.cc
+++ b/chrome/renderer/searchbox/searchbox_extension.cc
@@ -252,7 +252,8 @@ class SearchBoxExtensionWrapper : public v8::Extension {
   // region of the search box that overlaps the window.
   static v8::Handle<v8::Value> GetY(const v8::Arguments& args);
 
-  // Gets the width of the region of the search box that overlaps the window.
+  // Gets the width of the region of the search box that overlaps the window,
+  // i.e., the width of the omnibox.
   static v8::Handle<v8::Value> GetWidth(const v8::Arguments& args);
 
   // Gets the height of the region of the search box that overlaps the window.
@@ -261,14 +262,9 @@ class SearchBoxExtensionWrapper : public v8::Extension {
   // Gets Most Visited Items.
   static v8::Handle<v8::Value> GetMostVisitedItems(const v8::Arguments& args);
 
-  // Gets the width of the margin from the start-edge of the page to the start
-  // of the suggestions dropdown.
+  // Gets the start-edge margin to use with extended Instant.
   static v8::Handle<v8::Value> GetStartMargin(const v8::Arguments& args);
 
-  // Gets the width of the margin from the end-edge of the page to the end of
-  // the suggestions dropdown.
-  static v8::Handle<v8::Value> GetEndMargin(const v8::Arguments& args);
-
   // Returns true if the Searchbox itself is oriented right-to-left.
   static v8::Handle<v8::Value> GetRightToLeft(const v8::Arguments& args);
 
@@ -367,8 +363,6 @@ v8::Handle<v8::FunctionTemplate> SearchBoxExtensionWrapper::GetNativeFunction(
     return v8::FunctionTemplate::New(GetMostVisitedItems);
   if (name->Equals(v8::String::New("GetStartMargin")))
     return v8::FunctionTemplate::New(GetStartMargin);
-  if (name->Equals(v8::String::New("GetEndMargin")))
-    return v8::FunctionTemplate::New(GetEndMargin);
   if (name->Equals(v8::String::New("GetRightToLeft")))
     return v8::FunctionTemplate::New(GetRightToLeft);
   if (name->Equals(v8::String::New("GetAutocompleteResults")))
@@ -482,7 +476,6 @@ v8::Handle<v8::Value> SearchBoxExtensionWrapper::GetWidth(
     const v8::Arguments& args) {
   content::RenderView* render_view = GetRenderView();
   if (!render_view) return v8::Undefined();
-
   return v8::Int32::New(SearchBox::Get(render_view)->GetPopupBounds().width());
 }
 
@@ -504,14 +497,6 @@ v8::Handle<v8::Value> SearchBoxExtensionWrapper::GetStartMargin(
 }
 
 // static
-v8::Handle<v8::Value> SearchBoxExtensionWrapper::GetEndMargin(
-    const v8::Arguments& args) {
-  content::RenderView* render_view = GetRenderView();
-  if (!render_view) return v8::Undefined();
-  return v8::Int32::New(SearchBox::Get(render_view)->GetEndMargin());
-}
-
-// static
 v8::Handle<v8::Value> SearchBoxExtensionWrapper::GetRightToLeft(
     const v8::Arguments& args) {
   return v8::Boolean::New(base::i18n::IsRTL());