Settings reset prompt: add startup logic

Settings reset prompt: add startup logic

This adds the logic needed by the settings reset prompt to figure out
if a reset prompt should be shown to the user, and if so, which
setting the user should be prompted for.

BUG=
TBR=rkaplow@chromium.org

Review-Url: https://codereview.chromium.org/2727063006
Cr-Commit-Position: refs/heads/master@{#454550}
Committed: https://chromium.googlesource.com/chromium/src/+/856526d83e9120f70f27823f6399f596379a2600

[alito, 2017-03-02 20:46:01]

The CQ bit was checked by alito@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-02 20:46:36]

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

[alito, 2017-03-02 20:47:56]

PTAL!

[alito, 2017-03-02 20:48:30]

alito@chromium.org changed reviewers:
+ csharp@chromium.org, robertshield@chromium.org

[alito, 2017-03-02 20:48:30]

Now actually added reviewers... PTAL.

[commit-bot: I haz the power, 2017-03-02 21:33:28]

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

[commit-bot: I haz the power, 2017-03-02 21:33:29]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[csharp, 2017-03-03 00:50:46]

lgtm

https://codereview.chromium.org/2727063006/diff/1/chrome/browser/safe_browsin...
File
chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_config_unittest.cc
(right):

https://codereview.chromium.org/2727063006/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_config_unittest.cc:210:
constexpr const char kDelayParam[] = "delay_before_prompt_seconds";
I don't think you need the const, constexpr should be enough

https://codereview.chromium.org/2727063006/diff/1/chrome/browser/safe_browsin...
File
chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_controller.h
(right):

https://codereview.chromium.org/2727063006/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_controller.h:71:
// the dialog after a delay as determined by by the |kSettingsResetPrompt|
nit: by by -> by

https://codereview.chromium.org/2727063006/diff/1/chrome/common/pref_names.cc
File chrome/common/pref_names.cc (right):

https://codereview.chromium.org/2727063006/diff/1/chrome/common/pref_names.cc...
chrome/common/pref_names.cc:2485: // and will be increased when starting a new
prompt wave.
As discussed, this should probably just check for the value being different,
instead of greater.

[alito, 2017-03-03 01:35:00]

alito@chromium.org changed reviewers:
+ gab@chromium.org, rkaplow@chromium.org, sky@chromium.org

[alito, 2017-03-03 01:35:01]

Thanks.

Adding owners for the following files:

