Omnibox - Add About Flag to Reverse Title and URLs in the Dropdown

chrome/browser/autocomplete/bookmark_provider.cc

 // 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.
 
 #include "chrome/browser/autocomplete/bookmark_provider.h"
 
 #include <algorithm>
 #include <functional>
 #include <vector>
 
@@ skipping 206 matching lines @@
           match.fill_into_edit.substr(inline_autocomplete_offset);
     }
     match.allowed_to_be_default_match = match.inline_autocompletion.empty() ||
         !HistoryProvider::PreventInlineAutocomplete(input);
   }
   match.description = title;
   match.description_class =
       ClassificationsFromMatch(bookmark_match.title_match_positions,
                                match.description.size(),
                                false);
+  match.PossiblySwapContentsAndDescriptionForURLSuggestion(input);

[Peter Kasting, 2015/05/21 21:55:33]

Rather than adding these calls to each provider, can't we swap in one place (the
controller?) based on the match type?

[Mark P, 2015/05/21 23:47:36]

I thought about it quite a bit.  If we wanted to swap them in one place, the
best place to do the swap is in AutocompleteResult.  There's only one code path
there that incorporates matches from the providers (AppendMatches()).  However,
I found such a solution confusing.  This meant that in my mental model, I'd have
to remember that certain matches look different when in the combined flow
(AutocompleteController, AutocompleteResult) than returned by the providers. 
Having this strange situation--where the same data type should get interpreted
somewhat differently depending on where in the code it's been examined--I think
is too much of a design mess.  I prefer this solution, where each provider is
responsible for swapping (if appropriate) or not.

(This follows the same principle that you had me do when marking matches as
allowed to be the default match.  I wanted to do this in a central place but you
argued that it's best done within each provider.)

[Peter Kasting, 2015/05/22 00:52:42]

Huh.  Seems a lot simpler and clearer to me the other way.  But if you hate it,
then what about replacing the "contents" and "description" fields with some kind
of explicit "URL" field and a catchall "description/other text" sort of field? 
That might make the struct contents more understandable, and then we could do
the switch in the display code instead.  (Of course, that would mean we'd need a
per-display-implementation version of the solution.)
If we can mark matches as "allowed to be default" in a centralized way that's
just as readable and accurate as today's code, we should do that.  My original
argument was based on the idea that we couldn't really know this perfectly in a
central way, or at least not without writing something ugly and
incomprehensible.  If I was wrong then or now, let's change it.  Simpler code >
*

[Mark P, 2015/05/22 22:51:29]

I tried the other way.  It looks less messy than I thought.  It helps that I can
put the code in one place and write a clean, rational comment for it.  Let me
know what you think.

 
   // Summary on how a relevance score is determined for the match:
   //
   // For each match within the bookmark's title or URL (or both), calculate a
   // 'factor', sum up those factors, then use the sum to figure out a value
   // between the base score and the maximum score.
   //
   // The factor for each match is the product of:
   //
   //  1) how many characters in the bookmark's title/URL are part of this match.
@@ skipping 102 matching lines @@
        i != positions.end();
        ++i) {
     AutocompleteMatch::ACMatchClassifications new_class;
     AutocompleteMatch::ClassifyLocationInString(i->first, i->second - i->first,
         text_length, url_style, &new_class);
     classifications = AutocompleteMatch::MergeClassifications(
         classifications, new_class);
   }
   return classifications;
 }