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

Issue 9705021: Clean up TemplateURL prepopulate data: (Closed)

Created:
8 years, 9 months ago by Peter Kasting
Modified:
8 years, 9 months ago
Reviewers:
sky
CC:
chromium-reviews
Visibility:
Public.

Description

Clean up TemplateURL prepopulate data: * Replace a pair of overlapping functions in the API with two distinct accessors for relevant pieces of information about prepopulated URLs. One key side effect here is that we no longer return TemplateURL*s from these, which will make the upcoming refactoring changes slightly cleaner. * Make the implementation of these a bit more robust w.r.t. determining if a provided URL corresponds to Google. This will also be important later as we'll want to be able to match a hand-coded "google.co.uk" entry against an auto-generated "google.de" entry (or similar). * Remove a bunch of string conversions, as well as a lot of passing of raw C-style string pointers. * Remove using statements. * Remove two obscure engines which have the same hostname as another, different engine. These cause problems when we ask for the name or type of an engine because the answer we get back is basically indeterminate. I don't think either of these is very important (they lived at slots 4 and 5 in their respective countries) so just gut them. BUG=none TEST=none Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=126746

Patch Set 1 #

Patch Set 2 : #

Total comments: 2

Patch Set 3 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+503 lines, -603 lines) Patch
M chrome/app/generated_resources.grd View 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/browser/google/google_util.h View 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/browser/google/google_util.cc View 2 chunks +14 lines, -26 lines 0 comments Download
M chrome/browser/protector/histograms.cc View 1 2 2 chunks +4 lines, -11 lines 0 comments Download
M chrome/browser/search_engines/template_url_prepopulate_data.h View 2 chunks +16 lines, -9 lines 0 comments Download
M chrome/browser/search_engines/template_url_prepopulate_data.cc View 1 173 chunks +420 lines, -486 lines 0 comments Download
M chrome/browser/search_engines/template_url_prepopulate_data_unittest.cc View 8 chunks +42 lines, -71 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
Peter Kasting
8 years, 9 months ago (2012-03-14 17:49:34 UTC) #1
sky
https://chromiumcodereview.appspot.com/9705021/diff/3005/chrome/browser/protector/histograms.cc File chrome/browser/protector/histograms.cc (right): https://chromiumcodereview.appspot.com/9705021/diff/3005/chrome/browser/protector/histograms.cc#newcode45 chrome/browser/protector/histograms.cc:45: return t_url ? You sure you don't need the ...
8 years, 9 months ago (2012-03-14 21:18:05 UTC) #2
Peter Kasting
https://chromiumcodereview.appspot.com/9705021/diff/3005/chrome/browser/protector/histograms.cc File chrome/browser/protector/histograms.cc (right): https://chromiumcodereview.appspot.com/9705021/diff/3005/chrome/browser/protector/histograms.cc#newcode45 chrome/browser/protector/histograms.cc:45: return t_url ? On 2012/03/14 21:18:05, sky wrote: > ...
8 years, 9 months ago (2012-03-14 21:21:34 UTC) #3
sky
8 years, 9 months ago (2012-03-14 21:30:28 UTC) #4
LGTM

Powered by Google App Engine
This is Rietveld 408576698