Commit e7ccb7eb authored by treib's avatar treib Committed by Commit bot

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

Before, the WebContents was closed directly from OnDontProceed. This breaks if the caller expects the WebContents to survive (happened e.g. when a new interstitial was being initialized).
This CL instead posts a task to the message loop.
Also, the tab won't be closed anymore if it's the last one in the window, as that would lead to closing the window.

BUG=411841

Review URL: https://codereview.chromium.org/550973002

Cr-Commit-Position: refs/heads/master@{#293985}
parent ef3bf565
...@@ -214,9 +214,12 @@ IN_PROC_BROWSER_TEST_F(SupervisedUserBlockModeTest, OpenBlockedURLInNewTab) { ...@@ -214,9 +214,12 @@ IN_PROC_BROWSER_TEST_F(SupervisedUserBlockModeTest, OpenBlockedURLInNewTab) {
// On pressing the "back" button, the new tab should be closed, and we should // On pressing the "back" button, the new tab should be closed, and we should
// get back to the previous active tab. // get back to the previous active tab.
MockTabStripModelObserver observer(tab_strip); MockTabStripModelObserver observer(tab_strip);
base::RunLoop run_loop;
EXPECT_CALL(observer, EXPECT_CALL(observer,
TabClosingAt(tab_strip, tab, tab_strip->active_index())); TabClosingAt(tab_strip, tab, tab_strip->active_index()))
.WillOnce(testing::InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit));
GoBack(tab); GoBack(tab);
run_loop.Run();
EXPECT_EQ(prev_tab, tab_strip->GetActiveWebContents()); EXPECT_EQ(prev_tab, tab_strip->GetActiveWebContents());
} }
......
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
#include "chrome/browser/supervised_user/supervised_user_interstitial.h" #include "chrome/browser/supervised_user/supervised_user_interstitial.h"
#include "base/memory/weak_ptr.h"
#include "base/metrics/histogram.h" #include "base/metrics/histogram.h"
#include "base/prefs/pref_service.h" #include "base/prefs/pref_service.h"
#include "base/strings/string_number_conversions.h" #include "base/strings/string_number_conversions.h"
...@@ -23,6 +24,7 @@ ...@@ -23,6 +24,7 @@
#include "content/public/browser/navigation_details.h" #include "content/public/browser/navigation_details.h"
#include "content/public/browser/navigation_entry.h" #include "content/public/browser/navigation_entry.h"
#include "content/public/browser/web_contents.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 "content/public/browser/web_ui.h"
#include "grit/browser_resources.h" #include "grit/browser_resources.h"
#include "ui/base/l10n/l10n_util.h" #include "ui/base/l10n/l10n_util.h"
...@@ -30,7 +32,13 @@ ...@@ -30,7 +32,13 @@
#include "ui/base/webui/jstemplate_builder.h" #include "ui/base/webui/jstemplate_builder.h"
#include "ui/base/webui/web_ui_util.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::BrowserThread;
using content::WebContents;
namespace { namespace {
...@@ -45,11 +53,48 @@ std::string BuildAvatarImageUrl(const std::string& url, int size) { ...@@ -45,11 +53,48 @@ std::string BuildAvatarImageUrl(const std::string& url, int size) {
return result; 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() {
// On Android, FindBrowserWithWebContents and TabStripModel don't exist.
#if !defined(OS_ANDROID)
Browser* browser = chrome::FindBrowserWithWebContents(web_contents_);
DCHECK(browser);
TabStripModel* tab_strip = browser->tab_strip_model();
DCHECK_NE(TabStripModel::kNoTab,
tab_strip->GetIndexOfWebContents(web_contents_));
if (tab_strip->count() <= 1) {
// Don't close the last tab in the window.
web_contents_->RemoveUserData(UserDataKey());
return;
}
#endif
web_contents_->Close();
}
WebContents* web_contents_;
base::WeakPtrFactory<TabCloser> weak_ptr_factory_;
};
} // namespace
DEFINE_WEB_CONTENTS_USER_DATA_KEY(TabCloser);
// static // static
void SupervisedUserInterstitial::Show( void SupervisedUserInterstitial::Show(
content::WebContents* web_contents, WebContents* web_contents,
const GURL& url, const GURL& url,
const base::Callback<void(bool)>& callback) { const base::Callback<void(bool)>& callback) {
SupervisedUserInterstitial* interstitial = SupervisedUserInterstitial* interstitial =
...@@ -62,7 +107,7 @@ void SupervisedUserInterstitial::Show( ...@@ -62,7 +107,7 @@ void SupervisedUserInterstitial::Show(
} }
SupervisedUserInterstitial::SupervisedUserInterstitial( SupervisedUserInterstitial::SupervisedUserInterstitial(
content::WebContents* web_contents, WebContents* web_contents,
const GURL& url, const GURL& url,
const base::Callback<void(bool)>& callback) const base::Callback<void(bool)>& callback)
: web_contents_(web_contents), : web_contents_(web_contents),
...@@ -203,6 +248,12 @@ void SupervisedUserInterstitial::CommandReceived(const std::string& command) { ...@@ -203,6 +248,12 @@ void SupervisedUserInterstitial::CommandReceived(const std::string& command) {
UMA_HISTOGRAM_ENUMERATION("ManagedMode.BlockingInterstitialCommand", UMA_HISTOGRAM_ENUMERATION("ManagedMode.BlockingInterstitialCommand",
BACK, BACK,
HISTOGRAM_BOUNDING_VALUE); 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(); interstitial_page_->DontProceed();
return; return;
} }
...@@ -254,11 +305,6 @@ void SupervisedUserInterstitial::OnFilteringPrefsChanged() { ...@@ -254,11 +305,6 @@ void SupervisedUserInterstitial::OnFilteringPrefsChanged() {
void SupervisedUserInterstitial::DispatchContinueRequest( void SupervisedUserInterstitial::DispatchContinueRequest(
bool continue_request) { 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::PostTask(
BrowserThread::IO, FROM_HERE, base::Bind(callback_, continue_request)); BrowserThread::IO, FROM_HERE, base::Bind(callback_, continue_request));
} }
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment