Chromium Code Reviews Chromium Code Reviews
Issue
510533002:
Omnibox: Make URLs of Bookmarks Searchable - Try 2 (Closed)
DescriptionOmnibox: Make URLs of Bookmarks Searchable - Try 2
This is a reland of
https://codereview.chromium.org/489373005/
which was reverted because it caused failures in interactive_ui_tests on mac
on OmniboxViewTest.AcceptKeywordBySpace:
https://build.chromium.org/p/chromium.mac/builders/Mac%2010.6%20Tests%20(dbg)(1)/builds/52199
See [1].
It turns out these failures were because the test uses a bookmark with no
title and BookmarkProvider doesn't set classifications for these correctly.
(It set the title as having a classification where, without any text, it
doesn't make sense to have any classification.)
This change fixes this issue and adds a test in BookmarkProvider for it.
I will annotate the *second* patchset I upload with the differences
between the previously-reviewed changelist and this changelist.
(The first patchset has debug lines; ignore it.)
Otherwise, the changelist description remains the same, which I will paste
below [2].
I am marking this change as
TBR=sky
(because he previously reviewed this change and the new parts are
not under his purview.)
---
[1]
OmniboxViewTest.AcceptKeywordBySpace (run #1):
[ RUN ] OmniboxViewTest.AcceptKeywordBySpace
[12075:263:0822/122851:ERROR:location_bar_view_mac.mm(592)] Not implemented reached in virtual void LocationBarViewMac::EndOriginChipAnimations(bool)
[12075:263:0822/122852:FATAL:autocomplete_match.cc(516)] Check failed: classifications.empty().
0 libbase.dylib 0x1ca75c5f base::debug::StackTrace::StackTrace() + 63
1 libbase.dylib 0x1ca75cbb base::debug::StackTrace::StackTrace() + 43
2 libbase.dylib 0x1cb21862 logging::LogMessage::~LogMessage() + 82
3 libbase.dylib 0x1cb2058b logging::LogMessage::~LogMessage() + 43
4 interactive_ui_tests 0x11fb469d AutocompleteMatch::ValidateClassifications(std::basic_string\u003Cunsigned short, base::string16_char_traits, std::allocator\u003Cunsigned short> > const&, std::vector\u003CAutocompleteMatch::ACMatchClassification, std::allocator\u003CAutocompleteMatch::ACMatchClassification> > const&) const + 317
5 interactive_ui_tests 0x11fb453b AutocompleteMatch::Validate() const + 107
6 interactive_ui_tests 0x11fbbb51 AutocompleteResult::Validate() const + 113
7 interactive_ui_tests 0x0d6f5a3f AutocompleteController::UpdateResult(bool, bool) + 815
8 interactive_ui_tests 0x0d6f55b7 AutocompleteController::Start(AutocompleteInput const&) + 1799
...
[2]
This used to be controlled by a field trial created in
https://codereview.chromium.org/184663002
After evaluating this change, we've decided to launch it.
This change turns the flag on by default and removes the field trial code.
It also removes the index_urls_ parameters everywhere because those will
always be true forevermore.
I tested this interactively.
Also, all the unit tests still apparently pass. And yes some of these do
exercise this feature.
BUG=157204, 378854
Committed: https://crrev.com/56bd38e3cfb1b7d0e08e675626d572f4707fa77f
Cr-Commit-Position: refs/heads/master@{#292243}
Patch Set 1 #Patch Set 2 : revert debug lines in chrome/browser/ui/omnibox/omnibox_view_browsertest.cc #
Total comments: 7
Patch Set 3 : Peter's minor comments #Messages
Total messages: 12 (0 generated)
|
