New issue
Advanced search Search tips

Issue 693292 link

Starred by 2 users
Status: Archived
Owner:
pmonette@chromium.org
Closed: Sep 18
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 3
Type: Bug



Sign in to add a comment

Refactor the default browser prompt logic

Project Member Reported by pmonette@chromium.org, Feb 17 2017 
The logic that determines if the default browser prompt should be shown is scattered in multiple places, and shared between a few different preferences.

Of the top of my head:

prefs::kDefaultBrowserSettingEnabled
prefs::kResetCheckDefaultBrowser
prefs::kDefaultBrowserLastDeclined
prefs::kBrowserSuppressDefaultBrowserPrompt
installer::master_preferences::kSuppressFirstRunDefaultBrowserPrompt

ShouldShowDefaultBrowserPrompt()
ShowPrompt()
ShowDefaultBrowserPrompt()
StartupBrowserCreatorImpl::AddInfoBarsIfNecessary()
Project Member

Comment 1 by bugdroid1@chromium.org, Feb 21 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/31b2815487e8c7edcc7ae481cc3494887110ace3

commit 31b2815487e8c7edcc7ae481cc3494887110ace3
Author: pmonette <pmonette@chromium.org>
Date: Tue Feb 21 17:56:22 2017

Never show the default browser prompt over the first run promos

Also expose first_run::IsOnWelcomePage() so it can be reused.

BUG= 693292 

Review-Url: https://codereview.chromium.org/2692203009
Cr-Commit-Position: refs/heads/master@{#451794}

[modify] https://crrev.com/31b2815487e8c7edcc7ae481cc3494887110ace3/chrome/browser/first_run/first_run.cc
[modify] https://crrev.com/31b2815487e8c7edcc7ae481cc3494887110ace3/chrome/browser/first_run/first_run.h
[modify] https://crrev.com/31b2815487e8c7edcc7ae481cc3494887110ace3/chrome/browser/ui/startup/default_browser_prompt.cc

Comment 2 by pmonette@chromium.org, Feb 21 2017

Labels: Merge-Request-57
Requesting merge to m-57 for commit 31b2815487e8c7edcc7ae481cc3494887110ace3.
Project Member

Comment 3 by sheriffbot@chromium.org, Feb 22 2017

Labels: -Merge-Request-57 Hotlist-Merge-Approved Merge-Approved-57
Your change meets the bar and is auto-approved for M57. Please go ahead and merge the CL to branch 2987 manually. Please contact milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 4 by mummare...@chromium.org, Feb 22 2017 

If possible, could you please merge your change to M57 branch 2987 by 5:00 PM PT today, Wednesday (02/22) so we can pick it up for this week beta release. 
Thank you.
Project Member

Comment 5 by bugdroid1@chromium.org, Feb 22 2017

Labels: -merge-approved-57 merge-merged-2987
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f93abaa502aed3360e0222f7f27dc05243961379

commit f93abaa502aed3360e0222f7f27dc05243961379
Author: pmonette <pmonette@chromium.org>
Date: Wed Feb 22 19:48:05 2017

Never show the default browser prompt over the first run promos

Also expose first_run::IsOnWelcomePage() so it can be reused.

BUG= 693292 
NOTRY=true
NOPRESUBMIT=true

Review-Url: https://codereview.chromium.org/2692203009
Cr-Commit-Position: refs/heads/master@{#451794}
(cherry picked from commit 31b2815487e8c7edcc7ae481cc3494887110ace3)

Review-Url: https://codereview.chromium.org/2710073002
Cr-Commit-Position: refs/branch-heads/2987@{#644}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[modify] https://crrev.com/f93abaa502aed3360e0222f7f27dc05243961379/chrome/browser/first_run/first_run.cc
[modify] https://crrev.com/f93abaa502aed3360e0222f7f27dc05243961379/chrome/browser/first_run/first_run.h
[modify] https://crrev.com/f93abaa502aed3360e0222f7f27dc05243961379/chrome/browser/ui/startup/default_browser_prompt.cc

Comment 6 by ew...@chromium.org, Feb 28 2017 

Can we mark this as Fixed?

Comment 7 by pmonette@chromium.org, Feb 28 2017 

No this bug tracks the refactoring needed to unspaghetti the code.

Comment 8 by markchang@chromium.org, Sep 18

Status: Archived (was: Assigned)
Archiving old bugs that have only received trivial updates for some time.

If you feel this issue should still be addressed, feel free to reopen it or to file a new issue. Thanks!
►

Sign in to add a comment