Return `false` from ResourceNeedsLoad in presence of an MHTML archive.

After this CL, blink::ResourceFetcher should not start HTTP requests if
an MHTML archive is present. Additionally tweak GetCacheIdentifier to
make sure that globally cached requests (e.g. fulfilled via HTTP) are
not reused for requests within MHTML archive.

Change-Id: I3b7253c22bc42b941d9abd8dec82e58f730dd815
Bug: 1168249
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2638219Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Arthur Sonzogni <arthursonzogni@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Auto-Submit: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#846266}

content/browser/navigation_mhtml_browsertest.cc

@@ -69,6 +69,20 @@ class MhtmlArchive {
     content_ += "\n--MHTML_BOUNDARY\n" + content;
   }
 
+  void AddResource(const GURL& url,
+                   const std::string mime_type,
+                   const std::string headers,
+                   const std::string body) {
+    const char* document_template =
+        "Content-Type: $1\n"
+        "Content-Location: $2\n"
+        "$3"
+        "\n"
+        "$4";
+    AddResource(base::ReplaceStringPlaceholders(
+        document_template, {mime_type, url.spec(), headers, body}, nullptr));
+  }
+
   void AddHtmlDocument(const GURL& url,
                        const std::string headers,
                        const std::string body) {
@@ -736,4 +750,31 @@ IN_PROC_BROWSER_TEST_F(NavigationMhtmlBrowserTest, DataIframe) {
   }
 }
 
+// Regression test for https://crbug.com/1168249.
+IN_PROC_BROWSER_TEST_F(NavigationMhtmlBrowserTest, PreloadedTextTrack) {
+  // The test uses a cross-site subframe, so any HTTP requests that reach the
+  // NetworkService will have `network::ResourceRequest::request_initiator` with
+  // a tuple (or precursor tuple in case of opaque origins expected for MHTML
+  // documents) that is incompatible with `request_initiator_origin_lock` in
+  // `network::mojom::URLLoaderFactoryParams`.
+  MhtmlArchive mhtml_archive;
+  mhtml_archive.AddHtmlDocument(
+      GURL("http://main.com/main.html"), "",
+      R"( <iframe src="http://subframe.com/subframe.html"></iframe> )");
+  mhtml_archive.AddHtmlDocument(
+      GURL("http://subframe.com/subframe.html"), "",
+      R"( <link rel="preload" href="http://resource.com/track" as="track"> )");
+  mhtml_archive.AddResource(GURL("http://resource.com/track"), "text/vtt", "",
+                            "fake text track body");
+  GURL mhtml_url = mhtml_archive.Write("index.mhtml");
+
+  EXPECT_TRUE(NavigateToURL(shell(), mhtml_url));
+
+  // The main verification is that ResourceFetcher::StartLoad didn't reach
+  // NOTREACHED assertion (against HTTP resource loads triggered from MHTML
+  // documents). To detect such NOTREACHED (via renderer crash) it is sufficient
+  // for the test to wait for DidStopLoading notification (which is done
+  // underneath NavigateToURL called above).
+}
+
 }  // namespace content

third_party/blink/renderer/platform/loader/fetch/resource_fetcher.cc

@@ -616,6 +616,11 @@ ResourceFetcher::IsControlledByServiceWorker() const {
 bool ResourceFetcher::ResourceNeedsLoad(Resource* resource,
                                         const FetchParameters& params,
                                         RevalidationPolicy policy) {
+  // MHTML documents should not trigger actual loads (i.e. all resource requests
+  // should be fulfilled by the MHTML archive).
+  if (archive_)
+    return false;
+
   // Defer a font load until it is actually needed unless this is a link
   // preload.
   if (resource->GetType() == ResourceType::kFont && !params.IsLinkPreload())
@@ -1986,6 +1991,15 @@ bool ResourceFetcher::StartLoad(Resource* resource,
 
   ResourceLoader* loader = nullptr;
 
+  if (archive_ && resource->Url().ProtocolIsInHTTPFamily()) {
+    // MHTML documents should not trigger HTTP requests.
+    //
+    // TODO(lukasza): https://crbug.com/1151438: Remove the ad-hoc DwoC below,
+    // once the bug is fixed and verified.
+    NOTREACHED();
+    base::debug::DumpWithoutCrashing();
+  }
+
   {
     // Forbids JavaScript/revalidation until start()
     // to prevent unintended state transitions.
@@ -2138,6 +2152,11 @@ String ResourceFetcher::GetCacheIdentifier(const KURL& url) const {
   if (properties_->WebBundlePhysicalUrl().IsValid())
     return properties_->WebBundlePhysicalUrl().GetString();
 
+  // Requests that can be satisfied via `archive_` (i.e. MHTML) or
+  // `subresource_web_bundles_` should not participate in the global caching,
+  // but should use a bundle/mhtml-specific cache.
+  if (archive_)
+    return archive_->GetCacheIdentifier();
   for (auto& bundle : subresource_web_bundles_) {
     if (bundle->CanHandleRequest(url))
       return bundle->GetCacheIdentifier();

third_party/blink/renderer/platform/mhtml/mhtml_archive.cc

@@ -221,6 +221,7 @@ MHTMLArchive* MHTMLArchive::CreateArchive(
     const KURL& url,
     scoped_refptr<const SharedBuffer> data) {
   MHTMLArchive* archive = MakeGarbageCollected<MHTMLArchive>();
+  archive->archive_url_ = url;
 
   // |data| may be null if archive file is empty.
   if (!data || data->IsEmpty()) {
@@ -441,6 +442,10 @@ ArchiveResource* MHTMLArchive::SubresourceForURL(const KURL& url) const {
   return subresources_.at(url.GetString());
 }
 
+String MHTMLArchive::GetCacheIdentifier() const {
+  return archive_url_.GetString();
+}
+
 void MHTMLArchive::Trace(Visitor* visitor) const {
   visitor->Trace(main_resource_);
   visitor->Trace(subresources_);

third_party/blink/renderer/platform/mhtml/mhtml_archive.h

@@ -33,9 +33,9 @@
 
 #include "third_party/blink/public/mojom/loader/mhtml_load_result.mojom-blink-forward.h"
 #include "third_party/blink/renderer/platform/heap/handle.h"
+#include "third_party/blink/renderer/platform/weborigin/kurl.h"
 #include "third_party/blink/renderer/platform/wtf/hash_map.h"
 #include "third_party/blink/renderer/platform/wtf/text/string_hash.h"
-
 #include "third_party/blink/renderer/platform/wtf/vector.h"
 
 namespace blink {
@@ -102,6 +102,8 @@ class PLATFORM_EXPORT MHTMLArchive final
   ArchiveResource* MainResource() const { return main_resource_.Get(); }
   ArchiveResource* SubresourceForURL(const KURL&) const;
 
+  String GetCacheIdentifier() const;
+
   // The purported creation date (as expressed by the Date: header).
   base::Time Date() const { return date_; }
 
@@ -117,6 +119,9 @@ class PLATFORM_EXPORT MHTMLArchive final
   void AddSubresource(ArchiveResource*);
   static bool CanLoadArchive(const KURL&);
 
+  // URL of the MHTML resource (e.g. file:///foo/bar.mhtml).
+  KURL archive_url_;
+
   base::Time date_;
   Member<ArchiveResource> main_resource_;
   SubArchiveResources subresources_;