Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2736863002:
Migrate the pref to always show the Cast icon (Closed)
DescriptionMigrate the pref to always show the Cast icon
This CL fixes the bug in which the user preference to pin the Cast toolbar icon
disappears due to the lack of migration from the old pref to the new pref.
BUG=698931
Review-Url: https://codereview.chromium.org/2736863002
Cr-Commit-Position: refs/heads/master@{#455525}
Committed: https://chromium.googlesource.com/chromium/src/+/fb4e8ac40deaad4025dcacd96c9be4fbf2b7d193
Patch Set 1 : . #
Total comments: 2
Patch Set 2 : Group ifdefs #
Messages
Total messages: 23 (17 generated)
Description was changed from ========== Migrate the pref to always show the Cast icon BUG= ========== to ========== Migrate the pref to always show the Cast icon This CL fixes the bug in which the user preference to pin the Cast toolbar icon disappears due to the lack of migration from the old pref to the new pref. BUG=698931 ==========
Patchset #1 (id:1) has been deleted
Patchset #1 (id:20001) has been deleted
Description was changed from ========== Migrate the pref to always show the Cast icon This CL fixes the bug in which the user preference to pin the Cast toolbar icon disappears due to the lack of migration from the old pref to the new pref. BUG=698931 ========== to ========== Migrate the pref to always show the Cast icon This CL fixes the bug in which the user preference to pin the Cast toolbar icon disappears due to the lack of migration from the old pref to the new pref. BUG=698931 ==========
takumif@chromium.org changed reviewers: + bauerb@chromium.org, imcheng@chromium.org
Please take a look at: imcheng@: overall bauerb@: browser_prefs.cc Thanks!
lgtm
The CQ bit was checked by takumif@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
prefs/ LGTM with a nit: https://codereview.chromium.org/2736863002/diff/40001/chrome/browser/prefs/br... File chrome/browser/prefs/browser_prefs.cc (right): https://codereview.chromium.org/2736863002/diff/40001/chrome/browser/prefs/br... chrome/browser/prefs/browser_prefs.cc:266: #endif Can you move this before the OS_CHROMEOS #ifdefs? I try to keep the conditional includes sorted, but people just add them in random places :-(
The CQ bit was checked by takumif@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2736863002/diff/40001/chrome/browser/prefs/br... File chrome/browser/prefs/browser_prefs.cc (right): https://codereview.chromium.org/2736863002/diff/40001/chrome/browser/prefs/br... chrome/browser/prefs/browser_prefs.cc:266: #endif On 2017/03/08 14:05:24, Bernhard Bauer wrote: > Can you move this before the OS_CHROMEOS #ifdefs? I try to keep the conditional > includes sorted, but people just add them in random places :-( Putting this with other ENABLE_EXTENSIONS includes. Sorry, not too sure how you want the other conditionals sorted.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by takumif@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org, imcheng@chromium.org Link to the patchset: https://codereview.chromium.org/2736863002/#ps60001 (title: "Group ifdefs")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 60001, "attempt_start_ts": 1489004039732480,
"parent_rev": "c26ce11c47e62f20c3e28af2b596cb6fa9d190c3", "commit_rev":
"fb4e8ac40deaad4025dcacd96c9be4fbf2b7d193"}
Message was sent while issue was closed.
Description was changed from ========== Migrate the pref to always show the Cast icon This CL fixes the bug in which the user preference to pin the Cast toolbar icon disappears due to the lack of migration from the old pref to the new pref. BUG=698931 ========== to ========== Migrate the pref to always show the Cast icon This CL fixes the bug in which the user preference to pin the Cast toolbar icon disappears due to the lack of migration from the old pref to the new pref. BUG=698931 Review-Url: https://codereview.chromium.org/2736863002 Cr-Commit-Position: refs/heads/master@{#455525} Committed: https://chromium.googlesource.com/chromium/src/+/fb4e8ac40deaad4025dcacd96c9b... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:60001) as https://chromium.googlesource.com/chromium/src/+/fb4e8ac40deaad4025dcacd96c9b... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
