Pass user_text and full_text explicitly to Instant.

Pass user_text and full_text explicitly to Instant.

Keeps the promise made in https://chromiumcodereview.appspot.com/10824261#msg8
to not pass the inline autocompletion on its own. Also uses the correct piece
(user_text instead of full_text) in the extended API.

BUG=none
R=pkasting@chromium.org,sky@chromium.org
TEST=none; no change in functionality.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=153422

[sreeram, 2012-08-24 20:17:07]

Please review.

[sky, 2012-08-24 21:33:15]

LGTM

[Peter Kasting, 2012-08-24 22:27:19]

I don't really consider myself an instant expert...

http://codereview.chromium.org/10867061/diff/1/chrome/browser/instant/instant...
File chrome/browser/instant/instant_controller.cc (right):

http://codereview.chromium.org/10867061/diff/1/chrome/browser/instant/instant...
chrome/browser/instant/instant_controller.cc:183: string16 last_query_text =
mode_ == EXTENDED ? last_user_text_
Nit: Break after ==, not before :

http://codereview.chromium.org/10867061/diff/1/chrome/browser/instant/instant...
chrome/browser/instant/instant_controller.cc:191: *suggested_text =
last_suggestion_.text;
This looks like a case where we set this outparam to the result of a previous
query.  I'm not seeing a parallel place where we will set this outparam as the
result of _this_ query.  How do we communicate back that something should be
inlined from the current query as opposed to a previous one?

I'm asking all this because the name and function of this "suggested_text"
argument is somewhat confusing to me and I'm wondering if it can or should be
eliminated or reworked somehow, now that it's not used to pass data into this
function.

http://codereview.chromium.org/10867061/diff/1/chrome/browser/ui/omnibox/omni...
File chrome/browser/ui/omnibox/omnibox_edit_model.cc (right):

http://codereview.chromium.org/10867061/diff/1/chrome/browser/ui/omnibox/omni...
chrome/browser/ui/omnibox/omnibox_edit_model.cc:1135: if (has_temporary_text_) {
Nit: Remove {}

http://codereview.chromium.org/10867061/diff/1/chrome/browser/ui/omnibox/omni...
chrome/browser/ui/omnibox/omnibox_edit_model.cc:1159: 
NIt: Don't really need this new line

[sreeram, 2012-08-24 23:09:55]

http://codereview.chromium.org/10867061/diff/1/chrome/browser/instant/instant...
File chrome/browser/instant/instant_controller.cc (right):

http://codereview.chromium.org/10867061/diff/1/chrome/browser/instant/instant...
chrome/browser/instant/instant_controller.cc:183: string16 last_query_text =
mode_ == EXTENDED ? last_user_text_
On 2012/08/24 22:27:19, Peter Kasting wrote:
> Nit: Break after ==, not before :

Done.

http://codereview.chromium.org/10867061/diff/1/chrome/browser/instant/instant...
chrome/browser/instant/instant_controller.cc:191: *suggested_text =
last_suggestion_.text;
On 2012/08/24 22:27:19, Peter Kasting wrote:
> This looks like a case where we set this outparam to the result of a previous
> query.  I'm not seeing a parallel place where we will set this outparam as the
> result of _this_ query.  How do we communicate back that something should be
> inlined from the current query as opposed to a previous one?

For the current query (if it's not a duplicate of the previous one), we have to
send it to the renderer, and wait for a response. That's handled further below
in InstantController::SetSuggestions(), circa line 472.

> I'm asking all this because the name and function of this "suggested_text"
> argument is somewhat confusing to me and I'm wondering if it can or should be
> eliminated or reworked somehow, now that it's not used to pass data into this
> function.

Good point. Eliminated.

http://codereview.chromium.org/10867061/diff/1/chrome/browser/ui/omnibox/omni...
File chrome/browser/ui/omnibox/omnibox_edit_model.cc (right):

http://codereview.chromium.org/10867061/diff/1/chrome/browser/ui/omnibox/omni...
chrome/browser/ui/omnibox/omnibox_edit_model.cc:1135: if (has_temporary_text_) {
On 2012/08/24 22:27:19, Peter Kasting wrote:
> Nit: Remove {}

Done.

http://codereview.chromium.org/10867061/diff/1/chrome/browser/ui/omnibox/omni...
chrome/browser/ui/omnibox/omnibox_edit_model.cc:1159: 
On 2012/08/24 22:27:19, Peter Kasting wrote:
> NIt: Don't really need this new line

Done.

[Peter Kasting, 2012-08-24 23:13:08]

LGTM

http://codereview.chromium.org/10867061/diff/7002/chrome/browser/ui/omnibox/o...
File chrome/browser/ui/omnibox/omnibox_edit_model.cc (right):

http://codereview.chromium.org/10867061/diff/7002/chrome/browser/ui/omnibox/o...
chrome/browser/ui/omnibox/omnibox_edit_model.cc:186: void
OmniboxEditModel::SetSuggestedText(const string16& text,
Is this function now only called from OnWillKillFocus()?  Maybe it can be
simplified/inlined?

[sreeram, 2012-08-24 23:22:33]

http://codereview.chromium.org/10867061/diff/7002/chrome/browser/ui/omnibox/o...
File chrome/browser/ui/omnibox/omnibox_edit_model.cc (right):

http://codereview.chromium.org/10867061/diff/7002/chrome/browser/ui/omnibox/o...
chrome/browser/ui/omnibox/omnibox_edit_model.cc:186: void
OmniboxEditModel::SetSuggestedText(const string16& text,
On 2012/08/24 23:13:08, Peter Kasting wrote:
> Is this function now only called from OnWillKillFocus()?  Maybe it can be
> simplified/inlined?

No, it's called from the UI code as well. When Instant calls
delegate_->SetSuggestedText, that call goes to Browser -> LocationBar (each
platform variant) -> OmniboxEditModel.