Implement Win 10 onboarding content logic

Implement Win 10 onboarding content logic

The onboarding content is a set of startup promo pages that are shown at
most once to the user.

Also adds a pref to remember if the Win10 promo page has been seen.

BUG=648686
Committed: https://crrev.com/ab0bd29c2204b4c28d4aef9024bd44074710eb76
Cr-Commit-Position: refs/heads/master@{#439236}

[Patrick Monette, 2016-12-15 00:37:46]

Description was changed from

==========
Implement Win 10 onboarding content logic

BUG=648686
==========

to

==========
Implement Win 10 onboarding content logic

Also adds a pref to remember if the Win10 promo page has been seen.

BUG=648686
==========

[tmartino, 2016-12-15 16:41:28]

tmartino@chromium.org changed reviewers:
+ tmartino@chromium.org

[tmartino, 2016-12-15 16:41:29]

https://codereview.chromium.org/2578843002/diff/1/chrome/browser/ui/startup/s...
File chrome/browser/ui/startup/startup_browser_creator.cc (right):

https://codereview.chromium.org/2578843002/diff/1/chrome/browser/ui/startup/s...
chrome/browser/ui/startup/startup_browser_creator.cc:494:
registry->RegisterBooleanPref(prefs::kHasSeenWin10PromoPage, false);
Per talking with Eli and Rachel, let's make this a per-machine value (use Local
State) instead of per-profile. (Just move this up after line 480 above, and then
read from Local State in StartupTabProvider.)

https://codereview.chromium.org/2578843002/diff/1/chrome/browser/ui/startup/s...
File chrome/browser/ui/startup/startup_tab_provider.cc (right):

https://codereview.chromium.org/2578843002/diff/1/chrome/browser/ui/startup/s...
chrome/browser/ui/startup/startup_tab_provider.cc:36: bool has_seen_welcome_page
=
Let's define and populate has_seen_welcome_page, is_signed_in, and is_first_run
outside the #ifdef block, so that we don't duplicate that code. Those state
checks work for both calls.

[Patrick Monette, 2016-12-15 18:28:09]

https://codereview.chromium.org/2578843002/diff/1/chrome/browser/ui/startup/s...
File chrome/browser/ui/startup/startup_browser_creator.cc (right):

https://codereview.chromium.org/2578843002/diff/1/chrome/browser/ui/startup/s...
chrome/browser/ui/startup/startup_browser_creator.cc:494:
registry->RegisterBooleanPref(prefs::kHasSeenWin10PromoPage, false);
On 2016/12/15 16:41:28, tmartino wrote:
> Per talking with Eli and Rachel, let's make this a per-machine value (use
Local
> State) instead of per-profile. (Just move this up after line 480 above, and
then
> read from Local State in StartupTabProvider.)

Done.

https://codereview.chromium.org/2578843002/diff/1/chrome/browser/ui/startup/s...
File chrome/browser/ui/startup/startup_tab_provider.cc (right):

https://codereview.chromium.org/2578843002/diff/1/chrome/browser/ui/startup/s...
chrome/browser/ui/startup/startup_tab_provider.cc:36: bool has_seen_welcome_page
=
On 2016/12/15 16:41:28, tmartino wrote:
> Let's define and populate has_seen_welcome_page, is_signed_in, and
is_first_run
> outside the #ifdef block, so that we don't duplicate that code. Those state
> checks work for both calls.

Done.

[tmartino, 2016-12-15 19:00:10]

lgtm

one more thing, otherwise LGTM

https://codereview.chromium.org/2578843002/diff/20001/chrome/browser/ui/start...
File chrome/browser/ui/startup/startup_browser_creator_impl.cc (left):

