Adds a modal dialog implementation of the settings reset prompt.

Adds a modal dialog implementation of the settings reset prompt.

The settings reset prompt will ask users whose settings may have been
modified by unwanted software or extensions if they want to reset some
of their settings.

BUG=

Review-Url: https://codereview.chromium.org/2701313002
Cr-Commit-Position: refs/heads/master@{#453048}
Committed: https://chromium.googlesource.com/chromium/src/+/c61da3e4d5f324c493728c5cc2f56c8dc962817d

[alito, 2017-02-20 00:36:24]

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

[alito, 2017-02-20 00:36:25]

Scott, could you please take a look at the code for this new dialog?

We are currently working with the UX team to finalize the design. I'd like to
land this version and iterate with UX to get it into its final state.

Thanks,
/Ali

[sky, 2017-02-21 17:37:28]

You need test coverage of the view->controller interaction too.

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/safe_browsin...
File
chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_config.h
(left):

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_config.h:36:
virtual ~SettingsResetPromptConfig();
If you're going to cleanup this file, move the destructor above other functions
(see style guide).

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

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_controller.cc:112:
using LabelInfo = SettingsResetPromptController::LabelInfo;
I don't think you need the using here. LabelInfo should work fine.

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_controller.cc:123:
if (reset_search + reset_startup_urls + reset_homepage > 1) {
Uses casts to make this more obvious.

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_controller.cc:175:
// Append the explanation about the number of extensions that will be disabled
Define a format for this as appending isn't always the right thing in some
locales.

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_controller.cc:192:
using LabelInfo = SettingsResetPromptController::LabelInfo;
Same comment not needing using.

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_controller.cc:193:
details_text_.clear();
Why don't you DCHECK on empty like you do for main_text_ above?

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_controller.cc:249:
delete this;
If this object is going to control it's lifetime, then make the destructor
private.

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

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_controller.h:41:
LabelType type;
If you can, make both of these const.

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_controller.h:49:
// A controller should be created only if |model->ShouldPromptforReset()|
constructor/destructor above other functions (see style guide).

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_controller.h:55:
base::string16 GetWindowTitle();
GetWindowTitle() const ? Same comment for similar functions.

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_controller.h:59:
std::vector<LabelInfo> GetMainText();
const std::vector<LabelInfo>& GetMainText() const? Same comment for next line.

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_controller.h:79:
};
DISALLOW...

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/ui/views/set...
File chrome/browser/ui/views/settings_reset_prompt_dialog.cc (right):

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/ui/views/set...
chrome/browser/ui/views/settings_reset_prompt_dialog.cc:55: // single-line
labels that are indented and start with a bullet point. The
How do you know the single-line label doesn't need to wrap?

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/ui/views/set...
chrome/browser/ui/views/settings_reset_prompt_dialog.cc:74: };
DISALLOW...

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/ui/views/set...
chrome/browser/ui/views/settings_reset_prompt_dialog.cc:248: details_view_ = new
ExpandableMessageView(ContentWidth());
Why do you need to supply the width? Shouldn't ExpandableMessageView fill the
width?

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/ui/views/set...
chrome/browser/ui/views/settings_reset_prompt_dialog.cc:254:
SettingsResetPromptDialog::~SettingsResetPromptDialog() {}
What happens if you get here and neither accept nor cancel was called? This
could happen if the underlying widget is destroyed (say the browser closes).

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/ui/views/set...
chrome/browser/ui/views/settings_reset_prompt_dialog.cc:321: return Cancel();
This is the default implementation.

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/ui/views/set...
chrome/browser/ui/views/settings_reset_prompt_dialog.cc:349: int
SettingsResetPromptDialog::ContentWidth() const {
Why do you need a member function for this function. It uses no state.

[sky, 2017-02-21 17:39:29]

Also, please make sure you have a test to show the dialog. See
https://groups.google.com/a/chromium.org/forum/#!searchin/chromium-dev/Testin...
for details.

[alito, 2017-02-23 02:32:00]

Thanks Scott. I've addressed your comments. PTAnL!

Also added dialog browsertests (the DialogBrowserTest fixture seems really
useful to me. Would have loved to have had this while developing the dialog!)

/ali

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/safe_browsin...
File
chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_config.h
(left):

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_config.h:36:
virtual ~SettingsResetPromptConfig();
On 2017/02/21 17:37:27, sky wrote:
> If you're going to cleanup this file, move the destructor above other
functions
> (see style guide).

According to Google's external C++ style guide, factory functions should come
before the destructor: "prefer the following order: types (including typedef,
using, and nested structs and classes), constants, factory functions,
constructors, assignment operators, destructor, all other methods, data
members". Unless Chromium adds something to this (couldn't find anything...).

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

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_controller.cc:112:
using LabelInfo = SettingsResetPromptController::LabelInfo;
On 2017/02/21 17:37:27, sky wrote:
> I don't think you need the using here. LabelInfo should work fine.

