From d86d61ad977ee59202503b31cc338668e18028d8 Mon Sep 17 00:00:00 2001 From: Asami Doi Date: Mon, 8 Aug 2022 04:53:39 +0000 Subject: [PATCH] Add some CHECKs for crbug.com/1336617 This CL adds some CHECKs to investigate crbug.com/1336617. According to the crash logs, calling `provider->GetRuleIterator()` causes a crash. See an example crash log: https://crash.corp.google.com/browse?q=product_name%3D%27Chrome_Mac%27+AND+expanded_custom_data.ChromeCrashProto.ptype%3D%27browser%27+AND+expanded_custom_data.ChromeCrashProto.magic_signature_1.name%3D%27HostContentSettingsMap%3A%3AGetWebsiteSettingInternal%27+AND+EXISTS+%28SELECT+1+FROM+UNNEST%28CrashedStackTrace.StackFrame%29+WHERE+FunctionName%3D%27HostContentSettingsMap%3A%3AGetWebsiteSettingInternal%28GURL+const%26%2C+GURL+const%26%2C+ContentSettingsType%2C+HostContentSettingsMap%3A%3AProviderType%2C+content_settings%3A%3ASettingInfo*%29+const%27%29+AND+EXISTS+%28SELECT+1+FROM+UNNEST%28CrashedStackTrace.StackFrame%29+WHERE+FunctionName%3D%27HostContentSettingsMap%3A%3AGetWebsiteSettingInternal%28GURL+const%26%2C+GURL+const%26%2C+ContentSettingsType%2C+HostContentSettingsMap%3A%3AProviderType%2C+content_settings%3A%3ASettingInfo*%29+const%27%29&stbtiq=&reportid=67fe77a12c39ed26&index=9#0 Bug: 1336617 Change-Id: I2abfeb9f2b45cda9ebe749b0ae2bd4bb09ce5c8e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3800829 Reviewed-by: Christian Dullweber Reviewed-by: Victor Tan Commit-Queue: Asami Doi Reviewed-by: Evan Stade Cr-Commit-Position: refs/heads/main@{#1032420} --- .../core/browser/host_content_settings_map.cc | 4 ++++ components/embedder_support/content_settings_utils.cc | 5 +++++ weblayer/browser/content_browser_client_impl.cc | 1 + 3 files changed, 10 insertions(+) diff --git a/components/content_settings/core/browser/host_content_settings_map.cc b/components/content_settings/core/browser/host_content_settings_map.cc index db99f2296325c..7944f63614749 100644 --- a/components/content_settings/core/browser/host_content_settings_map.cc +++ b/components/content_settings/core/browser/host_content_settings_map.cc @@ -954,6 +954,10 @@ base::Value HostContentSettingsMap::GetContentSettingValueAndPatterns( ContentSettingsPattern* primary_pattern, ContentSettingsPattern* secondary_pattern, content_settings::SessionModel* session_model) { + // TODO(crbug.com/1336617): Remove this check once we figure out what is + // wrong. + CHECK(provider); + if (include_incognito) { // Check incognito-only specific settings. It's essential that the // |RuleIterator| gets out of scope before we get a rule iterator for the diff --git a/components/embedder_support/content_settings_utils.cc b/components/embedder_support/content_settings_utils.cc index 7ff1e66751633..9d43e5a9dfdf6 100644 --- a/components/embedder_support/content_settings_utils.cc +++ b/components/embedder_support/content_settings_utils.cc @@ -8,6 +8,7 @@ #include "components/content_settings/core/browser/cookie_settings.h" #include "components/content_settings/core/common/content_settings.h" #include "components/content_settings/core/common/content_settings_utils.h" +#include "content/public/browser/browser_thread.h" #include "net/cookies/site_for_cookies.h" #include "url/gurl.h" #include "url/origin.h" @@ -43,6 +44,10 @@ content::AllowServiceWorkerResult AllowServiceWorker( const absl::optional& top_frame_origin, const content_settings::CookieSettings* cookie_settings, const HostContentSettingsMap* settings_map) { + DCHECK_CURRENTLY_ON(content::BrowserThread::UI); + // TODO(crbug.com/1336617): Remove this check once we figure out what is + // wrong. + DCHECK(settings_map); GURL first_party_url = top_frame_origin ? top_frame_origin->GetURL() : GURL(); // Check if JavaScript is allowed. content_settings::SettingInfo info; diff --git a/weblayer/browser/content_browser_client_impl.cc b/weblayer/browser/content_browser_client_impl.cc index 636dd9c44b228..2f24cb0e0dbff 100644 --- a/weblayer/browser/content_browser_client_impl.cc +++ b/weblayer/browser/content_browser_client_impl.cc @@ -336,6 +336,7 @@ content::AllowServiceWorkerResult ContentBrowserClientImpl::AllowServiceWorker( const absl::optional& top_frame_origin, const GURL& script_url, content::BrowserContext* context) { + DCHECK_CURRENTLY_ON(content::BrowserThread::UI); return embedder_support::AllowServiceWorker( scope, site_for_cookies, top_frame_origin, CookieSettingsFactory::GetForBrowserContext(context).get(),