https://codereview.chromium.org/2578843002/diff/20001/chrome/browser/ui/start...
chrome/browser/ui/startup/startup_browser_creator_impl.cc:284: bool
UseConsolidatedFlow() {
Whoops, one more thing. We haven't launch-reviewed this, so we don't want to
couple the Win 10 and non-Win 10 launches. We'll have to add another Finch
feature controlling the behavior on Win10.

Let's put this function back in, and either add a new feature to
startup_features.h controlling the Win 10 behavior, or just leave it as false
and we can add the feature in a followup CL.

[Patrick Monette, 2016-12-15 20:17:34]

pmonette@chromium.org changed reviewers:
+ pkasting@chromium.org

[Patrick Monette, 2016-12-15 20:17:35]

Hi Peter, PTAL

https://codereview.chromium.org/2578843002/diff/20001/chrome/browser/ui/start...
File chrome/browser/ui/startup/startup_browser_creator_impl.cc (left):

https://codereview.chromium.org/2578843002/diff/20001/chrome/browser/ui/start...
chrome/browser/ui/startup/startup_browser_creator_impl.cc:284: bool
UseConsolidatedFlow() {
On 2016/12/15 19:00:10, tmartino wrote:
> Whoops, one more thing. We haven't launch-reviewed this, so we don't want to
> couple the Win 10 and non-Win 10 launches. We'll have to add another Finch
> feature controlling the behavior on Win10.
> 
> Let's put this function back in, and either add a new feature to
> startup_features.h controlling the Win 10 behavior, or just leave it as false
> and we can add the feature in a followup CL.

Done.

[Peter Kasting, 2016-12-16 08:53:55]

LGTM

Please update the CL description to give a summary of what "Implement Win 10
onboarding content logic" actually entails, so spelunkers don't have to try to
read the tea leaves of the bug (which is also super-vague) and/or the design doc
here.

https://codereview.chromium.org/2578843002/diff/40001/chrome/browser/ui/start...
File chrome/browser/ui/startup/startup_tab_provider.h (right):

https://codereview.chromium.org/2578843002/diff/40001/chrome/browser/ui/start...
chrome/browser/ui/startup/startup_tab_provider.h:68: #if defined(OS_WIN)
I have no detailed proposal here, but I don't love the #if OS_WINs in this
cross-platform code.  It might be nice to use a platform-specific subclass
instead to implement this sort of thing.

https://codereview.chromium.org/2578843002/diff/40001/chrome/browser/ui/start...
chrome/browser/ui/startup/startup_tab_provider.h:111: // true, a
Nit: rewrap

[Patrick Monette, 2016-12-16 18:59:42]

Description was changed from

==========
Implement Win 10 onboarding content logic

Also adds a pref to remember if the Win10 promo page has been seen.

BUG=648686
==========

to

==========
Implement Win 10 onboarding content logic

The onboarding content is a set of promo WebUI pages that are shown once
to the user.

Also adds a pref to remember if the Win10 promo page has been seen.

BUG=648686
==========

[Patrick Monette, 2016-12-16 19:01:22]

Description was changed from

==========
Implement Win 10 onboarding content logic

The onboarding content is a set of promo WebUI pages that are shown once
to the user.

Also adds a pref to remember if the Win10 promo page has been seen.

BUG=648686
==========

to

==========
Implement Win 10 onboarding content logic

The onboarding content is a set of startup promo pages that are shown once
to the user.

Also adds a pref to remember if the Win10 promo page has been seen.

BUG=648686
==========

[Patrick Monette, 2016-12-16 19:02:09]

Description was changed from

==========
Implement Win 10 onboarding content logic

The onboarding content is a set of startup promo pages that are shown once
to the user.

Also adds a pref to remember if the Win10 promo page has been seen.

BUG=648686
==========

to

==========
Implement Win 10 onboarding content logic

The onboarding content is a set of startup promo pages that are shown at
most once to the user.

Also adds a pref to remember if the Win10 promo page has been seen.

BUG=648686
==========

[Patrick Monette, 2016-12-16 19:02:18]

Thanks!

https://codereview.chromium.org/2578843002/diff/40001/chrome/browser/ui/start...
File chrome/browser/ui/startup/startup_tab_provider.h (right):

https://codereview.chromium.org/2578843002/diff/40001/chrome/browser/ui/start...
chrome/browser/ui/startup/startup_tab_provider.h:68: #if defined(OS_WIN)
On 2016/12/16 08:53:55, Peter Kasting wrote:
> I have no detailed proposal here, but I don't love the #if OS_WINs in this
> cross-platform code.  It might be nice to use a platform-specific subclass
> instead to implement this sort of thing.

Yeah it's less than ideal but since there is only a small Windows exception, I
think it's better to keep it like this for simplicity than to create a subclass.

https://codereview.chromium.org/2578843002/diff/40001/chrome/browser/ui/start...
chrome/browser/ui/startup/startup_tab_provider.h:111: // true, a
On 2016/12/16 08:53:55, Peter Kasting wrote:
> Nit: rewrap

Done.

[Patrick Monette, 2016-12-16 21:47:08]

The CQ bit was checked by pmonette@chromium.org

[Patrick Monette, 2016-12-16 21:47:08]

The patchset sent to the CQ was uploaded after l-g-t-m from
tmartino@chromium.org, pkasting@chromium.org
Link to the patchset: https://codereview.chromium.org/2578843002/#ps60001
(title: "Fixed comments")

[commit-bot: I haz the power, 2016-12-16 21:48:38]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-16 23:55:51]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1481924828572140,
"parent_rev": "23ae4c127cc2c1794d062e234bf4bc42c6901742", "commit_rev":
"6118ac4ff121754873ab87eb2639bc06d58bcf88"}

[commit-bot: I haz the power, 2016-12-16 23:56:18]

Description was changed from

==========
Implement Win 10 onboarding content logic

The onboarding content is a set of startup promo pages that are shown at
most once to the user.

Also adds a pref to remember if the Win10 promo page has been seen.

BUG=648686
==========

to

==========
Implement Win 10 onboarding content logic

The onboarding content is a set of startup promo pages that are shown at
most once to the user.

Also adds a pref to remember if the Win10 promo page has been seen.

BUG=648686

Review-Url: https://codereview.chromium.org/2578843002
==========

[commit-bot: I haz the power, 2016-12-16 23:56:19]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-12-17 00:00:15]

Description was changed from

==========
Implement Win 10 onboarding content logic

The onboarding content is a set of startup promo pages that are shown at
most once to the user.

Also adds a pref to remember if the Win10 promo page has been seen.

BUG=648686

Review-Url: https://codereview.chromium.org/2578843002
==========

to

==========
Implement Win 10 onboarding content logic

The onboarding content is a set of startup promo pages that are shown at
most once to the user.

Also adds a pref to remember if the Win10 promo page has been seen.

BUG=648686

Committed: https://crrev.com/ab0bd29c2204b4c28d4aef9024bd44074710eb76
Cr-Commit-Position: refs/heads/master@{#439236}
==========

[commit-bot: I haz the power, 2016-12-17 00:00:16]

Patchset 4 (id:??) landed as
https://crrev.com/ab0bd29c2204b4c28d4aef9024bd44074710eb76
Cr-Commit-Position: refs/heads/master@{#439236}