Done.

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_controller.cc:123:
if (reset_search + reset_startup_urls + reset_homepage > 1) {
On 2017/02/21 17:37:28, sky wrote:
> Uses casts to make this more obvious.

Now using int{} to cast the bools to int. Is this what you meant?

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_controller.cc:175:
// Append the explanation about the number of extensions that will be disabled
On 2017/02/21 17:37:27, sky wrote:
> Define a format for this as appending isn't always the right thing in some
> locales.

Done.

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_controller.cc:192:
using LabelInfo = SettingsResetPromptController::LabelInfo;
On 2017/02/21 17:37:28, sky wrote:
> Same comment not needing using.

Done.

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_controller.cc:193:
details_text_.clear();
On 2017/02/21 17:37:28, sky wrote:
> Why don't you DCHECK on empty like you do for main_text_ above?

Just forgot to change it. Now using DCHECK just like for main_text_ above.

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_controller.cc:249:
delete this;
On 2017/02/21 17:37:27, sky wrote:
> If this object is going to control it's lifetime, then make the destructor
> private.

Done.

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

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_controller.h:41:
LabelType type;
On 2017/02/21 17:37:28, sky wrote:
> If you can, make both of these const.

Seems hard to accomplish since I'm using a vector to hold LabelInfo objects
(using vector's emplace_back() instead of push_back() would still require the
type to be MoveInsertable).

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_controller.h:49:
// A controller should be created only if |model->ShouldPromptforReset()|
On 2017/02/21 17:37:28, sky wrote:
> constructor/destructor above other functions (see style guide).

Done.

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_controller.h:55:
base::string16 GetWindowTitle();
On 2017/02/21 17:37:28, sky wrote:
> GetWindowTitle() const ? Same comment for similar functions.

Done.

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_controller.h:59:
std::vector<LabelInfo> GetMainText();
On 2017/02/21 17:37:28, sky wrote:
> const std::vector<LabelInfo>& GetMainText() const? Same comment for next line.

Done.

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_controller.h:79:
};
On 2017/02/21 17:37:28, sky wrote:
> DISALLOW...

Done.

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/ui/views/set...
File chrome/browser/ui/views/settings_reset_prompt_dialog.cc (right):

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/ui/views/set...
chrome/browser/ui/views/settings_reset_prompt_dialog.cc:55: // single-line
labels that are indented and start with a bullet point. The
On 2017/02/21 17:37:28, sky wrote:
> How do you know the single-line label doesn't need to wrap?

The indented (and bulleted) labels are used only for displaying setting values
(so not instructions or explanations). I've added this to the comment to make it
clearer.

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/ui/views/set...
chrome/browser/ui/views/settings_reset_prompt_dialog.cc:74: };
On 2017/02/21 17:37:28, sky wrote:
> DISALLOW...

Done.

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/ui/views/set...
chrome/browser/ui/views/settings_reset_prompt_dialog.cc:248: details_view_ = new
ExpandableMessageView(ContentWidth());
On 2017/02/21 17:37:28, sky wrote:
> Why do you need to supply the width? Shouldn't ExpandableMessageView fill the
> width?

