Set kHasSeenWin10PromoPage in local_state instead of in profile prefs.

Set kHasSeenWin10PromoPage in local_state instead of in profile prefs.

The pref is registered to the local state so the old code was incorrect.

BUG=685799
Review-Url: https://codereview.chromium.org/2659643003
Cr-Commit-Position: refs/heads/master@{#446497}
Committed: https://chromium.googlesource.com/chromium/src/+/7c6a615b2b23452b33af668caab3945fe44b52a4

[Patrick Monette, 2017-01-26 20:09:24]

Patchset #1 (id:1) has been deleted

[Patrick Monette, 2017-01-26 20:09:49]

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

[Patrick Monette, 2017-01-26 20:09:50]

PTAL. Small bug fix.

[Peter Kasting, 2017-01-26 21:14:32]

LGTM on making code consistent, but I'm wondering if this should actually be in
the profile prefs instead of the local state.  If we already showed a user the
promo on one of their machines, isn't showing it again on another machine
annoying?

https://codereview.chromium.org/2659643003/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/welcome_win10_ui.cc (right):

https://codereview.chromium.org/2659643003/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/welcome_win10_ui.cc:125: Profile* profile =
Profile::FromWebUI(web_ui);
Nit: Or just inline this

[tmartino, 2017-01-26 21:32:52]

On 2017/01/26 at 21:14:32, pkasting wrote:
> LGTM on making code consistent, but I'm wondering if this should actually be
in the profile prefs instead of the local state.  If we already showed a user
the promo on one of their machines, isn't showing it again on another machine
annoying?

Drive-by comments: I actually checked with PM & UX on this point during
implementation. Because this promo relates to per-machine state (default browser
and whether Chrome is pinned to taskbar), they said they preferred to show
once-per-machine, rather than once-per-profile.

[Peter Kasting, 2017-01-26 21:41:03]

On 2017/01/26 21:32:52, tmartino wrote:
> On 2017/01/26 at 21:14:32, pkasting wrote:
> > LGTM on making code consistent, but I'm wondering if this should actually be
> in the profile prefs instead of the local state.  If we already showed a user
> the promo on one of their machines, isn't showing it again on another machine
> annoying?
> 
> Drive-by comments: I actually checked with PM & UX on this point during
> implementation. Because this promo relates to per-machine state (default
browser
> and whether Chrome is pinned to taskbar), they said they preferred to show
> once-per-machine, rather than once-per-profile.

Fair enough.  I assume we avoid suggesting to users that they pin/set as default
in cases where that's already true.  If not, that should be fixed separately :)

[Patrick Monette, 2017-01-26 22:05:26]

On 2017/01/26 21:41:03, Peter Kasting wrote:
> On 2017/01/26 21:32:52, tmartino wrote:
> > On 2017/01/26 at 21:14:32, pkasting wrote:
> > > LGTM on making code consistent, but I'm wondering if this should actually
be
> > in the profile prefs instead of the local state.  If we already showed a
user
> > the promo on one of their machines, isn't showing it again on another
machine
> > annoying?
> > 
> > Drive-by comments: I actually checked with PM & UX on this point during
> > implementation. Because this promo relates to per-machine state (default
> browser
> > and whether Chrome is pinned to taskbar), they said they preferred to show
> > once-per-machine, rather than once-per-profile.
> 
> Fair enough.  I assume we avoid suggesting to users that they pin/set as
default
> in cases where that's already true.  If not, that should be fixed separately
:)

Yeah we always check for default state before deciding to show.

[Patrick Monette, 2017-01-26 22:09:13]

Thanks

https://codereview.chromium.org/2659643003/diff/20001/chrome/browser/ui/webui...
File chrome/browser/ui/webui/welcome_win10_ui.cc (right):

https://codereview.chromium.org/2659643003/diff/20001/chrome/browser/ui/webui...
chrome/browser/ui/webui/welcome_win10_ui.cc:125: Profile* profile =
Profile::FromWebUI(web_ui);
On 2017/01/26 21:14:32, Peter Kasting wrote:
> Nit: Or just inline this

Done.

[Patrick Monette, 2017-01-26 22:13:06]

Description was changed from

==========
Set kHasSeenWin10PromoPage in local_state instead of in profile prefs.

The pref is registered to the local state so the old code was incorrect.

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

to

==========
Set kHasSeenWin10PromoPage in local_state instead of in profile prefs.

The pref is registered to the local state so the old code was incorrect.

