Implement upgrade-insecure-requests in browser for frame requests

third_party/WebKit/Source/core/loader/FrameLoader.cpp

 /*
  * Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011 Apple Inc. All rights
  * reserved.
  * Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies)
  * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved.
  * (http://www.torchmobile.com/)
  * Copyright (C) 2008 Alp Toker <alp@atoker.com>
  * Copyright (C) Research In Motion Limited 2009. All rights reserved.
  * Copyright (C) 2011 Kris Jordan <krisjordan@gmail.com>
  * Copyright (C) 2011 Google Inc. All rights reserved.
@@ skipping 47 matching lines @@
 #include "core/frame/csp/ContentSecurityPolicy.h"
 #include "core/html/HTMLFormElement.h"
 #include "core/html/HTMLFrameOwnerElement.h"
 #include "core/input/EventHandler.h"
 #include "core/inspector/ConsoleMessage.h"
 #include "core/loader/DocumentLoadTiming.h"
 #include "core/loader/DocumentLoader.h"
 #include "core/loader/FormSubmission.h"
 #include "core/loader/FrameLoadRequest.h"
 #include "core/loader/LinkLoader.h"
-#include "core/loader/MixedContentChecker.h"
 #include "core/loader/NavigationScheduler.h"
 #include "core/loader/NetworkHintsInterface.h"
 #include "core/loader/ProgressTracker.h"
 #include "core/loader/appcache/ApplicationCacheHost.h"
 #include "core/page/ChromeClient.h"
 #include "core/page/CreateWindow.h"
 #include "core/page/FrameTree.h"
 #include "core/page/Page.h"
 #include "core/page/WindowFeatures.h"
 #include "core/page/scrolling/ScrollingCoordinator.h"
@@ skipping 1271 matching lines @@
       Document::kNoDismissal) {
     return kNavigationPolicyIgnore;
   }
 
   // Record the latest requiredCSP value that will be used when sending this
   // request.
   ResourceRequest& resource_request = frame_load_request.GetResourceRequest();
   RecordLatestRequiredCSP();
   // Before modifying the request, check report-only CSP headers to give the
   // site owner a chance to learn about requests that need to be modified.
-  //
-  // TODO(estark): this doesn't work with --enable-browser-side-navigation,
-  // wherein 'frame-src' is checked in the browser process. Figure out what to
-  // do; maybe with browser-side navigation the upgrade should be happening in
-  // the browser process too. See also https://crbug.com/692595
   Settings* settings = frame_->GetSettings();
   MaybeCheckCSP(
       resource_request, navigation_type, frame_, navigation_policy,
       frame_load_request.ShouldCheckMainWorldContentSecurityPolicy() ==
           kCheckContentSecurityPolicy,
       settings && settings->GetBrowserSideNavigationEnabled(),
       ContentSecurityPolicy::CheckHeaderType::kCheckReportOnly);
   ModifyRequestForCSP(resource_request, nullptr);
 
   return ShouldContinueForNavigationPolicy(
@@ skipping 218 matching lines @@
     // Early return if the request has already been upgraded.
     if (!resource_request.HttpHeaderField(HTTPNames::Upgrade_Insecure_Requests)
              .IsNull()) {
       return;
     }
 
     resource_request.SetHTTPHeaderField(HTTPNames::Upgrade_Insecure_Requests,
                                         "1");
   }
 
+  // PlzNavigate: Upgrading subframe requests is handled by the browser process.
+  Settings* settings = frame_->GetSettings();
+  if (resource_request.GetFrameType() == WebURLRequest::kFrameTypeNested &&
+      settings && settings->GetBrowserSideNavigationEnabled()) {

[Mike West, 2017/05/30 07:36:30]

Tiny nit: You could skip checking `settings` here if you wrapped the assignment
in an if. Like `if (Settings* settings = frame_->GetSettings())`

[estark, 2017/06/01 04:18:30]

I'll skip this one if you don't mind -- my weakling brain finds such things hard
to read. Happy to do it if you feel strongly though :)

+    return;
+  }
   UpgradeInsecureRequest(resource_request, document);
 }
 
 void FrameLoader::UpgradeInsecureRequest(ResourceRequest& resource_request,
                                          Document* document) const {
   KURL url = resource_request.Url();
 
   // If we don't yet have an |m_document| (because we're loading an iframe, for
   // instance), check the FrameLoader's policy.
   WebInsecureRequestPolicy relevant_policy =
@@ skipping 75 matching lines @@
   // TODO(japhet): This is needed because the browser process DCHECKs if the
   // first entry we commit in a new frame has replacement set. It's unclear
   // whether the DCHECK is right, investigate removing this special case.
   bool replace_current_item = load_type == kFrameLoadTypeReplaceCurrentItem &&
                               (!Opener() || !request.Url().IsEmpty());
   loader->SetReplacesCurrentHistoryItem(replace_current_item);
   return loader;
 }
 
 }  // namespace blink