You are right. Since I'm explicitly setting the width of the dialog itself, this
can be omitted.

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/ui/views/set...
chrome/browser/ui/views/settings_reset_prompt_dialog.cc:254:
SettingsResetPromptDialog::~SettingsResetPromptDialog() {}
On 2017/02/21 17:37:28, sky wrote:
> What happens if you get here and neither accept nor cancel was called? This
> could happen if the underlying widget is destroyed (say the browser closes).

I added a call to the controller's Cancel() here. Is that sufficient or should
the dialog class override WidgetDelegate::WindowClosing()?

Also added checks in Accept() and Cancel() to ensure that the controller's
Accept() and Cancel() are called only once. I wasn't sure if DCHECKs would have
been sufficient (is it possible for a dialog's Accept() and Cancel() functions
to be called multiple times somehow, for example, if user double-clicks a
button?).

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/ui/views/set...
chrome/browser/ui/views/settings_reset_prompt_dialog.cc:321: return Cancel();
On 2017/02/21 17:37:28, sky wrote:
> This is the default implementation.

Actually, that is the default implementation only if a cancel button is also
shown in the dialog. In our case we show only the accept button and the "x" at
the top right of the dialog window. In this case the default implementation of
Close() calls Accept().

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/ui/views/set...
chrome/browser/ui/views/settings_reset_prompt_dialog.cc:349: int
SettingsResetPromptDialog::ContentWidth() const {
On 2017/02/21 17:37:28, sky wrote:
> Why do you need a member function for this function. It uses no state.

Removed.

[sky, 2017-02-23 04:27:31]

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/safe_browsin...
File
chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_config.h
(left):

https://codereview.chromium.org/2701313002/diff/1/chrome/browser/safe_browsin...
chrome/browser/safe_browsing/settings_reset_prompt/settings_reset_prompt_config.h:36:
virtual ~SettingsResetPromptConfig();
On 2017/02/23 02:31:59, alito wrote:
> On 2017/02/21 17:37:27, sky wrote:
> > If you're going to cleanup this file, move the destructor above other
> functions
> > (see style guide).
> 
> According to Google's external C++ style guide, factory functions should come
> before the destructor: "prefer the following order: types (including typedef,
> using, and nested structs and classes), constants, factory functions,
> constructors, assignment operators, destructor, all other methods, data
> members". Unless Chromium adds something to this (couldn't find anything...).

I clearly missed the 'factory functions' sections. Now I wonder if that changed
at some point...

https://codereview.chromium.org/2701313002/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/settings_reset_prompt_dialog.cc (right):

https://codereview.chromium.org/2701313002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/settings_reset_prompt_dialog.cc:58: class
SimpleMessageView : public views::View {
Do you really need a view subclass here? It seems like all you need is a
function to create the layoutmanager, and a function you call to add the labels.

https://codereview.chromium.org/2701313002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/settings_reset_prompt_dialog.cc:161: gfx::Size
GetPreferredSize() const override;
Prefix with where overrides come from.

https://codereview.chromium.org/2701313002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/settings_reset_prompt_dialog.cc:172: : visible_(false) {
This is mildly confusing given View also has a SetVisible. Can you use View's
SetVisible()?

https://codereview.chromium.org/2701313002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/settings_reset_prompt_dialog.cc:191: new
SimpleMessageView(views::kUnrelatedControlVerticalSpacing);
Move to member initializer list.

https://codereview.chromium.org/2701313002/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/settings_reset_prompt_dialog_browsertest.cc
(right):

https://codereview.chromium.org/2701313002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/settings_reset_prompt_dialog_browsertest.cc:45: enum
SettingType {
enum class

https://codereview.chromium.org/2701313002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/settings_reset_prompt_dialog_browsertest.cc:46: DSE,  //
Default search engine
Please spell these out, otherwise it's super cryptic.

https://codereview.chromium.org/2701313002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/settings_reset_prompt_dialog_browsertest.cc:129: };
DISALLOW...

[alito, 2017-02-23 05:37:09]

Thanks Scott. I addressed your comments, but was unsure about one of your
suggestions, so left you a question in the response. PTAnL.

/ali

https://codereview.chromium.org/2701313002/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/settings_reset_prompt_dialog.cc (right):

https://codereview.chromium.org/2701313002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/settings_reset_prompt_dialog.cc:58: class
SimpleMessageView : public views::View {
On 2017/02/23 04:27:31, sky wrote:
> Do you really need a view subclass here? It seems like all you need is a
> function to create the layoutmanager, and a function you call to add the
labels.

SimpleMessageView is used both in the main dialog class, where it is used
directly to display the main dialog text, and in ExpandableMessageView for the
expandable details section. Not sure how I could avoid subclassing View? Or
maybe I'm misunderstanding what you are suggesting...

https://codereview.chromium.org/2701313002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/settings_reset_prompt_dialog.cc:161: gfx::Size
GetPreferredSize() const override;
On 2017/02/23 04:27:31, sky wrote:
> Prefix with where overrides come from.

Done.

https://codereview.chromium.org/2701313002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/settings_reset_prompt_dialog.cc:172: : visible_(false) {
On 2017/02/23 04:27:31, sky wrote:
> This is mildly confusing given View also has a SetVisible. Can you use View's
> SetVisible()?

Done.

https://codereview.chromium.org/2701313002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/settings_reset_prompt_dialog.cc:191: new
SimpleMessageView(views::kUnrelatedControlVerticalSpacing);
On 2017/02/23 04:27:31, sky wrote:
> Move to member initializer list.

Done.

https://codereview.chromium.org/2701313002/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/settings_reset_prompt_dialog_browsertest.cc
(right):

https://codereview.chromium.org/2701313002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/settings_reset_prompt_dialog_browsertest.cc:45: enum
SettingType {
On 2017/02/23 04:27:31, sky wrote:
> enum class

Done.

https://codereview.chromium.org/2701313002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/settings_reset_prompt_dialog_browsertest.cc:46: DSE,  //
Default search engine
On 2017/02/23 04:27:31, sky wrote:
> Please spell these out, otherwise it's super cryptic.

Done.

https://codereview.chromium.org/2701313002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/settings_reset_prompt_dialog_browsertest.cc:129: };
On 2017/02/23 04:27:31, sky wrote:
> DISALLOW...

Done.

[sky, 2017-02-23 18:44:52]

https://codereview.chromium.org/2701313002/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/settings_reset_prompt_dialog.cc (right):

https://codereview.chromium.org/2701313002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/settings_reset_prompt_dialog.cc:58: class
SimpleMessageView : public views::View {
On 2017/02/23 05:37:08, alito wrote:
> On 2017/02/23 04:27:31, sky wrote:
> > Do you really need a view subclass here? It seems like all you need is a
> > function to create the layoutmanager, and a function you call to add the
> labels.
> 
> SimpleMessageView is used both in the main dialog class, where it is used
> directly to display the main dialog text, and in ExpandableMessageView for the
> expandable details section. Not sure how I could avoid subclassing View? Or
> maybe I'm misunderstanding what you are suggesting...

My main concern here is your are subclassing but not overriding any
functionality. This is typically an indication you don't need a subclass. You
can avoid the subclass by either using functions, say a CreateViewForLabels()
that returns a View and GridLayout (what you have in the constructor) and an
AddLabel() function that takes the GridLayout. Alternatively make
SimpleMessageView be an object that has a view, not is a view, in which case I
recommend naming it LabelBuilder. Does that make sense?

[alito, 2017-02-24 00:19:09]

Scott, I've made the change you suggested.

Also, after a chat with UI, we decided to add a Cancel button to the dialog. I
added a small patch separately for that as well.

PTAnL.

https://codereview.chromium.org/2701313002/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/settings_reset_prompt_dialog.cc (right):

https://codereview.chromium.org/2701313002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/settings_reset_prompt_dialog.cc:58: class
SimpleMessageView : public views::View {
On 2017/02/23 18:44:52, sky wrote:
> On 2017/02/23 05:37:08, alito wrote:
> > On 2017/02/23 04:27:31, sky wrote:
> > > Do you really need a view subclass here? It seems like all you need is a
> > > function to create the layoutmanager, and a function you call to add the
> > labels.
> > 
> > SimpleMessageView is used both in the main dialog class, where it is used
> > directly to display the main dialog text, and in ExpandableMessageView for
the
> > expandable details section. Not sure how I could avoid subclassing View? Or
> > maybe I'm misunderstanding what you are suggesting...
> 
> My main concern here is your are subclassing but not overriding any
> functionality. This is typically an indication you don't need a subclass. You
> can avoid the subclass by either using functions, say a CreateViewForLabels()
> that returns a View and GridLayout (what you have in the constructor) and an
> AddLabel() function that takes the GridLayout. Alternatively make
> SimpleMessageView be an object that has a view, not is a view, in which case I
> recommend naming it LabelBuilder. Does that make sense?

Ah, I see your point. Makes sense not to subclass the view. I've changed this to
use a CreateLabelView() function instead.

[sky, 2017-02-24 04:11:09]

LGTM - thanks!

[alito, 2017-02-24 17:21:31]

Thanks Scott.

Robert, any comments?

[robertshield, 2017-02-24 19:43:20]

lgtm

[alito, 2017-02-24 19:49:16]

The CQ bit was checked by alito@chromium.org

[commit-bot: I haz the power, 2017-02-24 19:50:08]

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-02-24 20:15:01]

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

[commit-bot: I haz the power, 2017-02-24 20:15:02]

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

[alito, 2017-02-24 21:09:37]

The CQ bit was checked by alito@chromium.org

[commit-bot: I haz the power, 2017-02-24 21:11:09]

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-02-24 22:53:11]

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

[commit-bot: I haz the power, 2017-02-24 22:53:12]

Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[alito, 2017-02-24 23:07:46]

The CQ bit was checked by alito@chromium.org

[alito, 2017-02-24 23:07:47]

The patchset sent to the CQ was uploaded after l-g-t-m from
robertshield@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2701313002/#ps100001
(title: "Fix member initializer list order in constructor")

[commit-bot: I haz the power, 2017-02-24 23:08: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-02-25 00:46:21]

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

[commit-bot: I haz the power, 2017-02-25 00:46:22]

Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[alito, 2017-02-25 01:14:42]

The CQ bit was checked by alito@chromium.org

[alito, 2017-02-25 01:14:43]

The patchset sent to the CQ was uploaded after l-g-t-m from
robertshield@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2701313002/#ps120001
(title: "Fix constness in mock test class")

[commit-bot: I haz the power, 2017-02-25 01:14: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-02-25 02:33:11]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1487985282605740,
"parent_rev": "88bcaa73ab7e95eaa4fa3ff8c893cda7383bb26c", "commit_rev":
"c61da3e4d5f324c493728c5cc2f56c8dc962817d"}

[commit-bot: I haz the power, 2017-02-25 02:34:10]

Description was changed from

==========
Adds a modal dialog implementation of the settings reset prompt.

The settings reset prompt will ask users whose settings may have been
modified by unwanted software or extensions if they want to reset some
of their settings.

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

to

==========
Adds a modal dialog implementation of the settings reset prompt.

The settings reset prompt will ask users whose settings may have been
modified by unwanted software or extensions if they want to reset some
of their settings.

BUG=

Review-Url: https://codereview.chromium.org/2701313002
Cr-Commit-Position: refs/heads/master@{#453048}
Committed:
https://chromium.googlesource.com/chromium/src/+/c61da3e4d5f324c493728c5cc2f5...
==========

[commit-bot: I haz the power, 2017-02-25 02:34:12]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/c61da3e4d5f324c493728c5cc2f5...