Add NavigationRequest-related crash keys to help with debugging.

Bug: 1026738 Change-Id: Idefdfbc0310e9d5bdad89ea25bf9b5a94aaa01ff Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2042041 Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org> Reviewed-by: Alex Moshchuk <alexmos@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Reviewed-by: Yutaka Hirano <yhirano@chromium.org> Cr-Commit-Position: refs/heads/master@{#739609}
FairyWorld · Feb 7, 2020 · 731c39c · 731c39c
1 parent d4a5793
commit 731c39c
Show file tree

Hide file tree

Showing 6 changed files with 77 additions and 23 deletions.
diff --git a/content/browser/frame_host/navigation_request.cc b/content/browser/frame_host/navigation_request.cc
@@ -16,6 +16,7 @@
 #include "base/metrics/histogram_macros.h"
 #include "base/optional.h"
 #include "base/rand_util.h"
+#include "base/stl_util.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/system/sys_info.h"
@@ -606,6 +607,40 @@ mojom::NavigationType ConvertToCrossDocumentType(mojom::NavigationType type) {
   }
 }
 
+base::debug::CrashKeyString* GetNavigationRequestUrlCrashKey() {
+  static auto* crash_key = base::debug::AllocateCrashKeyString(
+      "navigation_request_url", base::debug::CrashKeySize::Size256);
+  return crash_key;
+}
+
+base::debug::CrashKeyString* GetNavigationRequestInitiatorCrashKey() {
+  static auto* crash_key = base::debug::AllocateCrashKeyString(
+      "navigation_request_initiator", base::debug::CrashKeySize::Size64);
+  return crash_key;
+}
+
+class ScopedNavigationRequestCrashKeys {
+ public:
+  explicit ScopedNavigationRequestCrashKeys(
+      NavigationRequest* navigation_request)
+      : initiator_origin_(
+            GetNavigationRequestInitiatorCrashKey(),
+            base::OptionalOrNullptr(navigation_request->GetInitiatorOrigin())),
+        url_(GetNavigationRequestUrlCrashKey(),
+             navigation_request->GetURL().possibly_invalid_spec()) {}
+  ~ScopedNavigationRequestCrashKeys() = default;
+
+  // No copy constructor and no copy assignment operator.
+  ScopedNavigationRequestCrashKeys(const ScopedNavigationRequestCrashKeys&) =
+      delete;
+  ScopedNavigationRequestCrashKeys& operator=(
+      const ScopedNavigationRequestCrashKeys&) = delete;
+
+ private:
+  url::debug::ScopedOriginCrashKey initiator_origin_;
+  base::debug::ScopedCrashKeyString url_;
+};
+
 }  // namespace
 
 // static
@@ -1079,6 +1114,7 @@ void NavigationRequest::BeginNavigation() {
                                "BeginNavigation");
   DCHECK(!loader_);
   DCHECK(!render_frame_host_);
+  ScopedNavigationRequestCrashKeys crash_keys(this);
 
   state_ = WILL_START_NAVIGATION;
 
