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.cc

+// 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.
+
+#include "chrome/browser/ui/views/profiles/force_signout_dialog.h"
+
+#include <string>
+
+#include "base/strings/string16.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/signin/force_signout_timer.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_list.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/views/profiles/force_signout_dialog_view.h"
+#include "chrome/grit/generated_resources.h"
+#include "components/app_modal/app_modal_dialog_queue.h"
+#include "components/signin/core/browser/signin_manager.h"
+#include "ui/base/l10n/l10n_util.h"
+
+namespace {
+
+void Signout(SigninManager* signin_manager) {
+  signin_manager->SignOut(
+      signin_metrics::AUTHENTICATION_FAILED_WITH_FORCE_SIGNIN,
+      signin_metrics::SignoutDelete::KEEPING);
+}
+
+content::WebContents* GetWebContentsFromBrowser(Browser* browser) {
+  content::WebContents* web_contents =
+      browser->tab_strip_model()->GetActiveWebContents();
+  if (web_contents == nullptr)

[sky, 2017/05/04 03:45:11]

This should be a DCHECK. You earlier check the size, an the only way there is no
active web contents is if there are no tabs.

[zmin, 2017/05/04 23:13:52]

Done.

+    return browser->tab_strip_model()->GetWebContentsAt(0);
+  else
+    return web_contents;
+}
+
+bool IsBrowserBelongedToProfile(Browser* browser, Profile* profile) {

[sky, 2017/05/04 03:45:11]

How about naming this IsMatchingBrowser and include the tab_strip_model check
here too? Two questions on the implementation:
. Do you really want to incognito windows here? It seems weird to anchor to an
incognito window?
. Are you sure you don't want to look at the type? Anchoring to popups could be
weird.
. Don't you want to check visibility of the window?

[zmin, 2017/05/04 23:13:52]

Done.
Two questions on the implementation:
The purpose the dialog is warning the window closing. So
Visibility, yes I'll check the visibility. it seems no harm to close if all
windows are invisible.
Incognito and popups. I agree it's kinda weird. But since they will be closed
later as well, I think it make sense to show the warning msg before.

[sky, 2017/05/05 00:00:10]

Be careful there. You should verify you get the right thing if the window is
minimized. I would say in that case you should force the window to be shown and
then show the dialog.

+  return browser->profile()->GetOriginalProfile() ==
+         profile->GetOriginalProfile();
+}
+
+// Find a browser that is assoicated with |profile| to show the dialog for
+// Sign out warning.
+Browser* FindBrowserWithProfile(Profile* profile) {
+  Browser* browser = BrowserList::GetInstance()->GetLastActive();
+  if (browser && IsBrowserBelongedToProfile(browser, profile) &&
+      !browser->tab_strip_model()->empty()) {
+    return browser;
+  }
+  for (auto* browser : *BrowserList::GetInstance()) {
+    if (IsBrowserBelongedToProfile(browser, profile) &&
+        !browser->tab_strip_model()->empty()) {
+      return browser;
+    }
+  }
+  return nullptr;
+}
+
+}  // namespace
+
+// static
+void ForceSignoutDialog::ShowDialog(Profile* profile,
+                                    SigninManager* signin_manager,
+                                    ForceSignoutTimer* signout_timer) {
+  Browser* browser = FindBrowserWithProfile(profile);
+  if (browser == nullptr) {  // If there is no browser, we can just sign
+                             // out profile directly.
+    Signout(signin_manager);
+    return;
+  }
+  app_modal::AppModalDialogQueue::GetInstance()->AddDialog(
+      new ForceSignoutDialog(browser, signin_manager, signout_timer));
+}
+
+app_modal::NativeAppModalDialog* ForceSignoutDialog::CreateNativeDialog() {
+  return new ForceSignoutDialogView(browser_, this);
+}
+
+void ForceSignoutDialog::OnAccept() {
+  CompleteDialog();
+  Signout(signin_manager_);
+}
+void ForceSignoutDialog::OnCancel() {
+  CompleteDialog();
+  signout_timer_->Start();
+}
+
+std::string ForceSignoutDialog::GetEmail() {
+  return signin_manager_->GetAuthenticatedAccountInfo().email;
+}
+
+bool ForceSignoutDialog::IsDelayAllowed() {
+  return signout_timer_ != nullptr;
+}
+
+ForceSignoutDialog::ForceSignoutDialog(Browser* browser,
+                                       SigninManager* signin_manager,
+                                       ForceSignoutTimer* signout_timer)
+    : app_modal::AppModalDialog(

[sky, 2017/05/04 03:45:11]

Why are you using app_modal_dialog here? I understand that you want app modal,
but you needn't use the AppModal class for that. Using the Appmodal class ties
you to WebContents, which I'm not sure you want.

[zmin, 2017/05/04 23:13:52]

No, I don't want to be tied to WebContents. However, what i want is this:
https://cs.chromium.org/chromium/src/chrome/browser/ui/views/frame/browser_vi...
It prevent user open other browser window when the dialog is being displayed.
And right now I only found AppModalDialog class can do so with
AppModalDialogQueue. User can still use other chrome window when a normal window
modal dialog is opened.

[sky, 2017/05/05 00:00:10]

Also, what happens if the browser you are anchored to ends up closing? This
could happen for a number of reasons.

[sky, 2017/05/05 00:00:10]

Also, user could still interact with non-browser windows in Chrome. Are you sure
you want that?

+          GetWebContentsFromBrowser(browser),
+          l10n_util::GetStringUTF16(IDS_ENTERPRISE_FORCE_SIGNOUT_TITLE)),
+      browser_(browser),
+      signin_manager_(signin_manager),
+      signout_timer_(signout_timer) {}