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

Issue 2721993002: Omnibox - Make Clipboard Provider Enabled via an Experimental Feature (Closed)

Created:
3 years, 9 months ago by Mark P
Modified:
3 years, 9 months ago
CC:
chromium-reviews, dbeam+watch-options_chromium.org, jdonnelly+watch_chromium.org, michaelpg+watch-options_chromium.org, Matt Giuca, tfarina
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Omnibox - Make Clipboard Provider Enabled via an Experimental Feature In the process, revise the list of default providers from a built-in const to something that's determined dynamically when the omnibox system is created. The only interesting part of this changelist is in components/omnibox/browser/autocomplete_classifier.{h,cc} Pretty much everything else is downstream changes due to changing an interface. TBR=mariakhomenko,rohitrao mariakhomenko for trivial change in chrome/browser/android/omnibox/ rohitrao for trivial change in ios/chrome/browser/autocomplete/ BUG=642114 Review-Url: https://codereview.chromium.org/2721993002 Cr-Commit-Position: refs/heads/master@{#454035} Committed: https://chromium.googlesource.com/chromium/src/+/612b3dc0d5ab6aaf839cc33e71d5e4f1e49e2bf8

Patch Set 1 #

Patch Set 2 : remove unnecessary variable #

Total comments: 1

Patch Set 3 : reorder function #

Patch Set 4 : fix ios #

Total comments: 2

Patch Set 5 : fix vr #

Patch Set 6 : rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+55 lines, -43 lines) Patch
M chrome/browser/android/omnibox/autocomplete_controller_android.cc View 2 chunks +1 line, -4 lines 0 comments Download
M chrome/browser/android/vr_shell/vr_omnibox.cc View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/autocomplete/autocomplete_classifier_factory.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/app_list/search/omnibox_provider.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/views/frame/test_with_browser_view.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/webui/omnibox/omnibox_page_handler.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/webui/options/home_page_overlay_handler.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/webui/options/startup_pages_handler.cc View 1 chunk +1 line, -1 line 0 comments Download
M components/omnibox/browser/autocomplete_classifier.h View 1 2 2 chunks +5 lines, -5 lines 0 comments Download
M components/omnibox/browser/autocomplete_classifier.cc View 1 2 2 chunks +27 lines, -23 lines 0 comments Download
M components/omnibox/browser/omnibox_controller.cc View 1 chunk +1 line, -1 line 0 comments Download
M components/omnibox/browser/omnibox_edit_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M components/omnibox/browser/omnibox_field_trial.h View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M components/omnibox/browser/omnibox_field_trial.cc View 1 2 3 4 5 1 chunk +11 lines, -0 lines 0 comments Download
M ios/chrome/browser/autocomplete/autocomplete_classifier_factory.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 31 (25 generated)
Mark P
Hey Peter, This is not urgent. Please do at your leisure. It should be straightforward. ...
3 years, 9 months ago (2017-02-28 23:06:02 UTC) #3
Peter Kasting
LGTM
3 years, 9 months ago (2017-03-01 02:47:14 UTC) #4
Justin Donnelly
https://codereview.chromium.org/2721993002/diff/60001/components/omnibox/browser/autocomplete_classifier.cc File components/omnibox/browser/autocomplete_classifier.cc (right): https://codereview.chromium.org/2721993002/diff/60001/components/omnibox/browser/autocomplete_classifier.cc#newcode52 components/omnibox/browser/autocomplete_classifier.cc:52: (base::FeatureList::IsEnabled(omnibox::kEnableClipboardProvider) Shouldn't this still be default enabled on iOS?
3 years, 9 months ago (2017-03-01 14:51:36 UTC) #16
Mark P
https://codereview.chromium.org/2721993002/diff/60001/components/omnibox/browser/autocomplete_classifier.cc File components/omnibox/browser/autocomplete_classifier.cc (right): https://codereview.chromium.org/2721993002/diff/60001/components/omnibox/browser/autocomplete_classifier.cc#newcode52 components/omnibox/browser/autocomplete_classifier.cc:52: (base::FeatureList::IsEnabled(omnibox::kEnableClipboardProvider) On 2017/03/01 14:51:36, Justin Donnelly wrote: > Shouldn't ...
3 years, 9 months ago (2017-03-01 18:20:31 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2721993002/100001
3 years, 9 months ago (2017-03-01 21:17:33 UTC) #28
commit-bot: I haz the power
3 years, 9 months ago (2017-03-01 21:24:43 UTC) #31
Message was sent while issue was closed.
Committed patchset #6 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/612b3dc0d5ab6aaf839cc33e71d5...

Powered by Google App Engine
This is Rietveld 408576698