Chromium Code Reviews Chromium Code Reviews
Issue
10381016:
Remove the "autogenerate keyword" bit on TemplateURL. (Closed)
DescriptionRemove the "autogenerate keyword" bit on TemplateURL.
This is a large change because a number of other significant pieces were either inherently tied to the above change, or too difficult to separate from it:
* Because autogeneration was really only ever supposed to be used for the prepopulated Google entry, replace it with a system that auto-regenerates keywords for TemplateURLs that (a) use {google:baseURL} and (b) have a keyword that's currently "google.<TLD>" whenever the base URL changes. This means that users who manually create such TemplateURLs will automatically get the benefit of "autogenerated" keywords.
* Pass information to the KeywordTable as bare TemplateURLData objects instead of TemplateURLs, now that the latter are not needed to perform keyword generation.
* Remove the "autogenerate_keyword" column from the KeywordTable. This also takes the opportunity to remove the already-dead "logo_id" column (which I previously asked msw to leave in in order to only have to write the migration code once). This in turn requires adding version numbers to a lot of functions so they know which column set to use, as well as writing migration code to manually generate keywords for previously-autogenerated entries.
* Migrate the "autogenerate_keyword" bit in data from prefs and sync as well. For sync this requires a variety of followon changes to send back ACTION_UPDATEs for migrated TemplateURLs and coalesce multiple SyncChanges to the same GUID.
* Move various bits of TemplateURLService::GenerateKeyword() that were only used for the "autodetected on a webpage" case to the specific code for that case, in order to make GenerateKeyword() incapable of failing. This is important for the next item.
* Remove the possibility for keywords to simply be empty. All TemplateURLs should now have a keyword, whether they were previously marked as "autogenerated" or not. While the UI already tried to guarantee this, the TemplateURL class itself and various pieces of TemplateURLService did not, or didn't deal correctly with exceptions. Enforcing this makes it much easier to reason about keywords and is important for the next item.
* Guarantee that all keywords are unique, with one exception noted below. This allows callers to reliably refer to TemplateURLs by keyword; a future change will make AutocompleteMatch do precisely this. It also prevents weird edge cases in the UI and sync.
* Exception: explicitly allow extension keywords to overlap with each other and with one non-extension TemplateURL. Previously, the behavior was somewhat random and buggy when we added and removed extensions that defined keywords, especially if we also tried to add/edit/remove keywords from the settings UI. We now define an explicit precedence order: non-replaceable TemplateURL > extension-provided TemplateURLs > replaceable TemplateURL; within the extensions section, the most recently-added extension wins. As we add and remove keywords, the current "TemplateURL for keyword" is always kept up-to-date according to this precedence order (so e.g. removing a later extension will "expose" an earlier one).
BUG=none
TEST=Adding extensions that specify omnibox keywords which conflict with local keywords should result in predictable behavior as described above; removing the extensions should restore the prior behavior.
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=135775
Patch Set 1 #Patch Set 2 : #
Total comments: 16
Patch Set 3 : #Patch Set 4 : #
Total comments: 4
Patch Set 5 : #
Total comments: 32
Patch Set 6 : #
Total comments: 8
Messages
Total messages: 14 (0 generated)
|
