Switching out of keyword mode shouldn't count as a minimal change.

Switching out of keyword mode shouldn't count as a minimal change.

Previously, if you used space to get into keyword mode and later backspace at
the beginning of the keyword to get out of keyword mode, the backspace would
trigger an autocompletion run with |minimal_changes| == true.  This isn't really
correct; switching out of keyword mode can affect the results and scoring
significantly.  In certain cases, the old route triggered a DCHECK, because
the search provider didn't wipe the old keyword matches across
|minimal_changes|, but the new input had no corresponding keyword input text.

I looked briefly at leaving this as a "minimal change" and instead detecting and
handling it inside the relevant provider(s), but decided that was error-prone
and not really as semantically accurate.  Plus the potential additional flicker
here is fairly minor, as people don't usually exit keyword mode via backspace in
this way.

BUG=607379
TEST=In a debug build, type "google.comgoogle.com" (no quotes) into the omnibox, place the cursor between the two "google.com"s, hit space, then hit backspace.  Chrome should not DCHECK.

Committed: https://crrev.com/1919906f73c5c4919651c0d87185e6fb927384ce
Cr-Commit-Position: refs/heads/master@{#407888}

[Peter Kasting, 2016-07-26 02:45:48]

The CQ bit was checked by pkasting@chromium.org to run a CQ dry run

[Peter Kasting, 2016-07-26 02:45:57]

pkasting@chromium.org changed reviewers:
+ mpearson@chromium.org

[commit-bot: I haz the power, 2016-07-26 02:46:07]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-26 03:54:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-26 03:54:37]

Dry run: This issue passed the CQ dry run.

[Mark P, 2016-07-26 19:42:32]

lgtm

I agree with this change.  Thanks for the detailed explanation.

--mark

[Peter Kasting, 2016-07-26 19:52:56]

The CQ bit was checked by pkasting@chromium.org

[commit-bot: I haz the power, 2016-07-26 19:53:32]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-26 19:59:33]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-07-26 20:01:46]

Description was changed from

==========
Switching out of keyword mode shouldn't count as a minimal change.

Previously, if you used space to get into keyword mode and later backspace at
the beginning of the keyword to get out of keyword mode, the backspace would
trigger an autocompletion run with |minimal_changes| == true.  This isn't really
correct; switching out of keyword mode can affect the results and scoring
significantly.  In certain cases, the old route triggered a DCHECK, because
the search provider didn't wipe the old keyword matches across
|minimal_changes|, but the new input had no corresponding keyword input text.

I looked briefly at leaving this as a "minimal change" and instead detecting and
handling it inside the relevant provider(s), but decided that was error-prone
and not really as semantically accurate.  Plus the potential additional flicker
here is fairly minor, as people don't usually exit keyword mode via backspace in
this way.

BUG=607379
TEST=In a debug build, type "google.comgoogle.com" (no quotes) into the omnibox,
place the cursor between the two "google.com"s, hit space, then hit backspace. 
Chrome should not DCHECK.
==========

to

==========
Switching out of keyword mode shouldn't count as a minimal change.

Previously, if you used space to get into keyword mode and later backspace at
the beginning of the keyword to get out of keyword mode, the backspace would
trigger an autocompletion run with |minimal_changes| == true.  This isn't really
correct; switching out of keyword mode can affect the results and scoring
significantly.  In certain cases, the old route triggered a DCHECK, because
the search provider didn't wipe the old keyword matches across
|minimal_changes|, but the new input had no corresponding keyword input text.

I looked briefly at leaving this as a "minimal change" and instead detecting and
handling it inside the relevant provider(s), but decided that was error-prone
and not really as semantically accurate.  Plus the potential additional flicker
here is fairly minor, as people don't usually exit keyword mode via backspace in
this way.

BUG=607379
TEST=In a debug build, type "google.comgoogle.com" (no quotes) into the omnibox,
place the cursor between the two "google.com"s, hit space, then hit backspace. 
Chrome should not DCHECK.

Committed: https://crrev.com/1919906f73c5c4919651c0d87185e6fb927384ce
Cr-Commit-Position: refs/heads/master@{#407888}
==========

[commit-bot: I haz the power, 2016-07-26 20:01:47]

Patchset 1 (id:??) landed as
https://crrev.com/1919906f73c5c4919651c0d87185e6fb927384ce
Cr-Commit-Position: refs/heads/master@{#407888}