Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(75)

Side by Side Diff: chrome/browser/autocomplete/search_provider.cc

Issue 67693004: Omnibox: Don't Let Users Escape Keyword Mode Accidentally (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Mike's comments Created 7 years, 1 month ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch | Annotate | Revision Log
OLDNEW
1 // Copyright 2012 The Chromium Authors. All rights reserved. 1 // Copyright 2012 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "chrome/browser/autocomplete/search_provider.h" 5 #include "chrome/browser/autocomplete/search_provider.h"
6 6
7 #include <algorithm> 7 #include <algorithm>
8 #include <cmath> 8 #include <cmath>
9 9
10 #include "base/callback.h" 10 #include "base/callback.h"
(...skipping 1298 matching lines...) Expand 10 before | Expand all | Expand 10 after
1309 1309
1310 bool SearchProvider::IsTopMatchNavigationInKeywordMode( 1310 bool SearchProvider::IsTopMatchNavigationInKeywordMode(
1311 bool autocomplete_result_will_reorder_for_default_match) const { 1311 bool autocomplete_result_will_reorder_for_default_match) const {
1312 ACMatches::const_iterator first_match = 1312 ACMatches::const_iterator first_match =
1313 FindTopMatch(autocomplete_result_will_reorder_for_default_match); 1313 FindTopMatch(autocomplete_result_will_reorder_for_default_match);
1314 return !providers_.keyword_provider().empty() && 1314 return !providers_.keyword_provider().empty() &&
1315 (first_match != matches_.end()) && 1315 (first_match != matches_.end()) &&
1316 (first_match->type == AutocompleteMatchType::NAVSUGGEST); 1316 (first_match->type == AutocompleteMatchType::NAVSUGGEST);
1317 } 1317 }
1318 1318
1319 bool SearchProvider::HasKeywordDefaultMatchInKeywordMode() const {
1320 const TemplateURL* keyword_url = providers_.GetKeywordProviderURL();
1321 // If the user is not in keyword mode, return true to say that this
1322 // constraint is not violated.
1323 if (keyword_url == NULL)
1324 return true;
1325 for (ACMatches::const_iterator it = matches_.begin(); it != matches_.end();
1326 ++it) {
1327 if ((it->keyword == keyword_url->keyword()) &&
1328 it->allowed_to_be_default_match)
1329 return true;
1330 }
1331 return false;
1332 }
1333
1319 bool SearchProvider::IsTopMatchScoreTooLow( 1334 bool SearchProvider::IsTopMatchScoreTooLow(
1320 bool autocomplete_result_will_reorder_for_default_match) const { 1335 bool autocomplete_result_will_reorder_for_default_match) const {
1321 // In reorder mode, there's no such thing as a score that's too low. 1336 // In reorder mode, there's no such thing as a score that's too low.
1322 if (autocomplete_result_will_reorder_for_default_match) 1337 if (autocomplete_result_will_reorder_for_default_match)
1323 return false; 1338 return false;
1324 1339
1325 // Here we use CalculateRelevanceForVerbatimIgnoringKeywordModeState() 1340 // Here we use CalculateRelevanceForVerbatimIgnoringKeywordModeState()
1326 // rather than CalculateRelevanceForVerbatim() because the latter returns 1341 // rather than CalculateRelevanceForVerbatim() because the latter returns
1327 // a very low score (250) if keyword mode is active. This is because 1342 // a very low score (250) if keyword mode is active. This is because
1328 // when keyword mode is active the user probably wants the keyword matches, 1343 // when keyword mode is active the user probably wants the keyword matches,
(...skipping 46 matching lines...) Expand 10 before | Expand all | Expand 10 after
1375 1390
1376 // True if the omnibox will reorder matches as necessary to make the first 1391 // True if the omnibox will reorder matches as necessary to make the first
1377 // one something that is allowed to be the default match. 1392 // one something that is allowed to be the default match.
1378 const bool omnibox_will_reorder_for_legal_default_match = 1393 const bool omnibox_will_reorder_for_legal_default_match =
1379 OmniboxFieldTrial::ReorderForLegalDefaultMatch( 1394 OmniboxFieldTrial::ReorderForLegalDefaultMatch(
1380 input_.current_page_classification()); 1395 input_.current_page_classification());
1381 if (IsTopMatchNavigationInKeywordMode( 1396 if (IsTopMatchNavigationInKeywordMode(
1382 omnibox_will_reorder_for_legal_default_match)) { 1397 omnibox_will_reorder_for_legal_default_match)) {
1383 // Correct the suggested relevance scores if the top match is a 1398 // Correct the suggested relevance scores if the top match is a
1384 // navigation in keyword mode, since inlining a navigation match 1399 // navigation in keyword mode, since inlining a navigation match
1385 // would break the user out of keyword mode. By the way, if the top 1400 // would break the user out of keyword mode. This will only be
1386 // match is a non-keyword match (query or navsuggestion) in keyword 1401 // triggered in regular (non-reorder) mode; in reorder mode,
1387 // mode, the user would also break out of keyword mode. However, 1402 // navigation matches are marked as not allowed to be the default
1388 // that situation is impossible given the current scoring paradigm 1403 // match and hence IsTopMatchNavigation() will always return false.
1389 // and the fact that only one search engine (Google) provides suggested 1404 DCHECK(!omnibox_will_reorder_for_legal_default_match);
1390 // relevance scores at this time.
1391 DemoteKeywordNavigationMatchesPastTopQuery(); 1405 DemoteKeywordNavigationMatchesPastTopQuery();
1392 ConvertResultsToAutocompleteMatches(); 1406 ConvertResultsToAutocompleteMatches();
1393 DCHECK(!IsTopMatchNavigationInKeywordMode( 1407 DCHECK(!IsTopMatchNavigationInKeywordMode(
1394 omnibox_will_reorder_for_legal_default_match)); 1408 omnibox_will_reorder_for_legal_default_match));
1395 } 1409 }
1410 if (!HasKeywordDefaultMatchInKeywordMode()) {
1411 // In keyword mode, disregard the keyword verbatim suggested relevance
1412 // if necessary so there at least one keyword match that's allowed to
1413 // be the default match.
1414 keyword_results_.verbatim_relevance = -1;
1415 ConvertResultsToAutocompleteMatches();
1416 }
1396 if (IsTopMatchScoreTooLow(omnibox_will_reorder_for_legal_default_match)) { 1417 if (IsTopMatchScoreTooLow(omnibox_will_reorder_for_legal_default_match)) {
1397 // Disregard the suggested verbatim relevance if the top score is below 1418 // Disregard the suggested verbatim relevance if the top score is below
1398 // the usual verbatim value. For example, a BarProvider may rely on 1419 // the usual verbatim value. For example, a BarProvider may rely on
1399 // SearchProvider's verbatim or inlineable matches for input "foo" (all 1420 // SearchProvider's verbatim or inlineable matches for input "foo" (all
1400 // allowed to be default match) to always outrank its own lowly-ranked 1421 // allowed to be default match) to always outrank its own lowly-ranked
1401 // "bar" matches that shouldn't be the default match. 1422 // "bar" matches that shouldn't be the default match.
1402 default_results_.verbatim_relevance = -1; 1423 default_results_.verbatim_relevance = -1;
1403 keyword_results_.verbatim_relevance = -1; 1424 keyword_results_.verbatim_relevance = -1;
1404 ConvertResultsToAutocompleteMatches(); 1425 ConvertResultsToAutocompleteMatches();
1405 } 1426 }
(...skipping 18 matching lines...) Expand all
1424 // outranked all other matches. On the other hand, if the omnibox will 1445 // outranked all other matches. On the other hand, if the omnibox will
1425 // reorder matches as necessary to put a legal default match at the top, 1446 // reorder matches as necessary to put a legal default match at the top,
1426 // all we need to guarantee is that SearchProvider returns a legal 1447 // all we need to guarantee is that SearchProvider returns a legal
1427 // default match. (The omnibox always needs at least one legal default 1448 // default match. (The omnibox always needs at least one legal default
1428 // match, and it relies on SearchProvider to always return one.) 1449 // match, and it relies on SearchProvider to always return one.)
1429 ApplyCalculatedRelevance(); 1450 ApplyCalculatedRelevance();
1430 ConvertResultsToAutocompleteMatches(); 1451 ConvertResultsToAutocompleteMatches();
1431 } 1452 }
1432 DCHECK(!IsTopMatchNavigationInKeywordMode( 1453 DCHECK(!IsTopMatchNavigationInKeywordMode(
1433 omnibox_will_reorder_for_legal_default_match)); 1454 omnibox_will_reorder_for_legal_default_match));
1455 DCHECK(HasKeywordDefaultMatchInKeywordMode());
1434 DCHECK(!IsTopMatchScoreTooLow( 1456 DCHECK(!IsTopMatchScoreTooLow(
1435 omnibox_will_reorder_for_legal_default_match)); 1457 omnibox_will_reorder_for_legal_default_match));
1436 DCHECK(!IsTopMatchSearchWithURLInput( 1458 DCHECK(!IsTopMatchSearchWithURLInput(
1437 omnibox_will_reorder_for_legal_default_match)); 1459 omnibox_will_reorder_for_legal_default_match));
1438 DCHECK(HasValidDefaultMatch(omnibox_will_reorder_for_legal_default_match)); 1460 DCHECK(HasValidDefaultMatch(omnibox_will_reorder_for_legal_default_match));
1439 } 1461 }
1440 1462
1463 const TemplateURL* keyword_url = providers_.GetKeywordProviderURL();
1464 if ((keyword_url != NULL) && HasKeywordDefaultMatchInKeywordMode()) {
1465 // If there is a keyword match that is allowed to be the default match,
1466 // then prohibit default provider matches from being the default match lest
1467 // such matches cause the user to break out of keyword mode.
1468 for (ACMatches::iterator it = matches_.begin(); it != matches_.end();
1469 ++it) {
1470 if (it->keyword != keyword_url->keyword())
1471 it->allowed_to_be_default_match = false;
1472 }
1473 }
1474
1441 base::TimeTicks update_starred_start_time(base::TimeTicks::Now()); 1475 base::TimeTicks update_starred_start_time(base::TimeTicks::Now());
1442 UpdateStarredStateOfMatches(); 1476 UpdateStarredStateOfMatches();
1443 UMA_HISTOGRAM_TIMES("Omnibox.SearchProvider.UpdateStarredTime", 1477 UMA_HISTOGRAM_TIMES("Omnibox.SearchProvider.UpdateStarredTime",
1444 base::TimeTicks::Now() - update_starred_start_time); 1478 base::TimeTicks::Now() - update_starred_start_time);
1445 UpdateDone(); 1479 UpdateDone();
1446 UMA_HISTOGRAM_TIMES("Omnibox.SearchProvider.UpdateMatchesTime", 1480 UMA_HISTOGRAM_TIMES("Omnibox.SearchProvider.UpdateMatchesTime",
1447 base::TimeTicks::Now() - update_matches_start_time); 1481 base::TimeTicks::Now() - update_matches_start_time);
1448 } 1482 }
1449 1483
1450 void SearchProvider::AddNavigationResultsToMatches( 1484 void SearchProvider::AddNavigationResultsToMatches(
(...skipping 381 matching lines...) Expand 10 before | Expand all | Expand 10 after
1832 // Preserve the forced query '?' prefix in |match.fill_into_edit|. 1866 // Preserve the forced query '?' prefix in |match.fill_into_edit|.
1833 // Otherwise, user edits to a suggestion would show non-Search results. 1867 // Otherwise, user edits to a suggestion would show non-Search results.
1834 if (input_.type() == AutocompleteInput::FORCED_QUERY) { 1868 if (input_.type() == AutocompleteInput::FORCED_QUERY) {
1835 match.fill_into_edit.insert(0, ASCIIToUTF16("?")); 1869 match.fill_into_edit.insert(0, ASCIIToUTF16("?"));
1836 if (inline_autocomplete_offset != string16::npos) 1870 if (inline_autocomplete_offset != string16::npos)
1837 ++inline_autocomplete_offset; 1871 ++inline_autocomplete_offset;
1838 } 1872 }
1839 if (!input_.prevent_inline_autocomplete() && 1873 if (!input_.prevent_inline_autocomplete() &&
1840 (inline_autocomplete_offset != string16::npos)) { 1874 (inline_autocomplete_offset != string16::npos)) {
1841 DCHECK(inline_autocomplete_offset <= match.fill_into_edit.length()); 1875 DCHECK(inline_autocomplete_offset <= match.fill_into_edit.length());
1842 match.allowed_to_be_default_match = true; 1876 // A navsuggestion can only be the default match when there is no
1877 // keyword provider active, lest it appear first and break the user
1878 // out of keyword mode.
1879 match.allowed_to_be_default_match =
1880 (providers_.GetKeywordProviderURL() == NULL);
1843 match.inline_autocompletion = 1881 match.inline_autocompletion =
1844 match.fill_into_edit.substr(inline_autocomplete_offset); 1882 match.fill_into_edit.substr(inline_autocomplete_offset);
1845 } 1883 }
1846 1884
1847 match.contents = net::FormatUrl(navigation.url(), languages, 1885 match.contents = net::FormatUrl(navigation.url(), languages,
1848 format_types, net::UnescapeRule::SPACES, NULL, NULL, &match_start); 1886 format_types, net::UnescapeRule::SPACES, NULL, NULL, &match_start);
1849 // If the first match in the untrimmed string was inside a scheme that we 1887 // If the first match in the untrimmed string was inside a scheme that we
1850 // trimmed, look for a subsequent match. 1888 // trimmed, look for a subsequent match.
1851 if (match_start == string16::npos) 1889 if (match_start == string16::npos)
1852 match_start = match.contents.find(input); 1890 match_start = match.contents.find(input);
(...skipping 54 matching lines...) Expand 10 before | Expand all | Expand 10 after
1907 it->set_relevance(max_query_relevance); 1945 it->set_relevance(max_query_relevance);
1908 it->set_relevance_from_server(relevance_from_server); 1946 it->set_relevance_from_server(relevance_from_server);
1909 } 1947 }
1910 } 1948 }
1911 1949
1912 void SearchProvider::UpdateDone() { 1950 void SearchProvider::UpdateDone() {
1913 // We're done when the timer isn't running, there are no suggest queries 1951 // We're done when the timer isn't running, there are no suggest queries
1914 // pending, and we're not waiting on Instant. 1952 // pending, and we're not waiting on Instant.
1915 done_ = !timer_.IsRunning() && (suggest_results_pending_ == 0); 1953 done_ = !timer_.IsRunning() && (suggest_results_pending_ == 0);
1916 } 1954 }
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698