sky@ for:
  chrome/browser/chrome_browser_main_win.cc
  chrome/browser/ui/views/*

gab@ for:
  chrome/browser/prefs/*

rkaplow@ for: histograms.xml

https://codereview.chromium.org/2727063006/diff/1/chrome/browser/safe_browsin...
File
chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_config_unittest.cc
(right):

https://codereview.chromium.org/2727063006/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_config_unittest.cc:210:
constexpr const char kDelayParam[] = "delay_before_prompt_seconds";
On 2017/03/03 00:50:46, csharp wrote:
> I don't think you need the const, constexpr should be enough

Actually, I learned that it isn't enough. Without the const, the array will be
const, but it will be an array of chars not const chars.

https://codereview.chromium.org/2727063006/diff/1/chrome/browser/safe_browsin...
File
chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_controller.h
(right):

https://codereview.chromium.org/2727063006/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_controller.h:71:
// the dialog after a delay as determined by by the |kSettingsResetPrompt|
On 2017/03/03 00:50:46, csharp wrote:
> nit: by by -> by

Done.

https://codereview.chromium.org/2727063006/diff/1/chrome/common/pref_names.cc
File chrome/common/pref_names.cc (right):

https://codereview.chromium.org/2727063006/diff/1/chrome/common/pref_names.cc...
chrome/common/pref_names.cc:2485: // and will be increased when starting a new
prompt wave.
On 2017/03/03 00:50:46, csharp wrote:
> As discussed, this should probably just check for the value being different,
> instead of greater.

Done.

[alito, 2017-03-03 01:38:06]

alito@chromium.org changed reviewers:
+ bauerb@google.com

[alito, 2017-03-03 01:38:06]

Adding bauerb@ for chrome/browser/prefs/* since gab@ is tagged as slow due to
travel and perf.

[alito, 2017-03-03 01:38:27]

Description was changed from

==========
Settings reset prompt: add startup logic

This adds the logic needed by the settings reset prompt to figure out
if a reset prompt should be shown to the user, and if so, which
setting the user should be prompted for.

BUG=
==========

to

==========
Settings reset prompt: add startup logic

This adds the logic needed by the settings reset prompt to figure out
if a reset prompt should be shown to the user, and if so, which
setting the user should be prompted for.

BUG=
==========

[alito, 2017-03-03 01:38:27]

alito@chromium.org changed reviewers:
+ bauerb@chromium.org
- bauerb@google.com

[robertshield, 2017-03-03 01:53:57]

lgtm

[sky, 2017-03-03 04:03:01]

LGTM

[gab, 2017-03-03 07:39:27]

prefs lgtm w/ nit

https://codereview.chromium.org/2727063006/diff/40001/chrome/browser/prefs/ch...
File chrome/browser/prefs/chrome_pref_service_factory.cc (right):

https://codereview.chromium.org/2727063006/diff/40001/chrome/browser/prefs/ch...
chrome/browser/prefs/chrome_pref_service_factory.cc:248: #endif
#endif  // defined(OS_WIN)

[alito, 2017-03-03 07:51:01]

Description was changed from

==========
Settings reset prompt: add startup logic

This adds the logic needed by the settings reset prompt to figure out
if a reset prompt should be shown to the user, and if so, which
setting the user should be prompted for.

BUG=
==========

to

==========
Settings reset prompt: add startup logic

This adds the logic needed by the settings reset prompt to figure out
if a reset prompt should be shown to the user, and if so, which
setting the user should be prompted for.

BUG=
TBR=rkaplow@chromium.org
==========

[alito, 2017-03-03 07:51:18]

Thx gab! Landing now.

https://codereview.chromium.org/2727063006/diff/40001/chrome/browser/prefs/ch...
File chrome/browser/prefs/chrome_pref_service_factory.cc (right):

https://codereview.chromium.org/2727063006/diff/40001/chrome/browser/prefs/ch...
chrome/browser/prefs/chrome_pref_service_factory.cc:248: #endif
On 2017/03/03 07:39:26, gab (OOO until Mar 13) wrote:
> #endif  // defined(OS_WIN)

Done.

[alito, 2017-03-03 07:51:29]

The CQ bit was checked by alito@chromium.org

[alito, 2017-03-03 07:51:30]

The patchset sent to the CQ was uploaded after l-g-t-m from csharp@chromium.org,
sky@chromium.org, gab@chromium.org, robertshield@chromium.org
Link to the patchset: https://codereview.chromium.org/2727063006/#ps60001
(title: "Nits")

[commit-bot: I haz the power, 2017-03-03 07:51:43]

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-03-03 09:16:22]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1488527489662240,
"parent_rev": "5992cf13afe61e422c8fcff55349f2ad778841e9", "commit_rev":
"856526d83e9120f70f27823f6399f596379a2600"}

[commit-bot: I haz the power, 2017-03-03 09:17:02]

Description was changed from

==========
Settings reset prompt: add startup logic

This adds the logic needed by the settings reset prompt to figure out
if a reset prompt should be shown to the user, and if so, which
setting the user should be prompted for.

BUG=
TBR=rkaplow@chromium.org
==========

to

==========
Settings reset prompt: add startup logic

This adds the logic needed by the settings reset prompt to figure out
if a reset prompt should be shown to the user, and if so, which
setting the user should be prompted for.

BUG=
TBR=rkaplow@chromium.org

Review-Url: https://codereview.chromium.org/2727063006
Cr-Commit-Position: refs/heads/master@{#454550}
Committed:
https://chromium.googlesource.com/chromium/src/+/856526d83e9120f70f27823f6399...
==========

[commit-bot: I haz the power, 2017-03-03 09:17:04]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/856526d83e9120f70f27823f6399...