Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1748773002:
Simplify the default browser infobar. (Closed)
|
Created:
4 years, 9 months ago by grt (UTC plus 2) Modified:
4 years, 8 months ago CC:
chromium-reviews, tim+watch_chromium.org, dbeam+watch-options_chromium.org, michaelpg+watch-options_chromium.org, zea+watch_chromium.org, michaelpg+watch-md-settings_chromium.org, maxbogue+watch_chromium.org, plaree+watch_chromium.org, dbeam+watch-settings_chromium.org, stevenjb+watch-md-settings_chromium.org, Pam (message me for reviews) Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionSimplify the default browser infobar.
- Changes the names of the histograms and user actions so that they
reflect the action the user made rather than the behavior of the
buttons.
- Cleans up the InfoBarDelegate implementation a bit.
- Introduces a Win10+ string for the OK/Accept button's label (currently
the same as the original label) to enable experimentation.
- Adds support for refreshing the infobar after a configurable number of
days has elapsed.
BUG=589128
Committed: https://crrev.com/a872c10d41b21c4e46347c4a7866e141013c59e5
Cr-Commit-Position: refs/heads/master@{#386726}
Patch Set 1 #
Total comments: 2
Patch Set 2 : handle version-specific suppression before checking the feature #Patch Set 3 : sync to position 379825 #
Total comments: 17
Patch Set 4 : msw comments #Patch Set 5 : new approach without ditching the cancel button #
Total comments: 7
Patch Set 6 : fieldtrial testing #
Total comments: 2
Messages
Total messages: 65 (30 generated)
The CQ bit was checked by grt@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1748773002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1748773002/1
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: chromeos_amd64-generic_chromium_compile_only_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...) chromeos_daisy_chromium_compile_only_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...) chromeos_x86-generic_chromium_compile_only_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...) linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by grt@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1748773002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1748773002/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: chromeos_amd64-generic_chromium_compile_only_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...) chromeos_daisy_chromium_compile_only_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...) chromeos_x86-generic_chromium_compile_only_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)
The CQ bit was checked by grt@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1748773002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1748773002/40001
Description was changed from ========== Simplify the default browser infobar. BUG=589128 ========== to ========== Simplify the default browser infobar. This change removes the CANCEL button and adds support for refreshing the infobar after a configurable number of days has elapsed. It also adds helper functions for manipulating the prefs controlling the infobar for use by other code in Chrome. BUG=589128 ==========
Patchset #1 (id:1) has been deleted
Patchset #1 (id:20001) has been deleted
Patchset #1 (id:40001) has been deleted
Patchset #1 (id:60001) has been deleted
The CQ bit was checked by grt@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1748773002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1748773002/80001
grt@chromium.org changed reviewers: + msw@chromium.org, rkaplow@chromium.org
grt@chromium.org changed reviewers: + pam@chromium.org, zea@chromium.org
Reviews please: rkaplow: use of feature list + variations in default_browser_prompt.cc plus the change to actions.xml msw: chrome/browser/ui/* pam: chrome/browser/prefs/* zea: chrome/browser/sync/test/integration/two_client_preferences_sync_test.cc Thank you!
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
lgtm feature/actions lgtm https://codereview.chromium.org/1748773002/diff/80001/chrome/browser/ui/start... File chrome/browser/ui/startup/default_browser_prompt.cc (right): https://codereview.chromium.org/1748773002/diff/80001/chrome/browser/ui/start... chrome/browser/ui/startup/default_browser_prompt.cc:292: const std::string disable_version_string = don't think this is a big deal but should this check be above the above check? Since this way we'll get people who will be marked as enabled in the experiment who will return false here because of something else - will make the data slightly less clean
https://codereview.chromium.org/1748773002/diff/80001/chrome/browser/ui/start... File chrome/browser/ui/startup/default_browser_prompt.cc (right): https://codereview.chromium.org/1748773002/diff/80001/chrome/browser/ui/start... chrome/browser/ui/startup/default_browser_prompt.cc:292: const std::string disable_version_string = On 2016/03/07 16:10:40, rkaplow wrote: > don't think this is a big deal but should this check be above the above check? > Since this way we'll get people who will be marked as enabled in the experiment > who will return false here because of something else - will make the data > slightly less clean moved check above the above check. :-) thanks!
The CQ bit was checked by grt@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1748773002/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1748773002/120001
sync LGTM
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
I'd like to see PM/UX feedback on your proposal (on the bug). https://codereview.chromium.org/1748773002/diff/120001/chrome/browser/ui/brow... File chrome/browser/ui/browser_ui_prefs.cc (right): https://codereview.chromium.org/1748773002/diff/120001/chrome/browser/ui/brow... chrome/browser/ui/browser_ui_prefs.cc:74: registry->RegisterInt64Pref(prefs::kDefaultBrowserLastDismissed, 0L); nit: '0' here or '0L' above (w/RegisterInt64Pref) for consistency. https://codereview.chromium.org/1748773002/diff/120001/chrome/browser/ui/star... File chrome/browser/ui/startup/default_browser_prompt.cc (left): https://codereview.chromium.org/1748773002/diff/120001/chrome/browser/ui/star... chrome/browser/ui/startup/default_browser_prompt.cc:189: bool DefaultBrowserInfoBarDelegate::Cancel() { I think it's a little odd that we don't offer 'No, thanks' or some other good faith way to decline for now (besides the infobar's dismiss button), can you confirm that's the intent of UX (or okay with them)? https://codereview.chromium.org/1748773002/diff/120001/chrome/browser/ui/star... File chrome/browser/ui/startup/default_browser_prompt.cc (right): https://codereview.chromium.org/1748773002/diff/120001/chrome/browser/ui/star... chrome/browser/ui/startup/default_browser_prompt.cc:46: const base::Feature kInfobarRefreshFeature{"InfobarRefresh", nit: add a comment here? what does infobar refresh mean? https://codereview.chromium.org/1748773002/diff/120001/chrome/browser/ui/star... chrome/browser/ui/startup/default_browser_prompt.cc:64: // Value 1 is deprecated; do not re-use. nit: leave context with a comment (eg. "Value 1 (DONT_ASK_AGAIN) is deprecated...") or a renamed enum val (eg. DONT_ASK_AGAIN_DEPRECATED)? https://codereview.chromium.org/1748773002/diff/120001/chrome/browser/ui/star... chrome/browser/ui/startup/default_browser_prompt.cc:75: // InfoBarDelegate: I'm not sure why you pulled out some of the InfoBarDelegate functions here, but not the others (GetIdentifier, GetIconID, GetVectorIconID). Be consistent here. https://codereview.chromium.org/1748773002/diff/120001/chrome/browser/ui/star... chrome/browser/ui/startup/default_browser_prompt.cc:244: "DefaultBrowserInfobarRefresh", "PeriodDays"), Are these usually string literals scattered in code? Why not use constants? https://codereview.chromium.org/1748773002/diff/120001/chrome/browser/ui/star... chrome/browser/ui/startup/default_browser_prompt.cc:307: DCHECK(profile); nit: dcheck before dereference isn't really needed (below too) https://codereview.chromium.org/1748773002/diff/120001/chrome/browser/ui/star... File chrome/browser/ui/startup/default_browser_prompt.h (right): https://codereview.chromium.org/1748773002/diff/120001/chrome/browser/ui/star... chrome/browser/ui/startup/default_browser_prompt.h:18: // Marks the default browser prompt as having been dismissed. nit: maybe order these after the Show* function pair?
Thanks for the review. I've asked UX to confirm on the bug (which was filed by PM, FWIW). https://codereview.chromium.org/1748773002/diff/120001/chrome/browser/ui/brow... File chrome/browser/ui/browser_ui_prefs.cc (right): https://codereview.chromium.org/1748773002/diff/120001/chrome/browser/ui/brow... chrome/browser/ui/browser_ui_prefs.cc:74: registry->RegisterInt64Pref(prefs::kDefaultBrowserLastDismissed, 0L); On 2016/03/08 18:01:02, msw (out wed-fri) wrote: > nit: '0' here or '0L' above (w/RegisterInt64Pref) for consistency. Done. https://codereview.chromium.org/1748773002/diff/120001/chrome/browser/ui/star... File chrome/browser/ui/startup/default_browser_prompt.cc (left): https://codereview.chromium.org/1748773002/diff/120001/chrome/browser/ui/star... chrome/browser/ui/startup/default_browser_prompt.cc:189: bool DefaultBrowserInfoBarDelegate::Cancel() { On 2016/03/08 18:01:02, msw (out wed-fri) wrote: > I think it's a little odd that we don't offer 'No, thanks' or some other good > faith way to decline for now (besides the infobar's dismiss button), can you > confirm that's the intent of UX (or okay with them)? I discussed this with LaForge, who tells me that this is consistent with other UX. I will ask for confirmation on the bug. https://codereview.chromium.org/1748773002/diff/120001/chrome/browser/ui/star... File chrome/browser/ui/startup/default_browser_prompt.cc (right): https://codereview.chromium.org/1748773002/diff/120001/chrome/browser/ui/star... chrome/browser/ui/startup/default_browser_prompt.cc:46: const base::Feature kInfobarRefreshFeature{"InfobarRefresh", On 2016/03/08 18:01:02, msw (out wed-fri) wrote: > nit: add a comment here? what does infobar refresh mean? Done. https://codereview.chromium.org/1748773002/diff/120001/chrome/browser/ui/star... chrome/browser/ui/startup/default_browser_prompt.cc:64: // Value 1 is deprecated; do not re-use. On 2016/03/08 18:01:02, msw (out wed-fri) wrote: > nit: leave context with a comment (eg. "Value 1 (DONT_ASK_AGAIN) is > deprecated...") or a renamed enum val (eg. DONT_ASK_AGAIN_DEPRECATED)? Done. https://codereview.chromium.org/1748773002/diff/120001/chrome/browser/ui/star... chrome/browser/ui/startup/default_browser_prompt.cc:75: // InfoBarDelegate: On 2016/03/08 18:01:02, msw (out wed-fri) wrote: > I'm not sure why you pulled out some of the InfoBarDelegate functions here, but > not the others (GetIdentifier, GetIconID, GetVectorIconID). Be consistent here. Ah, an oversight. I thought I'd gone through each one, but must have gotten off the rails. Thanks for catching this. Done. https://codereview.chromium.org/1748773002/diff/120001/chrome/browser/ui/star... chrome/browser/ui/startup/default_browser_prompt.cc:244: "DefaultBrowserInfobarRefresh", "PeriodDays"), On 2016/03/08 18:01:02, msw (out wed-fri) wrote: > Are these usually string literals scattered in code? Why not use constants? I see both patterns used in the code. My inclination is to keep it as-is until there's more than one occurrence. In this case, neither literal appears more than once. I'd be glad to change it if you feel strongly, in which case would you prefer having the constants defined at the top of the unnamed namespace, or at function scope? https://codereview.chromium.org/1748773002/diff/120001/chrome/browser/ui/star... chrome/browser/ui/startup/default_browser_prompt.cc:307: DCHECK(profile); On 2016/03/08 18:01:02, msw (out wed-fri) wrote: > nit: dcheck before dereference isn't really needed (below too) Done. https://codereview.chromium.org/1748773002/diff/120001/chrome/browser/ui/star... File chrome/browser/ui/startup/default_browser_prompt.h (right): https://codereview.chromium.org/1748773002/diff/120001/chrome/browser/ui/star... chrome/browser/ui/startup/default_browser_prompt.h:18: // Marks the default browser prompt as having been dismissed. On 2016/03/08 18:01:03, msw (out wed-fri) wrote: > nit: maybe order these after the Show* function pair? They relate only to the ShowDefaultBrowserPrompt, so I thought it made sense to put them here. ShowFirstRunDefaultBrowserPrompt is for a specific first-run experience that is unaffected by Mark/Reset (by its nature of being first-run only).
lgtm https://codereview.chromium.org/1748773002/diff/120001/chrome/browser/ui/star... File chrome/browser/ui/startup/default_browser_prompt.cc (right): https://codereview.chromium.org/1748773002/diff/120001/chrome/browser/ui/star... chrome/browser/ui/startup/default_browser_prompt.cc:244: "DefaultBrowserInfobarRefresh", "PeriodDays"), On 2016/03/11 16:04:31, grt (very slow) wrote: > On 2016/03/08 18:01:02, msw (out wed-fri) wrote: > > Are these usually string literals scattered in code? Why not use constants? > > I see both patterns used in the code. My inclination is to keep it as-is until > there's more than one occurrence. In this case, neither literal appears more > than once. I'd be glad to change it if you feel strongly, in which case would > you prefer having the constants defined at the top of the unnamed namespace, or > at function scope? Acknowledged. It's fine as-is.
Pinging Pam for owners review in chrome/browser/prefs/*. Thanks.
Description was changed from ========== Simplify the default browser infobar. This change removes the CANCEL button and adds support for refreshing the infobar after a configurable number of days has elapsed. It also adds helper functions for manipulating the prefs controlling the infobar for use by other code in Chrome. BUG=589128 ========== to ========== Simplify the default browser infobar. - Changes the names of the histograms and user actions so that they reflect the action the user made rather than the behavior of the buttons. - Cleans up the InfoBarDelegate implementation a bit. - Introduces a Win10+ string for the OK/Accept button's label (currently the same as the original label) to enable experimentation. - Adds support for refreshing the infobar after a configurable number of days has elapsed. BUG=589128 ==========
The CQ bit was checked by grt@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1748773002/160001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1748773002/160001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by grt@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1748773002/180001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1748773002/180001
Patchset #5 (id:160001) has been deleted
Sending back out for final review. Please take another look, as the cancel button is no longer being removed. rkaplow: use of variation params in default_browser_prompt.cc plus the changes to actions.xml and histograms.xml msw: chrome/browser/ui/* pam: chrome/browser/prefs/* Thanks.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/1748773002/diff/180001/chrome/app/generated_r... File chrome/app/generated_resources.grd (right): https://codereview.chromium.org/1748773002/diff/180001/chrome/app/generated_r... chrome/app/generated_resources.grd:11096: + Set as default Do we plan to actually include a unique string here? Why not now? https://codereview.chromium.org/1748773002/diff/180001/chrome/browser/ui/star... File chrome/browser/ui/startup/default_browser_prompt.cc (right): https://codereview.chromium.org/1748773002/diff/180001/chrome/browser/ui/star... chrome/browser/ui/startup/default_browser_prompt.cc:152: void DefaultBrowserInfoBarDelegate::InfoBarDismissed() { We don't update kDefaultBrowserLastDeclined (on clicking (x) to close?), so this will now repeatedly show the infobar (on startup?) until someone actually declines; is that consistent with the existing behavior? Is it what we want?
https://codereview.chromium.org/1748773002/diff/180001/chrome/browser/ui/star... File chrome/browser/ui/startup/default_browser_prompt.cc (right): https://codereview.chromium.org/1748773002/diff/180001/chrome/browser/ui/star... chrome/browser/ui/startup/default_browser_prompt.cc:61: ACCEPT_INFO_BAR = 0, since you're renaming here, I would suggest rename the histogram enum values as well
Thanks for the comments, folks. Back to you. https://codereview.chromium.org/1748773002/diff/180001/chrome/app/generated_r... File chrome/app/generated_resources.grd (right): https://codereview.chromium.org/1748773002/diff/180001/chrome/app/generated_r... chrome/app/generated_resources.grd:11096: + Set as default On 2016/04/11 21:54:27, msw wrote: > Do we plan to actually include a unique string here? Why not now? Not yet. We will use a field trial to supply a Win10-specific string. Once we know what string works well for users, we will remove the field trial and land the specific string here. https://codereview.chromium.org/1748773002/diff/180001/chrome/browser/ui/star... File chrome/browser/ui/startup/default_browser_prompt.cc (right): https://codereview.chromium.org/1748773002/diff/180001/chrome/browser/ui/star... chrome/browser/ui/startup/default_browser_prompt.cc:61: ACCEPT_INFO_BAR = 0, On 2016/04/11 22:16:29, rkaplow wrote: > since you're renaming here, I would suggest rename the histogram enum values as > well Do you mean the values for the DefaultBrowserInfoBarUserInteraction enum in histograms.xml? I'm doing that: https://codereview.chromium.org/1748773002/diff/200001/tools/metrics/histogra.... Did I miss a spot? https://codereview.chromium.org/1748773002/diff/180001/chrome/browser/ui/star... chrome/browser/ui/startup/default_browser_prompt.cc:152: void DefaultBrowserInfoBarDelegate::InfoBarDismissed() { On 2016/04/11 21:54:27, msw wrote: > We don't update kDefaultBrowserLastDeclined (on clicking (x) to close?), so this > will now repeatedly show the infobar (on startup?) until someone actually > declines; is that consistent with the existing behavior? Yes, this is identical to the current behavior. 'x' dismisses it for the current session, but does not do so for future sessions. > Is it what we want? Yup. :-)
On 2016/04/12 13:14:45, grt (very slow) wrote: > Thanks for the comments, folks. Back to you. > > https://codereview.chromium.org/1748773002/diff/180001/chrome/app/generated_r... > File chrome/app/generated_resources.grd (right): > > https://codereview.chromium.org/1748773002/diff/180001/chrome/app/generated_r... > chrome/app/generated_resources.grd:11096: + Set as default > On 2016/04/11 21:54:27, msw wrote: > > Do we plan to actually include a unique string here? Why not now? > > Not yet. We will use a field trial to supply a Win10-specific string. Once we > know what string works well for users, we will remove the field trial and land > the specific string here. > > https://codereview.chromium.org/1748773002/diff/180001/chrome/browser/ui/star... > File chrome/browser/ui/startup/default_browser_prompt.cc (right): > > https://codereview.chromium.org/1748773002/diff/180001/chrome/browser/ui/star... > chrome/browser/ui/startup/default_browser_prompt.cc:61: ACCEPT_INFO_BAR = 0, > On 2016/04/11 22:16:29, rkaplow wrote: > > since you're renaming here, I would suggest rename the histogram enum values > as > > well > > Do you mean the values for the DefaultBrowserInfoBarUserInteraction enum in > histograms.xml? I'm doing that: > https://codereview.chromium.org/1748773002/diff/200001/tools/metrics/histogra.... > Did I miss a spot? No - I must have been silly. Yes, that was what I meant - not sure how I missed it.
lgtm
The CQ bit was checked by grt@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1748773002/200001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1748773002/200001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm https://codereview.chromium.org/1748773002/diff/180001/chrome/app/generated_r... File chrome/app/generated_resources.grd (right): https://codereview.chromium.org/1748773002/diff/180001/chrome/app/generated_r... chrome/app/generated_resources.grd:11096: + Set as default On 2016/04/12 13:14:45, grt (very slow) wrote: > On 2016/04/11 21:54:27, msw wrote: > > Do we plan to actually include a unique string here? Why not now? > > Not yet. We will use a field trial to supply a Win10-specific string. Once we > know what string works well for users, we will remove the field trial and land > the specific string here. Hopefully we'll do that soon, otherwise it'd be best to leave those changes for a separate CL.
gab@chromium.org changed reviewers: + gab@chromium.org
prefs lgtm w/ question https://codereview.chromium.org/1748773002/diff/200001/chrome/browser/prefs/b... File chrome/browser/prefs/browser_prefs.cc (right): https://codereview.chromium.org/1748773002/diff/200001/chrome/browser/prefs/b... chrome/browser/prefs/browser_prefs.cc:689: metrics_service When would there ever not be a g_browser_process->metrics_service()? Tests?
grt@chromium.org changed reviewers: - pam@chromium.org
Thanks for the quick review, Gab! https://codereview.chromium.org/1748773002/diff/200001/chrome/browser/prefs/b... File chrome/browser/prefs/browser_prefs.cc (right): https://codereview.chromium.org/1748773002/diff/200001/chrome/browser/prefs/b... chrome/browser/prefs/browser_prefs.cc:689: metrics_service On 2016/04/12 17:09:37, gab wrote: > When would there ever not be a g_browser_process->metrics_service()? Tests? I suppose so, but don't know for sure. I'm just following directions as per browser_process.h, which says "Services: any of these getters may return NULL" just above metrics_service().
The CQ bit was checked by grt@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from zea@chromium.org Link to the patchset: https://codereview.chromium.org/1748773002/#ps200001 (title: "fieldtrial testing")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1748773002/200001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1748773002/200001
Message was sent while issue was closed.
Description was changed from ========== Simplify the default browser infobar. - Changes the names of the histograms and user actions so that they reflect the action the user made rather than the behavior of the buttons. - Cleans up the InfoBarDelegate implementation a bit. - Introduces a Win10+ string for the OK/Accept button's label (currently the same as the original label) to enable experimentation. - Adds support for refreshing the infobar after a configurable number of days has elapsed. BUG=589128 ========== to ========== Simplify the default browser infobar. - Changes the names of the histograms and user actions so that they reflect the action the user made rather than the behavior of the buttons. - Cleans up the InfoBarDelegate implementation a bit. - Introduces a Win10+ string for the OK/Accept button's label (currently the same as the original label) to enable experimentation. - Adds support for refreshing the infobar after a configurable number of days has elapsed. BUG=589128 ==========
Message was sent while issue was closed.
Committed patchset #6 (id:200001)
Message was sent while issue was closed.
Description was changed from ========== Simplify the default browser infobar. - Changes the names of the histograms and user actions so that they reflect the action the user made rather than the behavior of the buttons. - Cleans up the InfoBarDelegate implementation a bit. - Introduces a Win10+ string for the OK/Accept button's label (currently the same as the original label) to enable experimentation. - Adds support for refreshing the infobar after a configurable number of days has elapsed. BUG=589128 ========== to ========== Simplify the default browser infobar. - Changes the names of the histograms and user actions so that they reflect the action the user made rather than the behavior of the buttons. - Cleans up the InfoBarDelegate implementation a bit. - Introduces a Win10+ string for the OK/Accept button's label (currently the same as the original label) to enable experimentation. - Adds support for refreshing the infobar after a configurable number of days has elapsed. BUG=589128 Committed: https://crrev.com/a872c10d41b21c4e46347c4a7866e141013c59e5 Cr-Commit-Position: refs/heads/master@{#386726} ==========
Message was sent while issue was closed.
Patchset 6 (id:??) landed as https://crrev.com/a872c10d41b21c4e46347c4a7866e141013c59e5 Cr-Commit-Position: refs/heads/master@{#386726} |
