Turn CHECKs to DCHECKs for NavigationHandle/NavigationLoader destruction.

There is no evidence of the CHECKs failing.

Bug: 857005
Change-Id: I8bb1b4372770987409996739f8c6b051e831809e
Reviewed-on: https://chromium-review.googlesource.com/1139837Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575564}

content/browser/frame_host/navigation_request.cc

@@ -728,9 +728,7 @@ void NavigationRequest::ResetForCrossDocumentRestart() {
   // Reset the NavigationHandle, which is now incorrectly marked as
   // same-document. Ensure |loader_| does not exist as it can hold raw pointers
   // to objects owned by the handle (see the comment in the header).
-  // TODO(falken): Turn this CHECK to a DCHECK if it holds, or else call
-  // loader_.reset() manually.
-  CHECK(!loader_);
+  DCHECK(!loader_);
   navigation_handle_.reset();
 
   // Convert the navigation type to the appropriate cross-document one.
@@ -1297,9 +1295,7 @@ void NavigationRequest::OnStartChecksComplete(
   // |loader_| should not exist if the service worker handle and app cache
   // handles will be destroyed, since it holds raw pointers to them. See the
   // comment in the header for |loader_|.
-  // TODO(falken): Turn this into a DCHECK if it holds, or else manually call
-  // loader_.reset() here.
-  CHECK(!loader_);
+  DCHECK(!loader_);
 
   // Only initialize the ServiceWorkerNavigationHandle if it can be created for
   // this frame.

content/browser/frame_host/navigation_request.h

@@ -374,10 +374,11 @@ class CONTENT_EXPORT NavigationRequest : public NavigationURLLoaderDelegate {
 
   NavigationState state_;
 
-  // It's important to ensure |navigation_handle_| outlives |loader_|, since
-  // the loader holds raw pointers to objects owned by the navigation handle
-  // (namely, the AppCache and service worker handles). So, declare the handle
-  // before the loader.
+  // It's important to ensure |navigation_handle_| outlives |loader_|, since the
+  // loader holds raw pointers to objects owned by the navigation handle
+  // (namely, the AppCache and service worker handles). The destruction order
+  // matters because it occurs over separate tasks on the IO thread. So, declare
+  // the handle before the loader.
   std::unique_ptr<NavigationHandleImpl> navigation_handle_;
   std::unique_ptr<NavigationURLLoader> loader_;