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

chrome/browser/instant/instant_controller.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CHROME_BROWSER_INSTANT_INSTANT_CONTROLLER_H_
 #define CHROME_BROWSER_INSTANT_INSTANT_CONTROLLER_H_
 
 #include <map>
 #include <string>
 #include <vector>
@@ skipping 73 matching lines @@
   // has typed. |full_text| is what the omnibox is showing. These may differ if
   // the user typed only some text, and the rest was inline autocompleted. If
   // |verbatim| is true, search results are shown for the exact omnibox text,
   // rather than the best guess as to what the user means. Returns true if the
   // update is accepted (i.e., if |match| is a search rather than a URL).
   bool Update(const AutocompleteMatch& match,
               const string16& user_text,
               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.
   void HandleAutocompleteResults(
       const std::vector<AutocompleteProvider*>& providers);
 
   // Called when the user presses up or down. |count| is a repeat count,
   // negative for moving up, positive for moving down. Returns true if Instant
   // handled the key press.
   bool OnUpOrDownKeyPressed(int count);
@@ skipping 96 matching lines @@
   // Calls OnStaleLoader if |stale_loader_timer_| is not running.
   void MaybeOnStaleLoader();
 
   // Destroys the |loader_| and its preview contents.
   void DeleteLoader();
 
   // Counterpart to Hide(). Asks the |browser_| to display the preview with
   // 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
   // |instant_url| and returns true; returns false otherwise.
   // Note: If the command-line switch kInstantURL is set, this method uses its
   // value for |instant_url| and returns true without examining |template_url|.
   bool GetInstantURL(const TemplateURL* template_url,
                      const GURL& tab_url,
                      std::string* instant_url) const;
 
@@ skipping 24 matching lines @@
 
   // True if the last match passed to Update() was a search (versus a URL).
   bool last_match_was_search_;
 
   // True if the omnibox is focused, false otherwise.
   bool is_omnibox_focused_;
 
   // 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_;
 
   // Timer used to ensure that the Instant page does not get too stale.
   base::OneShotTimer<InstantController> stale_loader_timer_;
 
   // For each key K => value N, the map says that we found that the search
   // engine identified by Instant URL K didn't support the Instant API in each
   // of the last N times that we loaded it. If an Instant URL isn't present in
   // the map at all or has a value 0, it means that search engine supports the
   // Instant API (or we assume it does, since we haven't determined it doesn't).
   std::map<std::string, int> blacklisted_urls_;
 
   // Search terms extraction (for autocomplete history matches) doesn't work
   // on Instant URLs. So, whenever the user commits an Instant search, we add
   // an equivalent non-Instant search URL to history, so that the search shows
   // up in autocomplete history matches.
   GURL url_for_history_;
 
   DISALLOW_COPY_AND_ASSIGN(InstantController);
 };
 
 #endif  // CHROME_BROWSER_INSTANT_INSTANT_CONTROLLER_H_