Implement the Instant extended API startMargin, endMargin, and rtl properties and the onmarginchang…

chrome/browser/instant/instant_controller.h

Index: chrome/browser/instant/instant_controller.h
diff --git a/chrome/browser/instant/instant_controller.h b/chrome/browser/instant/instant_controller.h
index 7c9f11c834bf9191966b2bb3bd1ce88cf990caac..6e08215b4325763e1576e8e7517f39765f283008 100644
--- a/chrome/browser/instant/instant_controller.h
+++ b/chrome/browser/instant/instant_controller.h
@@ -91,7 +91,10 @@ class InstantController {
               const string16& full_text,
               bool verbatim);
 
-  // Sets the bounds of the omnibox dropdown, in screen coordinates.
+  // Sets the bounds of the omnibox popup, in screen coordinates.
+  void SetPopupBounds(const gfx::Rect& bounds);

[dhollowa, 2012/11/14 17:20:46]

Are both the popup bounds and omnibox bounds still in use?  Or could you remove
the omnibox bounds?

[samarth, 2012/11/14 19:02:04]

I think we want omnibox bounds and not popup bounds since the popup doesn't
exist anymore.

[melevin, 2012/11/14 22:16:27]

We might want to do this in a separate change. The popup bounds are currently
used to provide the Searchbox API x, y, width, and height properties. Although
they are isolated as unused in the new API, they are still a part of it.

[dhollowa, 2012/11/14 22:53:56]

We shouldn't really be setting/sending full "bounds" if we only need left/right
"margin" information.

[melevin, 2012/11/15 23:40:01]

I've split the bounds into separate start and end ints.

+
+  // Sets the bounds of the omnibox itself, in screen coordinates.
   void SetOmniboxBounds(const gfx::Rect& bounds);
 
   // Send autocomplete results from |providers| to the preview page.
@@ -208,7 +211,7 @@ class InstantController {
   // the given |height|.
   void Show(InstantShownReason reason, int height, InstantSizeUnits units);
 
-  // Send the omnibox dropdown bounds to the page.
+  // Send the omnibox popup bounds to the page.
   void SendBoundsToPage();
 
   // If |template_url| is a valid TemplateURL for use with Instant, fills in
@@ -253,12 +256,15 @@ class InstantController {
   // True if the active tab in the current window is the NTP, false otherwise.
   bool active_tab_is_ntp_;
 
+  // Current omnibox popup bounds.

[dhollowa, 2012/11/14 17:20:46]

Would it be possible to eliminate this "cache" of bounds?  The same bounds are
held on the renderer side, as well as in the OmniboxView class, so having a
third copy here seems like a potential source of error.

[melevin, 2012/11/14 22:16:27]

I don't think so.

We need at least one additional set of bounds here to be able to tell when the
bounds changed, so we need at least "last_popup_bounds_". But it also turns out
that "popup_bounds_" is used to pass arguments to the SendBoundsToPage function.
The reason we can't pass arguments directly to SendBoundsToPage is due to the
way base::OneShotTimer works. It has to take a argument-less member function as
an argument.

+  gfx::Rect popup_bounds_;
+
+  // Last popup bounds passed to the page.
+  gfx::Rect last_popup_bounds_;
+
   // Current omnibox bounds.
   gfx::Rect omnibox_bounds_;
 
-  // Last bounds passed to the page.
-  gfx::Rect last_omnibox_bounds_;
-
   // Timer used to update the bounds of the omnibox.
   base::OneShotTimer<InstantController> update_bounds_timer_;