Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(42)

Issue 1598553003: Implement the Windows desktop search redirection feature. (Closed)

Created:
2 years, 10 months ago by fdoray
Modified:
2 years, 9 months ago
CC:
chromium-reviews, blundell+watchlist_chromium.org, sdefresne+watchlist_chromium.org, droger+watchlist_chromium.org, asvitkine+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Implement the Windows desktop search redirection feature. With this CL, Windows desktop searches are redirected to the default search engine when the DesktopSearchRedirection feature is enabled. The first time that a desktop search is redirected, an infobar is shown to inform the user of the redirection. BUG=539963 Committed: https://crrev.com/2e7c477ce837b177ed32eaa7a2fd1e7911b67a87 Cr-Commit-Position: refs/heads/master@{#373049}

Patch Set 1 #

Total comments: 8

Patch Set 2 : don't assume that a Browser exists + fix nits #

Total comments: 49

Patch Set 3 : address pkasting comments #8 #

Patch Set 4 : self-review #

Total comments: 24

Patch Set 5 : nits + rebase #

Patch Set 6 : rebase #

Patch Set 7 : rebase #

Patch Set 8 : fix build errors due to prefs being moved in components #

Unified diffs Side-by-side diffs Delta from patch set Stats (+660 lines, -348 lines) Patch
M chrome/app/generated_resources.grd View 1 2 3 4 5 6 1 chunk +4 lines, -4 lines 0 comments Download
M chrome/browser/about_flags.cc View 1 2 3 4 5 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/search_engines/template_url_service_factory.cc View 1 2 2 chunks +2 lines, -7 lines 0 comments Download
M chrome/browser/ui/startup/startup_browser_creator.h View 1 2 3 4 5 4 chunks +15 lines, -5 lines 0 comments Download
M chrome/browser/ui/startup/startup_browser_creator.cc View 1 2 3 4 5 5 chunks +18 lines, -22 lines 0 comments Download
M chrome/browser/ui/startup/startup_browser_creator_impl.cc View 1 2 3 4 5 2 chunks +19 lines, -0 lines 0 comments Download
D chrome/browser/ui/startup/startup_browser_creator_win_unittest.cc View 1 chunk +0 lines, -90 lines 0 comments Download
M chrome/chrome_tests_unit.gypi View 1 2 3 4 5 6 1 chunk +0 lines, -1 line 0 comments Download
M components/components_strings.grd View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download
M components/components_tests.gyp View 1 2 3 4 5 6 1 chunk +1 line, -1 line 0 comments Download
M components/infobars/core/infobar_delegate.h View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M components/search_engines.gypi View 1 2 3 4 5 6 7 2 chunks +6 lines, -2 lines 0 comments Download
M components/search_engines/BUILD.gn View 1 2 3 4 5 6 7 4 chunks +8 lines, -3 lines 0 comments Download
M components/search_engines/DEPS View 1 2 3 4 5 6 7 1 chunk +3 lines, -0 lines 0 comments Download
A components/search_engines/desktop_search_redirection_infobar_delegate.h View 1 2 3 4 1 chunk +56 lines, -0 lines 0 comments Download
A components/search_engines/desktop_search_redirection_infobar_delegate.cc View 1 2 3 4 1 chunk +115 lines, -0 lines 0 comments Download
A components/search_engines/desktop_search_utils.h View 1 2 3 4 1 chunk +45 lines, -0 lines 0 comments Download
A components/search_engines/desktop_search_utils.cc View 1 2 3 4 5 6 7 1 chunk +138 lines, -0 lines 0 comments Download
A components/search_engines/desktop_search_utils_unittest.cc View 1 2 3 4 5 6 7 1 chunk +151 lines, -0 lines 0 comments Download
D components/search_engines/desktop_search_win.h View 1 chunk +0 lines, -44 lines 0 comments Download
D components/search_engines/desktop_search_win.cc View 1 chunk +0 lines, -73 lines 0 comments Download
D components/search_engines/desktop_search_win_unittest.cc View 1 chunk +0 lines, -47 lines 0 comments Download
D components/search_engines/detect_desktop_search_win_unittest.cc View 1 2 1 chunk +0 lines, -47 lines 0 comments Download
A components/search_engines_strings.grdp View 1 2 1 chunk +10 lines, -0 lines 0 comments Download
M tools/metrics/actions/actions.xml View 1 2 3 4 5 6 1 chunk +34 lines, -0 lines 0 comments Download
M tools/metrics/histograms/histograms.xml View 1 2 3 4 5 6 3 chunks +31 lines, -0 lines 0 comments Download

Messages