@@ -1360,6 +1396,8 @@ NavigationRequest::TakeClientSecurityState() {
 void NavigationRequest::OnRequestRedirected(
     const net::RedirectInfo& redirect_info,
     network::mojom::URLResponseHeadPtr response_head) {
+  ScopedNavigationRequestCrashKeys crash_keys(this);
+
   // Sanity check - this can only be set at commit time.
   DCHECK(!auth_challenge_info_);
 
@@ -1581,6 +1619,8 @@ void NavigationRequest::OnResponseStarted(
     bool is_download,
     NavigationDownloadPolicy download_policy,
     base::Optional<SubresourceLoaderParams> subresource_loader_params) {
+  ScopedNavigationRequestCrashKeys crash_keys(this);
+
   // The |loader_|'s job is finished. It must not call the NavigationRequest
   // anymore from now.
   loader_.reset();
@@ -1907,6 +1947,7 @@ void NavigationRequest::OnRequestFailedInternal(
          state_ == WILL_FAIL_REQUEST);
   DCHECK(!(status.error_code == net::ERR_ABORTED &&
            error_page_content.has_value()));
+  ScopedNavigationRequestCrashKeys crash_keys(this);
 
   // The request failed, the |loader_| must not call the NavigationRequest
   // anymore from now while the error page is being loaded.

diff --git a/services/network/cors/cors_url_loader_factory.cc b/services/network/cors/cors_url_loader_factory.cc
@@ -9,6 +9,7 @@
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/metrics/histogram_macros.h"
+#include "base/stl_util.h"
 #include "mojo/public/cpp/bindings/message.h"
 #include "mojo/public/cpp/bindings/remote.h"
 #include "net/base/load_flags.h"
@@ -28,6 +29,7 @@
 #include "services/network/resource_scheduler/resource_scheduler_client.h"
 #include "services/network/url_loader.h"
 #include "services/network/url_loader_factory.h"
+#include "url/origin.h"
 
 namespace network {
 
@@ -308,9 +310,9 @@ bool CorsURLLoaderFactory::IsSane(const NetworkContext* context,
       // TODO(lukasza): https://crbug.com/920634: Report bad message and return
       // false below.
       NOTREACHED();
-      debug::ScopedOriginCrashKey initiator_lock_crash_key(
+      url::debug::ScopedOriginCrashKey initiator_lock_crash_key(
           debug::GetRequestInitiatorSiteLockCrashKey(),
-          request_initiator_site_lock_);
+          base::OptionalOrNullptr(request_initiator_site_lock_));
       mojo::ReportBadMessage(
           "CorsURLLoaderFactory: lock VS initiator mismatch");
       return false;

diff --git a/services/network/crash_keys.cc b/services/network/crash_keys.cc
@@ -4,6 +4,7 @@
 
 #include "services/network/crash_keys.h"
 
+#include "base/stl_util.h"
 #include "services/network/public/cpp/resource_request.h"
 #include "url/gurl.h"
 
@@ -32,20 +33,11 @@ base::debug::CrashKeyString* GetRequestInitiatorSiteLockCrashKey() {
   return crash_key;
 }
 
-ScopedOriginCrashKey::ScopedOriginCrashKey(
-    base::debug::CrashKeyString* crash_key,
-    const base::Optional<url::Origin>& value)
-    : base::debug::ScopedCrashKeyString(
-          crash_key,
-          value ? value->GetDebugString() : "base::nullopt") {}
-
-ScopedOriginCrashKey::~ScopedOriginCrashKey() = default;
-
 ScopedRequestCrashKeys::ScopedRequestCrashKeys(
     const network::ResourceRequest& request)
     : url_(GetUrlCrashKey(), request.url.possibly_invalid_spec()),
       request_initiator_(GetRequestInitiatorCrashKey(),
-                         request.request_initiator) {}
+                         base::OptionalOrNullptr(request.request_initiator)) {}
 
 ScopedRequestCrashKeys::~ScopedRequestCrashKeys() = default;
 

diff --git a/services/network/crash_keys.h b/services/network/crash_keys.h
@@ -17,16 +17,6 @@ namespace debug {
 
 base::debug::CrashKeyString* GetRequestInitiatorSiteLockCrashKey();
 
-class ScopedOriginCrashKey : public base::debug::ScopedCrashKeyString {
- public:
-  ScopedOriginCrashKey(base::debug::CrashKeyString* crash_key,
-                       const base::Optional<url::Origin>& value);
-  ~ScopedOriginCrashKey();
-
-  ScopedOriginCrashKey(const ScopedOriginCrashKey&) = delete;
-  ScopedOriginCrashKey& operator=(const ScopedOriginCrashKey&) = delete;
-};
-
 class ScopedRequestCrashKeys {
  public:
   ScopedRequestCrashKeys(const network::ResourceRequest& request);
@@ -37,7 +27,7 @@ class ScopedRequestCrashKeys {
 
  private:
   base::debug::ScopedCrashKeyString url_;
-  ScopedOriginCrashKey request_initiator_;
+  url::debug::ScopedOriginCrashKey request_initiator_;
 };
 
 }  // namespace debug

diff --git a/url/origin.cc b/url/origin.cc
@@ -351,4 +351,17 @@ bool Origin::Nonce::operator!=(const Origin::Nonce& other) const {
   return !(*this == other);
 }
 
+namespace debug {
+
+ScopedOriginCrashKey::ScopedOriginCrashKey(
+    base::debug::CrashKeyString* crash_key,
+    const url::Origin* value)
+    : base::debug::ScopedCrashKeyString(
+          crash_key,
+          value ? value->GetDebugString() : "nullptr") {}
+
+ScopedOriginCrashKey::~ScopedOriginCrashKey() = default;
+
+}  // namespace debug
+
 }  // namespace url
diff --git a/url/origin.h b/url/origin.h
@@ -11,6 +11,7 @@
 
 #include "base/component_export.h"
 #include "base/debug/alias.h"
+#include "base/debug/crash_logging.h"
 #include "base/optional.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_piece.h"
@@ -408,6 +409,21 @@ COMPONENT_EXPORT(URL) bool IsSameOriginWith(const GURL& a, const GURL& b);
 #define DEBUG_ALIAS_FOR_ORIGIN(var_name, origin) \
   DEBUG_ALIAS_FOR_CSTR(var_name, (origin).Serialize().c_str(), 128)
 
+namespace debug {
+
+class COMPONENT_EXPORT(URL) ScopedOriginCrashKey
+    : public base::debug::ScopedCrashKeyString {
+ public:
+  ScopedOriginCrashKey(base::debug::CrashKeyString* crash_key,
+                       const url::Origin* value);
+  ~ScopedOriginCrashKey();
+
+  ScopedOriginCrashKey(const ScopedOriginCrashKey&) = delete;
+  ScopedOriginCrashKey& operator=(const ScopedOriginCrashKey&) = delete;
+};
+
+}  // namespace debug
+
 }  // namespace url
 
 #endif  // URL_ORIGIN_H_