DCHECK(browser_context) in most methods of ContentBrowserClient.

ContentBrowserClients assume that |browser_context| client is not null
(e.g. HeadlessContentBrowserClient::DoesSiteRequireDedicatedProcess and
ChromeContentBrowserClientExtensionsPart::DoesSiteRequireDedicatedProcess).
To avoid surprises, this CL adds DCHECK(browser_context) to most methods
of ContentBrowserClients, so that the assumption is documented and
verified during //content-layer tests.

Bug: 787576
Change-Id: I115ab53bb64cebe50b5993a39b3e153e3054b1b3
Reviewed-on: https://chromium-review.googlesource.com/c/1276559Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599752}

content/browser/browsing_instance.cc

@@ -18,6 +18,7 @@ namespace content {
 BrowsingInstance::BrowsingInstance(BrowserContext* browser_context)
     : browser_context_(browser_context),
       active_contents_count_(0u) {
+  DCHECK(browser_context);
 }
 
 bool BrowsingInstance::HasSiteInstance(const GURL& url) {

content/browser/child_process_security_policy_impl.cc

@@ -19,6 +19,7 @@
 #include "build/build_config.h"
 #include "content/browser/bad_message.h"
 #include "content/browser/isolated_origin_util.h"
+#include "content/browser/renderer_host/render_process_host_impl.h"
 #include "content/browser/site_instance_impl.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
@@ -1174,8 +1175,16 @@ void ChildProcessSecurityPolicyImpl::LockToOrigin(int child_id,
   // call GetOriginLock or CheckOriginLock from any thread).
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  // "gurl" can be currently empty in some cases, such as file://blah.
-  DCHECK_EQ(SiteInstanceImpl::DetermineProcessLockURL(nullptr, gurl), gurl);
+#if DCHECK_IS_ON()
+  // Sanity-check that the |gurl| argument can be used as a lock.
+  RenderProcessHost* rph = RenderProcessHostImpl::FromID(child_id);
+  if (rph) {  // |rph| can be null in unittests.
+    DCHECK_EQ(SiteInstanceImpl::DetermineProcessLockURL(
+                  rph->GetBrowserContext(), gurl),
+              gurl);
+  }
+#endif
+
   base::AutoLock lock(lock_);
   auto state = security_state_.find(child_id);
   DCHECK(state != security_state_.end());

content/browser/frame_host/navigation_entry_impl_unittest.cc

@@ -4,6 +4,8 @@
 
 #include "content/browser/frame_host/navigation_entry_impl.h"
 
+#include <utility>
+
 #include "base/path_service.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_util.h"
@@ -13,6 +15,8 @@
 #include "build/build_config.h"
 #include "content/browser/site_instance_impl.h"
 #include "content/public/browser/ssl_status.h"
+#include "content/public/test/test_browser_context.h"
+#include "content/public/test/test_browser_thread_bundle.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using base::ASCIIToUTF16;
@@ -54,7 +58,7 @@ class NavigationEntryTest : public testing::Test {
   void SetUp() override {
     entry1_.reset(new NavigationEntryImpl);
 
-    instance_ = SiteInstanceImpl::Create(nullptr);
+    instance_ = SiteInstanceImpl::Create(&browser_context_);
     entry2_.reset(new NavigationEntryImpl(
         instance_, GURL("test:url"),
         Referrer(GURL("from"), blink::kWebReferrerPolicyDefault),
@@ -69,6 +73,10 @@ class NavigationEntryTest : public testing::Test {
   std::unique_ptr<NavigationEntryImpl> entry2_;
   // SiteInstances are deleted when their NavigationEntries are gone.
   scoped_refptr<SiteInstanceImpl> instance_;
+
+ private:
+  TestBrowserThreadBundle thread_bundle_;
+  TestBrowserContext browser_context_;
 };
 
 // Test unique ID accessors

content/browser/service_worker/embedded_worker_instance.cc

@@ -261,6 +261,7 @@ void SetupOnUIThread(base::WeakPtr<ServiceWorkerProcessManager> process_manager,
 
   // TODO(crbug.com/862854): Support changes to RendererPreferences while the
   // worker is running.
+  DCHECK(process_manager->browser_context() || process_manager->IsShutdown());
   GetContentClient()->browser()->UpdateRendererPreferencesForWorker(
       process_manager->browser_context(), &params->renderer_preferences);
 

content/browser/service_worker/service_worker_process_manager.cc

@@ -29,6 +29,7 @@ ServiceWorkerProcessManager::ServiceWorkerProcessManager(
       new_process_id_for_test_(ChildProcessHost::kInvalidUniqueID),
       weak_this_factory_(this) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  DCHECK(browser_context);
   weak_this_ = weak_this_factory_.GetWeakPtr();
 }
 

content/browser/site_instance_impl.cc

@@ -59,6 +59,7 @@ SiteInstanceImpl::~SiteInstanceImpl() {
 // static
 scoped_refptr<SiteInstanceImpl> SiteInstanceImpl::Create(
     BrowserContext* browser_context) {
+  DCHECK(browser_context);
   return base::WrapRefCounted(
       new SiteInstanceImpl(new BrowsingInstance(browser_context)));
 }
@@ -67,6 +68,7 @@ scoped_refptr<SiteInstanceImpl> SiteInstanceImpl::Create(
 scoped_refptr<SiteInstanceImpl> SiteInstanceImpl::CreateForURL(
     BrowserContext* browser_context,
     const GURL& url) {
+  DCHECK(browser_context);
   // This will create a new SiteInstance and BrowsingInstance.
   scoped_refptr<BrowsingInstance> instance(
       new BrowsingInstance(browser_context));
@@ -314,6 +316,7 @@ BrowserContext* SiteInstanceImpl::GetBrowserContext() const {
 // static
 scoped_refptr<SiteInstance> SiteInstance::Create(
     BrowserContext* browser_context) {
+  DCHECK(browser_context);
   return SiteInstanceImpl::Create(browser_context);
 }
 
@@ -321,6 +324,7 @@ scoped_refptr<SiteInstance> SiteInstance::Create(
 scoped_refptr<SiteInstance> SiteInstance::CreateForURL(
     BrowserContext* browser_context,
     const GURL& url) {
+  DCHECK(browser_context);
   return SiteInstanceImpl::CreateForURL(browser_context, url);
 }
 
@@ -341,6 +345,8 @@ bool SiteInstanceImpl::IsSameWebSite(BrowserContext* browser_context,
                                      const GURL& real_src_url,
                                      const GURL& real_dest_url,
                                      bool should_compare_effective_urls) {
+  DCHECK(browser_context);
+
   GURL src_url =
       should_compare_effective_urls
           ? SiteInstanceImpl::GetEffectiveURL(browser_context, real_src_url)
@@ -555,6 +561,8 @@ bool SiteInstanceImpl::HasEffectiveURL(BrowserContext* browser_context,
 bool SiteInstanceImpl::DoesSiteRequireDedicatedProcess(
     BrowserContext* browser_context,
     const GURL& url) {
+  DCHECK(browser_context);
+
   // If --site-per-process is enabled, site isolation is enabled everywhere.
   if (SiteIsolationPolicy::UseDedicatedProcessesForAllSites())
     return true;
@@ -590,6 +598,8 @@ bool SiteInstanceImpl::DoesSiteRequireDedicatedProcess(
 // static
 bool SiteInstanceImpl::ShouldLockToOrigin(BrowserContext* browser_context,
                                           GURL site_url) {
+  DCHECK(browser_context);
+
   // Don't lock to origin in --single-process mode, since this mode puts
   // cross-site pages into the same process.
   if (RenderProcessHost::run_renderer_in_process())

content/public/browser/content_browser_client.cc

@@ -78,6 +78,7 @@ bool ContentBrowserClient::AllowGpuLaunchRetryOnIOThread() {
 
 GURL ContentBrowserClient::GetEffectiveURL(BrowserContext* browser_context,
                                            const GURL& url) {
+  DCHECK(browser_context);
   return url;
 }
 
@@ -87,6 +88,7 @@ bool ContentBrowserClient::ShouldCompareEffectiveURLsForSiteInstanceSelection(
     bool is_main_frame,
     const GURL& candidate_url,
     const GURL& destination_url) {
+  DCHECK(browser_context);
   return true;
 }
 
@@ -96,6 +98,7 @@ bool ContentBrowserClient::ShouldUseMobileFlingCurve() const {
 
 bool ContentBrowserClient::ShouldUseProcessPerSite(
     BrowserContext* browser_context, const GURL& effective_url) {
+  DCHECK(browser_context);
   return false;
 }
 
@@ -108,11 +111,13 @@ bool ContentBrowserClient::ShouldUseSpareRenderProcessHost(
 bool ContentBrowserClient::DoesSiteRequireDedicatedProcess(
     BrowserContext* browser_context,
     const GURL& effective_site_url) {
+  DCHECK(browser_context);
   return false;
 }
 
 bool ContentBrowserClient::ShouldLockToOrigin(BrowserContext* browser_context,
                                               const GURL& effective_url) {
+  DCHECK(browser_context);
   return true;
 }
 
@@ -165,6 +170,7 @@ bool ContentBrowserClient::ShouldAllowOpenURL(SiteInstance* site_instance,
 bool ContentBrowserClient::IsURLAcceptableForWebUI(
     BrowserContext* browser_context,
     const GURL& url) {
+  DCHECK(browser_context);
   return false;
 }
 
@@ -185,6 +191,7 @@ bool ContentBrowserClient::MayReuseHost(RenderProcessHost* process_host) {
 
 bool ContentBrowserClient::ShouldTryToUseExistingProcessHost(
       BrowserContext* browser_context, const GURL& url) {
+  DCHECK(browser_context);
   return false;
 }
 
@@ -247,6 +254,7 @@ std::string ContentBrowserClient::GetApplicationLocale() {
 }
 
 std::string ContentBrowserClient::GetAcceptLangs(BrowserContext* context) {
+  DCHECK(context);
   return std::string();
 }
 
@@ -282,16 +290,20 @@ bool ContentBrowserClient::AllowSharedWorker(
     BrowserContext* context,
     int render_process_id,
     int render_frame_id) {
+  DCHECK(context);
   return true;
 }
 
 bool ContentBrowserClient::IsDataSaverEnabled(BrowserContext* context) {
+  DCHECK(context);
   return false;
 }
 
 void ContentBrowserClient::UpdateRendererPreferencesForWorker(
     BrowserContext* browser_context,
-    RendererPreferences* out_prefs) {}
+    RendererPreferences* out_prefs) {
+  // |browser_context| may be null (e.g. during shutdown of a service worker).
+}
 
 bool ContentBrowserClient::AllowGetCookie(const GURL& url,
                                           const GURL& first_party,
@@ -346,6 +358,7 @@ ContentBrowserClient::AllowWebBluetooth(
     content::BrowserContext* browser_context,
     const url::Origin& requesting_origin,
     const url::Origin& embedding_origin) {
+  DCHECK(browser_context);
   return AllowWebBluetoothResult::ALLOW;
 }
 
@@ -361,6 +374,8 @@ void ContentBrowserClient::GetQuotaSettings(
     BrowserContext* context,
     StoragePartition* partition,
     storage::OptionalQuotaSettingsCallback callback) {
+  DCHECK(context);
+
   // By default, no quota is provided, embedders should override.
   std::move(callback).Run(storage::GetNoQuotaSettings());
 }
@@ -412,12 +427,15 @@ bool ContentBrowserClient::ShouldUseGmsCoreGeolocationProvider() {
 std::string ContentBrowserClient::GetStoragePartitionIdForSite(
     BrowserContext* browser_context,
     const GURL& site) {
+  DCHECK(browser_context);
   return std::string();
 }
 
 bool ContentBrowserClient::IsValidStoragePartitionId(
     BrowserContext* browser_context,
     const std::string& partition_id) {
+  DCHECK(browser_context);
+
   // Since the GetStoragePartitionIdForChildProcess() only generates empty
   // strings, we should only ever see empty strings coming back.
   return partition_id.empty();
@@ -430,6 +448,8 @@ void ContentBrowserClient::GetStoragePartitionConfigForSite(
     std::string* partition_domain,
     std::string* partition_name,
     bool* in_memory) {
+  DCHECK(browser_context);
+
   partition_domain->clear();
   partition_name->clear();
   *in_memory = false;
@@ -497,17 +517,20 @@ bool ContentBrowserClient::AllowPepperSocketAPI(
     const GURL& url,
     bool private_api,
     const SocketPermissionRequest* params) {
+  DCHECK(browser_context);
   return false;
 }
 
 bool ContentBrowserClient::IsPepperVpnProviderAPIAllowed(
     BrowserContext* browser_context,
     const GURL& url) {
+  DCHECK(browser_context);
   return false;
 }
 
 std::unique_ptr<VpnServiceProxy> ContentBrowserClient::GetVpnServiceProxy(
     BrowserContext* browser_context) {
+  DCHECK(browser_context);
   return nullptr;
 }
 
@@ -527,17 +550,22 @@ TracingDelegate* ContentBrowserClient::GetTracingDelegate() {
 bool ContentBrowserClient::IsPluginAllowedToCallRequestOSFileHandle(
     BrowserContext* browser_context,
     const GURL& url) {
+  DCHECK(browser_context);
   return false;
 }
 
 bool ContentBrowserClient::IsPluginAllowedToUseDevChannelAPIs(
     BrowserContext* browser_context,
     const GURL& url) {
+  // |browser_context| may be null (e.g. when called from
+  // PpapiPluginProcessHost::PpapiPluginProcessHost).
+
   return false;
 }
 
 std::string ContentBrowserClient::GetServiceUserIdForBrowserContext(
     BrowserContext* browser_context) {
+  DCHECK(browser_context);
   return base::GenerateGUID();
 }
 
@@ -564,6 +592,7 @@ void ContentBrowserClient::OpenURL(
     content::BrowserContext* browser_context,
     const content::OpenURLParams& params,
     const base::Callback<void(content::WebContents*)>& callback) {
+  DCHECK(browser_context);
   callback.Run(nullptr);
 }
 
@@ -676,6 +705,7 @@ bool ContentBrowserClient::WillCreateURLLoaderFactory(
     const url::Origin& request_initiator,
     network::mojom::URLLoaderFactoryRequest* factory_request,
     bool* bypass_redirect_checks) {
+  DCHECK(browser_context);
   return false;
 }
 
@@ -698,6 +728,7 @@ network::mojom::NetworkContextPtr ContentBrowserClient::CreateNetworkContext(
     BrowserContext* context,
     bool in_memory,
     const base::FilePath& relative_partition_path) {
+  DCHECK(context);
   if (!base::FeatureList::IsEnabled(network::features::kNetworkService))
     return nullptr;
 
@@ -746,6 +777,7 @@ bool ContentBrowserClient::ShowPaymentHandlerWindow(
     content::BrowserContext* browser_context,
     const GURL& url,
     base::OnceCallback<void(bool, int, int)> callback) {
+  DCHECK(browser_context);
   return false;
 }
 
@@ -806,7 +838,9 @@ bool ContentBrowserClient::IsSafeRedirectTarget(const GURL& url,
 
 void ContentBrowserClient::RegisterRendererPreferenceWatcherForWorkers(
     BrowserContext* browser_context,
-    mojom::RendererPreferenceWatcherPtr watcher) {}
+    mojom::RendererPreferenceWatcherPtr watcher) {
+  // |browser_context| may be null (e.g. during shutdown of a service worker).
+}
 
 base::Optional<std::string> ContentBrowserClient::GetOriginPolicyErrorPage(
     OriginPolicyErrorReason error_reason,