Total messages: 31 (11 generated)
fdoray
pkasting@: Can you review this CL? Thanks! asvitkine@: Can you review tools/metrics/*? Note: The UI ...
2 years, 10 months ago (2016-01-16 18:07:35 UTC) #2
fdoray
+asvitkine@ pkasting@: Can you review this CL? Thanks! asvitkine@: Can you review tools/metrics/*? Note: The ...
2 years, 10 months ago (2016-01-16 18:09:10 UTC) #4
Alexei Svitkine (slow)
https://codereview.chromium.org/1598553003/diff/1/chrome/browser/ui/startup/startup_browser_creator.cc File chrome/browser/ui/startup/startup_browser_creator.cc (right): https://codereview.chromium.org/1598553003/diff/1/chrome/browser/ui/startup/startup_browser_creator.cc#newcode407 chrome/browser/ui/startup/startup_browser_creator.cc:407: DCHECK(browser); What guarantees this is true? I see an ...
2 years, 10 months ago (2016-01-18 16:08:52 UTC) #5
fdoray
asvitkine@: Can you take another look? pkasting@: Can you review this CL? https://codereview.chromium.org/1598553003/diff/1/chrome/browser/ui/startup/startup_browser_creator.cc File chrome/browser/ui/startup/startup_browser_creator.cc ...
2 years, 10 months ago (2016-01-18 18:51:45 UTC) #6
Alexei Svitkine (slow)
metrics lgtm - I only did a minimal review of the rest of the CL, ...
2 years, 10 months ago (2016-01-19 19:55:11 UTC) #7
Peter Kasting
https://codereview.chromium.org/1598553003/diff/20001/chrome/app/generated_resources.grd File chrome/app/generated_resources.grd (right): https://codereview.chromium.org/1598553003/diff/20001/chrome/app/generated_resources.grd#newcode5961 chrome/app/generated_resources.grd:5961: + Enable the experimental implementation of the "Cache-Control: stale-while-revalidate" ...
2 years, 10 months ago (2016-01-20 03:03:02 UTC) #8
fdoray
pkasting@: Can you take another look? caitkp@: Can you review components/components_strings.grd, components/components_tests.gyp, components/search_engines_strings.grdp? ainslie@: Can ...
2 years, 10 months ago (2016-01-21 21:02:34 UTC) #11
Peter Kasting
LGTM. I'm definitely interested in hearing rkaplow/asvitkine describe the reason for using both actions and ...
2 years, 10 months ago (2016-01-22 00:38:57 UTC) #12
Alexei Svitkine (slow)
Re: The reason for both actions & histograms: Histograms are the best way to look ...
2 years, 10 months ago (2016-01-22 15:54:05 UTC) #13
Cait (Slow)
components/components_strings.grd, components/components_tests.gyp, components/search_engines_strings.grdp LGTM
2 years, 10 months ago (2016-01-22 16:03:48 UTC) #14
Peter Kasting
On 2016/01/22 15:54:05, Alexei Svitkine (very slow) wrote: > Re: The reason for both actions ...
2 years, 10 months ago (2016-01-22 22:41:05 UTC) #15
fdoray
On 2016/01/22 22:41:05, Peter Kasting wrote: > On 2016/01/22 15:54:05, Alexei Svitkine (very slow) wrote: ...
2 years, 9 months ago (2016-02-01 15:15:32 UTC) #16
fdoray
asvitkine@: Can you reply to pkasting's comment #15? I think that the Search.DesktopSearch.RedirectionInfobarCloseAction histogram isn't ...
2 years, 9 months ago (2016-02-01 15:15:54 UTC) #17
Alexei Svitkine (slow)
On Mon, Feb 1, 2016 at 10:15 AM, <fdoray@chromium.org> wrote: > > asvitkine@: Can you ...
2 years, 9 months ago (2016-02-01 16:19:38 UTC) #18
fdoray
> It's up to you. If you just use actions, you lose a lot of ...
2 years, 9 months ago (2016-02-02 16:36:18 UTC) #19
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1598553003/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1598553003/120001
2 years, 9 months ago (2016-02-02 16:39:46 UTC) #22
commit-bot: I haz the power
Try jobs failed on following builders: ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator_ninja/builds/125142) mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED, ...
2 years, 9 months ago (2016-02-02 16:50:39 UTC) #24
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1598553003/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1598553003/140001
2 years, 9 months ago (2016-02-02 20:32:39 UTC) #27
commit-bot: I haz the power
Committed patchset #8 (id:140001)
2 years, 9 months ago (2016-02-02 21:56:50 UTC) #29
commit-bot: I haz the power
2 years, 9 months ago (2016-02-02 21:58:24 UTC) #31
Message was sent while issue was closed.
Patchset 8 (id:??) landed as
https://crrev.com/2e7c477ce837b177ed32eaa7a2fd1e7911b67a87
Cr-Commit-Position: refs/heads/master@{#373049}

Powered by Google App Engine
This is Rietveld 408576698