BUG=685799
==========

[Patrick Monette, 2017-01-26 22:17:01]

The CQ bit was checked by pmonette@chromium.org

[Patrick Monette, 2017-01-26 22:17:02]

The patchset sent to the CQ was uploaded after l-g-t-m from
pkasting@chromium.org
Link to the patchset: https://codereview.chromium.org/2659643003/#ps40001
(title: "Inline Profile::FromWebUI()")

[commit-bot: I haz the power, 2017-01-26 22:17:51]

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, 2017-01-26 23:11:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-26 23:11:11]

Failed to apply patch for chrome/browser/ui/webui/welcome_win10_ui.cc:
While running git apply --index -p1;
  error: patch failed: chrome/browser/ui/webui/welcome_win10_ui.cc:92
  error: chrome/browser/ui/webui/welcome_win10_ui.cc: patch does not apply

Patch:       chrome/browser/ui/webui/welcome_win10_ui.cc
Index: chrome/browser/ui/webui/welcome_win10_ui.cc
diff --git a/chrome/browser/ui/webui/welcome_win10_ui.cc
b/chrome/browser/ui/webui/welcome_win10_ui.cc
index
cf416e30e2e22d7db852d3c33e6e5510d548bf74..1d5922df126e88e24897a06fb2f1eebfff95588f
100644
--- a/chrome/browser/ui/webui/welcome_win10_ui.cc
+++ b/chrome/browser/ui/webui/welcome_win10_ui.cc
@@ -8,6 +8,7 @@
 
 #include "base/feature_list.h"
 #include "base/memory/ptr_util.h"
+#include "chrome/browser/browser_process.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/startup/startup_features.h"
 #include "chrome/browser/ui/webui/welcome_win10_handler.h"
@@ -92,10 +93,9 @@ WelcomeWin10UI::WelcomeWin10UI(content::WebUI* web_ui, const
GURL& url)
   static const char kCssFilePath[] = "welcome.css";
   static const char kJsFilePath[] = "welcome.js";
 
-  Profile* profile = Profile::FromWebUI(web_ui);
-
-  // Store that this profile has been shown the Win10 promo page.
-  profile->GetPrefs()->SetBoolean(prefs::kHasSeenWin10PromoPage, true);
+  // Remember that the Win10 promo page has been shown.
+  g_browser_process->local_state()->SetBoolean(prefs::kHasSeenWin10PromoPage,
+                                               true);
 
   // Determine which variation to show.
   bool is_first_run = !UrlContainsKeyValueInQuery(url, "text", "faster");
@@ -122,7 +122,7 @@ WelcomeWin10UI::WelcomeWin10UI(content::WebUI* web_ui, const
GURL& url)
   html_source->AddResourcePath("logo-small.png", IDR_PRODUCT_LOGO_64);
   html_source->AddResourcePath("logo-large.png", IDR_PRODUCT_LOGO_128);
 
-  content::WebUIDataSource::Add(profile, html_source);
+  content::WebUIDataSource::Add(Profile::FromWebUI(web_ui), html_source);
 }
 
 WelcomeWin10UI::~WelcomeWin10UI() = default;

[Patrick Monette, 2017-01-26 23:16:03]

The CQ bit was checked by pmonette@chromium.org

[Patrick Monette, 2017-01-26 23:16:04]

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

[commit-bot: I haz the power, 2017-01-26 23:16: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, 2017-01-27 00:08:06]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1485472563913560,
"parent_rev": "1edbeeb9ee14e4bdbf673410967f3d0e9a03f8ce", "commit_rev":
"7c6a615b2b23452b33af668caab3945fe44b52a4"}

[commit-bot: I haz the power, 2017-01-27 00:08:43]

Description was changed from

==========
Set kHasSeenWin10PromoPage in local_state instead of in profile prefs.

The pref is registered to the local state so the old code was incorrect.

BUG=685799
==========

to

==========
Set kHasSeenWin10PromoPage in local_state instead of in profile prefs.

The pref is registered to the local state so the old code was incorrect.

BUG=685799

Review-Url: https://codereview.chromium.org/2659643003
Cr-Commit-Position: refs/heads/master@{#446497}
Committed:
https://chromium.googlesource.com/chromium/src/+/7c6a615b2b23452b33af668caab3...
==========

[commit-bot: I haz the power, 2017-01-27 00:08:44]

Committed patchset #3 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/7c6a615b2b23452b33af668caab3...