New issue
Advanced search Search tips

Issue 688574 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 3
Type: Bug



Sign in to add a comment

StartupBrowserCreatorImpl refactor tech debt

Project Member Reported by tmartino@chromium.org, Feb 3 2017

Issue description

This bug tracks cleanup efforts left after the SBCI refactor.
 
Project Member

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

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

commit e2288ebbd5ec887e98b1140b6b8c12d56514c9aa
Author: tmartino <tmartino@chromium.org>
Date: Thu Feb 09 21:07:29 2017

Cleaning up a number of outstanding comments from refactor of StartupBrowserCreatorImpl. Specifically:

- Adds SessionRestore::Behavior typedef, replacing uint32_t as arg

- Renames |is_ephemeral_profile| to |is_incognito_or_guest|, as concerns were raised that “ephemeral profile” is a specific, loaded term.

- Changes DetermineBrowserOpenBehavior to use bitmask.

- Adds unit tests for DetermineBrowserOpenBehavior.

- Removes extraneous TODOs.

- Adds browser tests testing welcome pages.

BUG= 688574 

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

[modify] https://crrev.com/e2288ebbd5ec887e98b1140b6b8c12d56514c9aa/chrome/browser/sessions/session_restore.cc
[modify] https://crrev.com/e2288ebbd5ec887e98b1140b6b8c12d56514c9aa/chrome/browser/sessions/session_restore.h
[modify] https://crrev.com/e2288ebbd5ec887e98b1140b6b8c12d56514c9aa/chrome/browser/ui/startup/startup_browser_creator_browsertest.cc
[modify] https://crrev.com/e2288ebbd5ec887e98b1140b6b8c12d56514c9aa/chrome/browser/ui/startup/startup_browser_creator_impl.cc
[modify] https://crrev.com/e2288ebbd5ec887e98b1140b6b8c12d56514c9aa/chrome/browser/ui/startup/startup_browser_creator_impl.h
[modify] https://crrev.com/e2288ebbd5ec887e98b1140b6b8c12d56514c9aa/chrome/browser/ui/startup/startup_browser_creator_impl_unittest.cc
[modify] https://crrev.com/e2288ebbd5ec887e98b1140b6b8c12d56514c9aa/chrome/browser/ui/startup/startup_tab_provider.cc
[modify] https://crrev.com/e2288ebbd5ec887e98b1140b6b8c12d56514c9aa/chrome/browser/ui/startup/startup_tab_provider.h
[modify] https://crrev.com/e2288ebbd5ec887e98b1140b6b8c12d56514c9aa/chrome/browser/ui/startup/startup_tab_provider_unittest.cc

Project Member

Comment 2 by bugdroid1@chromium.org, May 9 2017

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

commit 4000f08774959883da8bef366561798e98cb50eb
Author: tmartino <tmartino@chromium.org>
Date: Tue May 09 04:42:28 2017

[Desktop FRE] Fix incorrect emission of histogram

This CL fixes a bug where the Welcome.Win10.NewPromoPageAdded histogram
is emitted for the wrong page--it is meant to track chrome://welcome-win10,
which corresponds to Welcome.Win10.OldPromoPageAdded tracking the old
default browser promo. This CL also addresses a refactoring nit requested
in a prior review.

BUG= 688574 

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

[modify] https://crrev.com/4000f08774959883da8bef366561798e98cb50eb/chrome/browser/ui/startup/startup_tab_provider.cc

Labels: Merge-Request-59
Requesting merge for 4000f08774959883da8bef366561798e98cb50eb to m59
Project Member

Comment 4 by sheriffbot@chromium.org, May 10 2017

Labels: -Merge-Request-59 Hotlist-Merge-Approved Merge-Approved-59
Your change meets the bar and is auto-approved for M59. Please go ahead and merge the CL to branch 3071 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop)

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

Comment 5 by gov...@chromium.org, May 12 2017

Please merge your change to M59 branch 3071 by 4:00 PM PT, Monday (05/15) so we can take it in for next week beta release. Thank you.
Project Member

Comment 6 by sheriffbot@chromium.org, May 15 2017

Cc: pmonette@chromium.org
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

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

Comment 7 by bugdroid1@chromium.org, May 15 2017

Labels: -merge-approved-59 merge-merged-3071
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/248a38b867f44add9113570c34c6eef71090da80

commit 248a38b867f44add9113570c34c6eef71090da80
Author: Francois Doray <fdoray@chromium.org>
Date: Mon May 15 18:56:06 2017

[Desktop FRE] Fix incorrect emission of histogram

This CL fixes a bug where the Welcome.Win10.NewPromoPageAdded histogram
is emitted for the wrong page--it is meant to track chrome://welcome-win10,
which corresponds to Welcome.Win10.OldPromoPageAdded tracking the old
default browser promo. This CL also addresses a refactoring nit requested
in a prior review.

BUG= 688574 

Review-Url: https://codereview.chromium.org/2848713002
Cr-Original-Commit-Position: refs/heads/master@{#470218}
Review-Url: https://codereview.chromium.org/2879193004 .
Cr-Commit-Position: refs/branch-heads/3071@{#562}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/248a38b867f44add9113570c34c6eef71090da80/chrome/browser/ui/startup/startup_tab_provider.cc

Status: Fixed (was: Started)

Sign in to add a comment