Supervised user interstitial: Don't destroy WebContents that might still be in use

chrome/browser/supervised_user/supervised_user_interstitial.cc

Index: chrome/browser/supervised_user/supervised_user_interstitial.cc
diff --git a/chrome/browser/supervised_user/supervised_user_interstitial.cc b/chrome/browser/supervised_user/supervised_user_interstitial.cc
index a87ed8cd127aed77d99c486199fbe8bc3f400ed8..e7888ffeb91995e5b0e447658d09bfc0a088c703 100644
--- a/chrome/browser/supervised_user/supervised_user_interstitial.cc
+++ b/chrome/browser/supervised_user/supervised_user_interstitial.cc
@@ -4,6 +4,7 @@
 
 #include "chrome/browser/supervised_user/supervised_user_interstitial.h"
 
+#include "base/memory/weak_ptr.h"
 #include "base/metrics/histogram.h"
 #include "base/prefs/pref_service.h"
 #include "base/strings/string_number_conversions.h"
@@ -23,6 +24,7 @@
 #include "content/public/browser/navigation_details.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/browser/web_contents_user_data.h"
 #include "content/public/browser/web_ui.h"
 #include "grit/browser_resources.h"
 #include "ui/base/l10n/l10n_util.h"
@@ -30,7 +32,13 @@
 #include "ui/base/webui/jstemplate_builder.h"
 #include "ui/base/webui/web_ui_util.h"
 
+#if !defined(OS_ANDROID)
+#include "chrome/browser/ui/browser_finder.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#endif
+
 using content::BrowserThread;
+using content::WebContents;
 
 namespace {
 
@@ -45,11 +53,52 @@ std::string BuildAvatarImageUrl(const std::string& url, int size) {
   return result;
 }
 
-} // namespace
+class TabCloser : public content::WebContentsUserData<TabCloser> {
+  // To use, call TabCloser::CreateForWebContents.
+ private:
+  friend class content::WebContentsUserData<TabCloser>;
+
+  explicit TabCloser(WebContents* web_contents)
+      : web_contents_(web_contents), weak_ptr_factory_(this) {
+    BrowserThread::PostTask(
+        BrowserThread::UI,
+        FROM_HERE,
+        base::Bind(&TabCloser::CloseTabImpl, weak_ptr_factory_.GetWeakPtr()));
+  }
+  virtual ~TabCloser() {}
+
+  void CloseTabImpl() {
+#if defined(OS_ANDROID)
+    // On Android, FindBrowserWithWebContents and TabStripModel don't exist.
+    // For now, just close the WebContents directly.
+    web_contents_->Close();
+#else
+    Browser* browser = chrome::FindBrowserWithWebContents(web_contents_);
+    DCHECK(browser);
+    TabStripModel* tab_strip = browser->tab_strip_model();
+    if (tab_strip->count() > 1) {
+      int index = tab_strip->GetIndexOfWebContents(web_contents_);
+      DCHECK_NE(TabStripModel::kNoTab, index);
+      tab_strip->CloseWebContentsAt(

[Bernhard Bauer, 2014/09/09 14:12:20]

Do we actually need to do it this way, or could we also call
web_contents_->Close()?

[Marc Treib, 2014/09/09 14:28:10]

Turns out that web_contents_->Close() ends up calling the tab strip anyway (via
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...),
so it doesn't really matter. But we still need to get the tab strip to check if
there's more than one tab, so.. meh. Oh well, it keeps the difference between
Android and non-Android a bit smaller, so, done.

[Bernhard Bauer, 2014/09/09 14:31:51]

Yeah, exactly.

+          index, TabStripModel::CLOSE_CREATE_HISTORICAL_TAB);
+    } else {
+      // Don't close the last tab in the window.
+      web_contents_->RemoveUserData(UserDataKey());
+    }
+#endif
+  }
+
+  WebContents* web_contents_;
+  base::WeakPtrFactory<TabCloser> weak_ptr_factory_;
+};
+
+}  // namespace
+
+DEFINE_WEB_CONTENTS_USER_DATA_KEY(TabCloser);
 
 // static
 void SupervisedUserInterstitial::Show(
-    content::WebContents* web_contents,
+    WebContents* web_contents,
     const GURL& url,
     const base::Callback<void(bool)>& callback) {
   SupervisedUserInterstitial* interstitial =
@@ -62,7 +111,7 @@ void SupervisedUserInterstitial::Show(
 }
 
 SupervisedUserInterstitial::SupervisedUserInterstitial(
-    content::WebContents* web_contents,
+    WebContents* web_contents,
     const GURL& url,
     const base::Callback<void(bool)>& callback)
     : web_contents_(web_contents),
@@ -203,6 +252,12 @@ void SupervisedUserInterstitial::CommandReceived(const std::string& command) {
     UMA_HISTOGRAM_ENUMERATION("ManagedMode.BlockingInterstitialCommand",
                               BACK,
                               HISTOGRAM_BOUNDING_VALUE);
+
+    // Close the tab if there is no history entry to go back to.
+    DCHECK(web_contents_->GetController().GetTransientEntry());
+    if (web_contents_->GetController().GetEntryCount() == 1)
+      TabCloser::CreateForWebContents(web_contents_);
+
     interstitial_page_->DontProceed();
     return;
   }
@@ -254,11 +309,6 @@ void SupervisedUserInterstitial::OnFilteringPrefsChanged() {
 
 void SupervisedUserInterstitial::DispatchContinueRequest(
     bool continue_request) {
-  // If there is no history entry to go back to, close the tab instead.
-  int nav_entry_count = web_contents_->GetController().GetEntryCount();
-  if (!continue_request && nav_entry_count == 0)
-    web_contents_->Close();
-
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE, base::Bind(callback_, continue_request));
 }