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

chrome/browser/supervised_user/supervised_user_interstitial.cc

 // Copyright 2014 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/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"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/infobars/infobar_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/supervised_user/supervised_user_service.h"
 #include "chrome/browser/supervised_user/supervised_user_service_factory.h"
+#include "chrome/browser/ui/browser_finder.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/infobars/core/infobar.h"
 #include "components/infobars/core/infobar_delegate.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/interstitial_page.h"
 #include "content/public/browser/navigation_controller.h"
 #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"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/base/webui/jstemplate_builder.h"
 #include "ui/base/webui/web_ui_util.h"
 
 using content::BrowserThread;
+using content::WebContents;
 
 namespace {
 
 static const int kAvatarSize1x = 45;
 static const int kAvatarSize2x = 90;
 
 std::string BuildAvatarImageUrl(const std::string& url, int size) {
   std::string result = url;
   size_t slash = result.rfind('/');
   if (slash != std::string::npos)
     result.insert(slash, "/s" + base::IntToString(size));
   return result;
 }
 
-} // namespace
+class TabCloser : public content::WebContentsUserData<TabCloser> {
+ public:
+  static void CloseTab(WebContents* web_contents) {
+    CreateForWebContents(web_contents);
+  }
+
+ 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() {
+    Browser* browser = chrome::FindBrowserWithWebContents(web_contents_);
+    DCHECK(browser);
+    TabStripModel* tab_strip = browser->tab_strip_model();
+    // Don't close the last tab in the window.
+    if (tab_strip->count() > 1) {
+      int index = tab_strip->GetIndexOfWebContents(web_contents_);
+      DCHECK_NE(TabStripModel::kNoTab, index);
+      tab_strip->CloseWebContentsAt(
+          index, TabStripModel::CLOSE_CREATE_HISTORICAL_TAB);
+    } else {
+      web_contents_->RemoveUserData(UserDataKey());
+    }
+  }
+
+  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 =
       new SupervisedUserInterstitial(web_contents, url, callback);
 
   // If Init() does not complete fully, immediately delete the interstitial.
   if (!interstitial->Init())
     delete interstitial;
   // Otherwise |interstitial_page_| is responsible for deleting it.
 }
 
 SupervisedUserInterstitial::SupervisedUserInterstitial(
-    content::WebContents* web_contents,
+    WebContents* web_contents,
     const GURL& url,
     const base::Callback<void(bool)>& callback)
     : web_contents_(web_contents),
       interstitial_page_(NULL),
       url_(url),
-      callback_(callback) {}
+      callback_(callback),
+      back_button_pressed_(false) {}
 
 SupervisedUserInterstitial::~SupervisedUserInterstitial() {}
 
 bool SupervisedUserInterstitial::Init() {
   if (ShouldProceed()) {
     // It can happen that the site was only allowed very recently and the URL
     // filter on the IO thread had not been updated yet. Proceed with the
     // request without showing the interstitial.
     DispatchContinueRequest(true);
     return false;
@@ skipping 114 matching lines @@
     BACK,
     NTP,
     ACCESS_REQUEST,
     HISTOGRAM_BOUNDING_VALUE
   };
 
   if (command == "\"back\"") {
     UMA_HISTOGRAM_ENUMERATION("ManagedMode.BlockingInterstitialCommand",
                               BACK,
                               HISTOGRAM_BOUNDING_VALUE);
+    back_button_pressed_ = true;
     interstitial_page_->DontProceed();
     return;
   }
 
   if (command == "\"request\"") {
     UMA_HISTOGRAM_ENUMERATION("ManagedMode.BlockingInterstitialCommand",
                               ACCESS_REQUEST,
                               HISTOGRAM_BOUNDING_VALUE);
 
     Profile* profile =
@@ skipping 31 matching lines @@
          SupervisedUserURLFilter::BLOCK;
 }
 
 void SupervisedUserInterstitial::OnFilteringPrefsChanged() {
   if (ShouldProceed())
     interstitial_page_->Proceed();
 }
 
 void SupervisedUserInterstitial::DispatchContinueRequest(
     bool continue_request) {
-  // If there is no history entry to go back to, close the tab instead.
+  // On "go back", close the tab if there is no history entry to go back to.
   int nav_entry_count = web_contents_->GetController().GetEntryCount();
-  if (!continue_request && nav_entry_count == 0)
-    web_contents_->Close();
+  if (back_button_pressed_ && !continue_request && nav_entry_count == 0)

[Bernhard Bauer, 2014/09/09 11:32:31]

Just call this directly from CommandReceived(). If you feel that's too ugly,
wrap it in a method OnBackSelected or something.

[Marc Treib, 2014/09/09 11:51:25]

Right, that makes more sense now.
Small wrinkle: At this point, the transient navigation entry is still around, so
I have to check for count==1 instead of count==0.

+    TabCloser::CloseTab(web_contents_);
 
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE, base::Bind(callback_, continue_request));
 }