Commit 9839f18b authored by Tim Volodine's avatar Tim Volodine Committed by Commit Bot

[WebLayer] Implement safebrowsing shutdown

Implement safebrowsing shutdown when the browser shuts down
(i.e. when last instance of BrowserImpl is destroyed).

This ensures proper cleanup of the safebrowsing service,
pending requests and the corresponding remote safe browsing
database manager.

In particular in this patch:
- move safe_browsing_service ownership to browser_process
- shutdown safebrowsing on destruction of last BrowserImpl
- add code to count live BrowserImpl instances (corresponds
  to BrowserImpl.Observer on the java side)
- add checks and make sure to stop the safebrowsing
  database manager on IO thread.

BUG=1071533,1015418

Change-Id: I39ad06a489df20454f2970db287b1f11b81ded1e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2132448
Commit-Queue: Tim Volodine <timvolodine@chromium.org>
Reviewed-by: default avatarRichard Coles <torne@chromium.org>
Reviewed-by: default avatarJohn Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#762004}
parent aff3febd
...@@ -26,6 +26,7 @@ ...@@ -26,6 +26,7 @@
#include "base/android/jni_array.h" #include "base/android/jni_array.h"
#include "base/android/jni_string.h" #include "base/android/jni_string.h"
#include "base/json/json_writer.h" #include "base/json/json_writer.h"
#include "weblayer/browser/browser_process.h"
#include "weblayer/browser/java/jni/BrowserImpl_jni.h" #include "weblayer/browser/java/jni/BrowserImpl_jni.h"
#endif #endif
...@@ -37,6 +38,9 @@ using base::android::ScopedJavaLocalRef; ...@@ -37,6 +38,9 @@ using base::android::ScopedJavaLocalRef;
namespace weblayer { namespace weblayer {
// TODO(timvolodine): consider using an observer for this, crbug.com/1068713.
int BrowserImpl::browser_count_ = 0;
std::unique_ptr<Browser> Browser::Create( std::unique_ptr<Browser> Browser::Create(
Profile* profile, Profile* profile,
const PersistenceInfo* persistence_info) { const PersistenceInfo* persistence_info) {
...@@ -59,6 +63,14 @@ BrowserImpl::~BrowserImpl() { ...@@ -59,6 +63,14 @@ BrowserImpl::~BrowserImpl() {
RemoveTab(tabs_.back().get()); RemoveTab(tabs_.back().get());
#endif #endif
profile_->DecrementBrowserImplCount(); profile_->DecrementBrowserImplCount();
browser_count_--;
DCHECK(browser_count_ >= 0);
#if defined(OS_ANDROID)
if (browser_count_ == 0) {
weblayer::BrowserProcess::GetInstance()->StopSafeBrowsingService();
}
#endif
} }
TabImpl* BrowserImpl::CreateTabForSessionRestore( TabImpl* BrowserImpl::CreateTabForSessionRestore(
...@@ -312,6 +324,7 @@ void BrowserImpl::RemoveObserver(BrowserObserver* observer) { ...@@ -312,6 +324,7 @@ void BrowserImpl::RemoveObserver(BrowserObserver* observer) {
BrowserImpl::BrowserImpl(ProfileImpl* profile) : profile_(profile) { BrowserImpl::BrowserImpl(ProfileImpl* profile) : profile_(profile) {
profile_->IncrementBrowserImplCount(); profile_->IncrementBrowserImplCount();
browser_count_++;
} }
void BrowserImpl::RestoreStateIfNecessary( void BrowserImpl::RestoreStateIfNecessary(
......
...@@ -143,6 +143,7 @@ class BrowserImpl : public Browser { ...@@ -143,6 +143,7 @@ class BrowserImpl : public Browser {
std::string persistence_id_; std::string persistence_id_;
std::unique_ptr<BrowserPersister> browser_persister_; std::unique_ptr<BrowserPersister> browser_persister_;
base::OnceClosure visible_security_state_changed_callback_for_tests_; base::OnceClosure visible_security_state_changed_callback_for_tests_;
static int browser_count_;
}; };
} // namespace weblayer } // namespace weblayer
......
...@@ -22,6 +22,10 @@ ...@@ -22,6 +22,10 @@
#include "weblayer/browser/system_network_context_manager.h" #include "weblayer/browser/system_network_context_manager.h"
#include "weblayer/common/weblayer_paths.h" #include "weblayer/common/weblayer_paths.h"
#if defined(OS_ANDROID)
#include "weblayer/browser/safe_browsing/safe_browsing_service.h"
#endif
namespace weblayer { namespace weblayer {
namespace { namespace {
...@@ -121,4 +125,23 @@ void BrowserProcess::CreateNetworkQualityObserver() { ...@@ -121,4 +125,23 @@ void BrowserProcess::CreateNetworkQualityObserver() {
DCHECK(network_quality_observer_); DCHECK(network_quality_observer_);
} }
#if defined(OS_ANDROID)
SafeBrowsingService* BrowserProcess::GetSafeBrowsingService(
std::string user_agent) {
if (!safe_browsing_service_) {
// Create and initialize safe_browsing_service on first get.
// Note: Initialize() needs to happen on UI thread.
safe_browsing_service_ = std::make_unique<SafeBrowsingService>(user_agent);
safe_browsing_service_->Initialize();
}
return safe_browsing_service_.get();
}
void BrowserProcess::StopSafeBrowsingService() {
if (safe_browsing_service_) {
safe_browsing_service_->StopDBManager();
}
}
#endif
} // namespace weblayer } // namespace weblayer
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/scoped_refptr.h" #include "base/memory/scoped_refptr.h"
#include "base/sequence_checker.h" #include "base/sequence_checker.h"
#include "build/build_config.h"
#include "services/network/public/cpp/network_quality_tracker.h" #include "services/network/public/cpp/network_quality_tracker.h"
class PrefRegistrySimple; class PrefRegistrySimple;
...@@ -24,6 +25,7 @@ class SharedURLLoaderFactory; ...@@ -24,6 +25,7 @@ class SharedURLLoaderFactory;
} }
namespace weblayer { namespace weblayer {
class SafeBrowsingService;
// Class that holds global state in the browser process. Should be used only on // Class that holds global state in the browser process. Should be used only on
// the UI thread. // the UI thread.
...@@ -47,6 +49,11 @@ class BrowserProcess { ...@@ -47,6 +49,11 @@ class BrowserProcess {
network_time::NetworkTimeTracker* GetNetworkTimeTracker(); network_time::NetworkTimeTracker* GetNetworkTimeTracker();
network::NetworkQualityTracker* GetNetworkQualityTracker(); network::NetworkQualityTracker* GetNetworkQualityTracker();
#if defined(OS_ANDROID)
SafeBrowsingService* GetSafeBrowsingService(std::string user_agent);
void StopSafeBrowsingService();
#endif
private: private:
void RegisterPrefs(PrefRegistrySimple* pref_registry); void RegisterPrefs(PrefRegistrySimple* pref_registry);
void CreateNetworkQualityObserver(); void CreateNetworkQualityObserver();
...@@ -61,6 +68,10 @@ class BrowserProcess { ...@@ -61,6 +68,10 @@ class BrowserProcess {
network::NetworkQualityTracker::RTTAndThroughputEstimatesObserver> network::NetworkQualityTracker::RTTAndThroughputEstimatesObserver>
network_quality_observer_; network_quality_observer_;
#if defined(OS_ANDROID)
std::unique_ptr<SafeBrowsingService> safe_browsing_service_;
#endif
SEQUENCE_CHECKER(sequence_checker_); SEQUENCE_CHECKER(sequence_checker_);
DISALLOW_COPY_AND_ASSIGN(BrowserProcess); DISALLOW_COPY_AND_ASSIGN(BrowserProcess);
......
...@@ -586,14 +586,8 @@ void ContentBrowserClientImpl::CreateFeatureListAndFieldTrials() { ...@@ -586,14 +586,8 @@ void ContentBrowserClientImpl::CreateFeatureListAndFieldTrials() {
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
SafeBrowsingService* ContentBrowserClientImpl::GetSafeBrowsingService() { SafeBrowsingService* ContentBrowserClientImpl::GetSafeBrowsingService() {
if (!safe_browsing_service_) { return weblayer::BrowserProcess::GetInstance()->GetSafeBrowsingService(
// Create and initialize safe_browsing_service on first get. GetUserAgent());
// Note: Initialize() needs to happen on UI thread.
safe_browsing_service_ =
std::make_unique<SafeBrowsingService>(GetUserAgent());
safe_browsing_service_->Initialize();
}
return safe_browsing_service_.get();
} }
#endif #endif
......
...@@ -116,10 +116,6 @@ class ContentBrowserClientImpl : public content::ContentBrowserClient { ...@@ -116,10 +116,6 @@ class ContentBrowserClientImpl : public content::ContentBrowserClient {
MainParams* params_; MainParams* params_;
#if defined(OS_ANDROID)
std::unique_ptr<SafeBrowsingService> safe_browsing_service_;
#endif
std::unique_ptr<FeatureListCreator> feature_list_creator_; std::unique_ptr<FeatureListCreator> feature_list_creator_;
}; };
......
...@@ -200,4 +200,18 @@ void SafeBrowsingService::AddInterface( ...@@ -200,4 +200,18 @@ void SafeBrowsingService::AddInterface(
base::CreateSingleThreadTaskRunner({content::BrowserThread::UI})); base::CreateSingleThreadTaskRunner({content::BrowserThread::UI}));
} }
void SafeBrowsingService::StopDBManager() {
base::PostTask(FROM_HERE, {content::BrowserThread::IO},
base::BindOnce(&SafeBrowsingService::StopDBManagerOnIOThread,
base::Unretained(this)));
}
void SafeBrowsingService::StopDBManagerOnIOThread() {
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
if (safe_browsing_db_manager_) {
safe_browsing_db_manager_->StopOnIOThread(true /*shutdown*/);
safe_browsing_db_manager_.reset();
}
}
} // namespace weblayer } // namespace weblayer
...@@ -53,6 +53,7 @@ class SafeBrowsingService { ...@@ -53,6 +53,7 @@ class SafeBrowsingService {
CreateSafeBrowsingNavigationThrottle(content::NavigationHandle* handle); CreateSafeBrowsingNavigationThrottle(content::NavigationHandle* handle);
void AddInterface(service_manager::BinderRegistry* registry, void AddInterface(service_manager::BinderRegistry* registry,
content::RenderProcessHost* render_process_host); content::RenderProcessHost* render_process_host);
void StopDBManager();
private: private:
SafeBrowsingUIManager* GetSafeBrowsingUIManager(); SafeBrowsingUIManager* GetSafeBrowsingUIManager();
...@@ -68,6 +69,7 @@ class SafeBrowsingService { ...@@ -68,6 +69,7 @@ class SafeBrowsingService {
GetURLLoaderFactoryOnIOThread(); GetURLLoaderFactoryOnIOThread();
void CreateURLLoaderFactoryForIO( void CreateURLLoaderFactoryForIO(
mojo::PendingReceiver<network::mojom::URLLoaderFactory> receiver); mojo::PendingReceiver<network::mojom::URLLoaderFactory> receiver);
void StopDBManagerOnIOThread();
// The UI manager handles showing interstitials. Accessed on both UI and IO // The UI manager handles showing interstitials. Accessed on both UI and IO
// thread. // thread.
......
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