If force-sign-in policy is enabled, popup warning dialog before window closing if auth token becom…

chrome/browser/ui/views/profiles/force_signout_dialog.h

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_UI_VIEWS_PROFILES_FORCE_SIGNOUT_DIALOG_H_
+#define CHROME_BROWSER_UI_VIEWS_PROFILES_FORCE_SIGNOUT_DIALOG_H_
+
+#include <memory>
+#include <string>
+
+#include "base/callback.h"
+#include "base/macros.h"
+#include "ui/views/controls/button/button.h"
+#include "ui/views/window/dialog_delegate.h"
+
+namespace base {
+class RepeatingTimer;
+class TimeDelta;
+class Timer;
+}  // namespace base
+
+class Browser;
+class Profile;
+class SigninManager;
+
+// A modal dialog that displays a warning message of the auth failure
+// before signing the user out and closing browser windows.
+class ForceSignoutDialog : public views::DialogDelegateView {

[sky, 2017/06/05 23:43:18]

WDYT of naming this ReauthenticateDialog?

[zmin, 2017/06/06 21:00:44]

There're lots of different situations requires reauth, how about
ForceReauthenticateDialog

[sky, 2017/06/06 23:10:39]

ForcedReauthenticationDialog?

+ public:
+  // Show the dialog for |browser|, call signout_callback after the dialog is

[sky, 2017/06/05 23:43:19]

there is no signout_callback.

[zmin, 2017/06/06 21:00:43]

Done.

+  // confirmed or canceled by user.
+  ForceSignoutDialog(Browser* browser,

[sky, 2017/06/05 23:43:18]

Does this function need to be public? Don't you want to force usage of Show?

[zmin, 2017/06/06 21:00:43]

Move it to private

+                     SigninManager* signin_manager,
+                     base::Timer* close_timer);

[sky, 2017/06/05 23:43:18]

Document ownership. Also, why does the caller need to supply a timer?

[zmin, 2017/06/06 21:00:43]

The |close_timer| is used to track the remaining time mainly. 
It will also be used to 
1) Stop the timer if there is no opened browser window as we can sign out user
without delay.
2) Close the dialog if the timer is stopped

[sky, 2017/06/06 23:10:39]

I get that you want a close_timer, but why does it need to be passed in? Can't
the dialog own and creates it's own timer?

+  ~ForceSignoutDialog() override;
+
+  // Shows a warning dialog for |profile|. If there are no Browser windows
+  // associated with |profile| this signs out the profile immediately, otherwise
+  // the user clicks accept or cancel on the dialog. Sign out the profile if
+  // user confirmed the window or sign out the profile with a delay if
+  // |delay_allow| is true. Call |signout_callback| with false if user cancel

[sky, 2017/06/05 23:43:18]

There is no delay_allow or signout_callback.

[zmin, 2017/06/06 21:00:43]

Done.

+  // the dialog, otherwise call it with true.
+  static ForceSignoutDialog* ShowDialog(Profile* profile,

[sky, 2017/06/05 23:43:19]

Document who owns the return value.

[zmin, 2017/06/06 21:00:43]

Done.

+                                        SigninManager* signin_manager,
+                                        base::Timer* close_timer);
+
+  // override views::DialogDelegateView
+  bool Accept() override;
+  bool Cancel() override;
+  void WindowClosing() override;
+  base::string16 GetWindowTitle() const override;
+  base::string16 GetDialogButtonLabel(ui::DialogButton button) const override;
+  ui::ModalType GetModalType() const override;
+
+  // override views::View
+  void AddedToWidget() override;
+
+ private:
+  void OnCountDown();
+  base::TimeDelta GetCountDownRemainTime() const;

[sky, 2017/06/05 23:43:18]

remaining?
optional: rename this GetTimeRemaining().

[zmin, 2017/06/06 21:00:44]

Done.

+
+  Browser* browser_;
+  SigninManager* signin_manager_;
+  base::Timer* close_timer_;
+  base::RepeatingTimer refresh_timer_;

[sky, 2017/06/05 23:43:18]

It's not obvious why you have two timers and is worth a comment.

[zmin, 2017/06/06 21:00:43]

Done.

+
+  DISALLOW_COPY_AND_ASSIGN(ForceSignoutDialog);
+};
+
+#endif  // CHROME_BROWSER_UI_VIEWS_PROFILES_FORCE_SIGNOUT_DIALOG_H_