Revert "Move content::ManifestFetcher in blink"

This reverts commit e7a24ceb.

Reason for revert:  Suspected cause of Use-after-poison in blink::ThreadableLoader::Cancel

Original change's description:
> Move content::ManifestFetcher in blink
> 
> This is a part of moving manifest implementation to blink.
> This CL moves content::ManifestFetcher to blink and introduces
> WebManifestFetcher in blink/public/web to expose the fetcher to
> content::ManifestManager.
> 
> Bug: 704441
> Change-Id: Ia4114390f124973542aca0151d06baec82fdaec1
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1548972
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
> Commit-Queue: Miyoung Shin <myid.shin@igalia.com>
> Cr-Commit-Position: refs/heads/master@{#649147}

TBR=dgozman@chromium.org,yhirano@chromium.org,haraken@chromium.org,dominickn@chromium.org,myid.shin@igalia.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 704441
Change-Id: I719bf26205f0aa1377ad5ab320fcd23f3e187ac6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1562572Reviewed-by: Miyoung Shin <myid.shin@igalia.com>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Miyoung Shin <myid.shin@igalia.com>
Cr-Commit-Position: refs/heads/master@{#649688}

content/renderer/BUILD.gn

@@ -100,6 +100,8 @@ target(link_target_type, "renderer") {
     "effective_connection_type_helper.h",
     "fetchers/associated_resource_fetcher_impl.cc",
     "fetchers/associated_resource_fetcher_impl.h",
+    "fetchers/manifest_fetcher.cc",
+    "fetchers/manifest_fetcher.h",
     "fetchers/multi_resolution_image_resource_fetcher.cc",
     "fetchers/multi_resolution_image_resource_fetcher.h",
     "fetchers/resource_fetcher_impl.cc",

content/renderer/fetchers/manifest_fetcher.cc

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "content/renderer/fetchers/manifest_fetcher.h"
+
+#include "base/bind.h"
+#include "base/logging.h"
+#include "content/public/renderer/associated_resource_fetcher.h"
+#include "services/network/public/mojom/request_context_frame_type.mojom.h"
+#include "third_party/blink/public/platform/web_url_request.h"
+#include "third_party/blink/public/web/web_associated_url_loader_options.h"
+#include "third_party/blink/public/web/web_local_frame.h"
+
+namespace content {
+
+ManifestFetcher::ManifestFetcher(const GURL& url)
+    : completed_(false) {
+  fetcher_.reset(AssociatedResourceFetcher::Create(url));
+}
+
+ManifestFetcher::~ManifestFetcher() {
+  if (!completed_)
+    Cancel();
+}
+
+void ManifestFetcher::Start(blink::WebLocalFrame* frame,
+                            bool use_credentials,
+                            const Callback& callback) {
+  callback_ = callback;
+
+  blink::WebAssociatedURLLoaderOptions options;
+  fetcher_->SetLoaderOptions(options);
+
+  // See https://w3c.github.io/manifest/. Use "include" when use_credentials is
+  // true, and "omit" otherwise.
+  fetcher_->Start(
+      frame, blink::mojom::RequestContextType::MANIFEST,
+      network::mojom::FetchRequestMode::kCors,
+      use_credentials ? network::mojom::FetchCredentialsMode::kInclude
+                      : network::mojom::FetchCredentialsMode::kOmit,
+      base::Bind(&ManifestFetcher::OnLoadComplete, base::Unretained(this)));
+}
+
+void ManifestFetcher::Cancel() {
+  DCHECK(!completed_);
+  fetcher_->Cancel();
+}
+
+void ManifestFetcher::OnLoadComplete(const blink::WebURLResponse& response,
+                                     const std::string& data) {
+  DCHECK(!completed_);
+  completed_ = true;
+
+  Callback callback = callback_;
+  std::move(callback).Run(response, data);
+}
+
+}  // namespace content

content/renderer/fetchers/manifest_fetcher.h

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CONTENT_RENDERER_FETCHERS_MANIFEST_FETCHER_H_
+#define CONTENT_RENDERER_FETCHERS_MANIFEST_FETCHER_H_
+
+#include <memory>
+#include <string>
+
+#include "base/callback.h"
+#include "base/macros.h"
+#include "content/common/content_export.h"
+#include "third_party/blink/public/platform/web_url_response.h"
+
+class GURL;
+
+namespace blink {
+class WebLocalFrame;
+}
+
+namespace content {
+
+class AssociatedResourceFetcher;
+
+// Helper class to download a Web Manifest. When an instance is created, the
+// caller need to call Start() and wait for the passed callback to be executed.
+// If the fetch fails, the callback will be called with two empty objects.
+class CONTENT_EXPORT ManifestFetcher {
+ public:
+  // This will be called asynchronously after the URL has been fetched,
+  // successfully or not.  If there is a failure, response and data will both be
+  // empty.  |response| and |data| are both valid until the URLFetcher instance
+  // is destroyed.
+  typedef base::Callback<void(const blink::WebURLResponse& response,
+                              const std::string& data)> Callback;
+
+  explicit ManifestFetcher(const GURL& url);
+  virtual ~ManifestFetcher();
+
+  void Start(blink::WebLocalFrame* frame,
+             bool use_credentials,
+             const Callback& callback);
+  void Cancel();
+
+ private:
+  void OnLoadComplete(const blink::WebURLResponse& response,
+                      const std::string& data);
+
+  bool completed_;
+  Callback callback_;
+  std::unique_ptr<AssociatedResourceFetcher> fetcher_;
+
+  DISALLOW_COPY_AND_ASSIGN(ManifestFetcher);
+};
+
+}  // namespace content
+
+#endif  // CONTENT_RENDERER_FETCHERS_MANIFEST_FETCHER_H_

content/renderer/manifest/manifest_manager.cc

@@ -10,6 +10,7 @@
 #include "base/no_destructor.h"
 #include "base/strings/nullable_string16.h"
 #include "content/public/renderer/render_frame.h"
+#include "content/renderer/fetchers/manifest_fetcher.h"
 #include "content/renderer/manifest/manifest_uma_util.h"
 #include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"
 #include "third_party/blink/public/mojom/manifest/manifest.mojom.h"
@@ -17,7 +18,6 @@
 #include "third_party/blink/public/web/web_console_message.h"
 #include "third_party/blink/public/web/web_document.h"
 #include "third_party/blink/public/web/web_local_frame.h"
-#include "third_party/blink/public/web/web_manifest_fetcher.h"
 #include "third_party/blink/public/web/web_manifest_parser.h"
 
 namespace content {
@@ -114,8 +114,7 @@ void ManifestManager::FetchManifest() {
     return;
   }
 
-  blink::WebDocument document = render_frame()->GetWebFrame()->GetDocument();
-  manifest_url_ = document.ManifestURL();
+  manifest_url_ = render_frame()->GetWebFrame()->GetDocument().ManifestURL();
 
   if (manifest_url_.is_empty()) {
     ManifestUmaUtil::FetchFailed(ManifestUmaUtil::FETCH_EMPTY_URL);
@@ -123,11 +122,13 @@ void ManifestManager::FetchManifest() {
     return;
   }
 
-  fetcher_.reset(new blink::WebManifestFetcher(manifest_url_));
-
-  fetcher_->Start(&document, document.ManifestUseCredentials(),
-                  base::BindOnce(&ManifestManager::OnManifestFetchComplete,
-                                 base::Unretained(this), document.Url()));
+  fetcher_.reset(new ManifestFetcher(manifest_url_));
+  fetcher_->Start(
+      render_frame()->GetWebFrame(),
+      render_frame()->GetWebFrame()->GetDocument().ManifestUseCredentials(),
+      base::Bind(&ManifestManager::OnManifestFetchComplete,
+                 base::Unretained(this),
+                 render_frame()->GetWebFrame()->GetDocument().Url()));
 }
 
 static const std::string& GetMessagePrefix() {
@@ -138,9 +139,9 @@ static const std::string& GetMessagePrefix() {
 void ManifestManager::OnManifestFetchComplete(
     const GURL& document_url,
     const blink::WebURLResponse& response,
-    const blink::WebString& data) {
+    const std::string& data) {
   fetcher_.reset();
-  if (response.IsNull() && data.IsEmpty()) {
+  if (response.IsNull() && data.empty()) {
     manifest_debug_info_ = nullptr;
     ManifestUmaUtil::FetchFailed(ManifestUmaUtil::FETCH_UNSPECIFIED_REASON);
     ResolveCallbacks(ResolveStateFailure);
@@ -149,15 +150,14 @@ void ManifestManager::OnManifestFetchComplete(
 
   ManifestUmaUtil::FetchSucceeded();
   GURL response_url = response.CurrentRequestUrl();
-  std::string data_string = data.Utf8();
-  base::StringPiece data_piece(data_string);
+  base::StringPiece data_piece(data);
 
   blink::WebVector<blink::ManifestError> errors;
   bool result = blink::WebManifestParser::ParseManifest(
       data_piece, response_url, document_url, &manifest_, &errors);
 
   manifest_debug_info_ = blink::mojom::ManifestDebugInfo::New();
-  manifest_debug_info_->raw_manifest = data_string;
+  manifest_debug_info_->raw_manifest = data;
 
   for (const auto& error : errors) {
     blink::WebConsoleMessage message;

content/renderer/manifest/manifest_manager.h

@@ -20,7 +20,6 @@
 class GURL;
 
 namespace blink {
-class WebManifestFetcher;
 class WebURLResponse;
 }
 
@@ -72,10 +71,10 @@ class ManifestManager : public RenderFrameObserver,
   void FetchManifest();
   void OnManifestFetchComplete(const GURL& document_url,
                                const blink::WebURLResponse& response,
-                               const blink::WebString& data);
+                               const std::string& data);
   void ResolveCallbacks(ResolveState state);
 
-  std::unique_ptr<blink::WebManifestFetcher> fetcher_;
+  std::unique_ptr<ManifestFetcher> fetcher_;
 
   // Whether the RenderFrame may have an associated Manifest. If true, the frame
   // may have a manifest, if false, it can't have one. This boolean is true when

third_party/blink/public/BUILD.gn

@@ -461,7 +461,6 @@ source_set("blink_headers") {
     "web/web_language_detection_details.h",
     "web/web_local_frame.h",
     "web/web_local_frame_client.h",
-    "web/web_manifest_fetcher.h",
     "web/web_manifest_parser.h",
     "web/web_meaningful_layout.h",
     "web/web_media_player_action.h",

third_party/blink/public/web/web_manifest_fetcher.h

-// Copyright 2019 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef THIRD_PARTY_BLINK_PUBLIC_WEB_WEB_MANIFEST_FETCHER_H_
-#define THIRD_PARTY_BLINK_PUBLIC_WEB_WEB_MANIFEST_FETCHER_H_
-
-#include <memory>
-#include "base/callback.h"
-#include "third_party/blink/public/platform/web_common.h"
-#include "third_party/blink/public/platform/web_private_ptr.h"
-
-namespace blink {
-
-class ManifestFetcher;
-class WebDocument;
-class WebURL;
-class WebURLResponse;
-class WebString;
-
-class WebManifestFetcher {
- public:
-  // This will be called asynchronously after the URL has been fetched,
-  // successfully or not.  If there is a failure, response and data will both be
-  // empty.  |response| and |data| are both valid until the ManifestFetcher
-  // instance is destroyed.
-  using Callback =
-      base::OnceCallback<void(const WebURLResponse&, const WebString&)>;
-
-  BLINK_EXPORT explicit WebManifestFetcher(const WebURL& url);
-  BLINK_EXPORT ~WebManifestFetcher() { Reset(); }
-
-  BLINK_EXPORT void Reset();
-
-  BLINK_EXPORT void Start(WebDocument* web_document,
-                          bool use_credentials,
-                          Callback callback);
-  BLINK_EXPORT void Cancel();
-
- private:
-  bool IsNull() const { return private_.IsNull(); }
-
-  WebPrivatePtr<ManifestFetcher> private_;
-};
-
-}  // namespace blink
-
-#endif  // THIRD_PARTY_BLINK_PUBLIC_WEB_WEB_MANIFEST_FETCHER_H_

third_party/blink/renderer/modules/exported/BUILD.gn

@@ -13,7 +13,6 @@ blink_modules_sources("exported") {
     "web_dom_media_stream_track.cc",
     "web_embedded_worker_impl.cc",
     "web_embedded_worker_impl.h",
-    "web_manifest_fetcher.cc",
     "web_manifest_parser.cc",
     "web_storage_event_dispatcher_impl.cc",
     "web_user_media_request.cc",

third_party/blink/renderer/modules/exported/web_manifest_fetcher.cc

-// Copyright 2019 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "third_party/blink/public/web/web_manifest_fetcher.h"
-
-#include "third_party/blink/public/platform/web_url.h"
-#include "third_party/blink/public/web/web_document.h"
-#include "third_party/blink/renderer/core/dom/document.h"
-#include "third_party/blink/renderer/modules/manifest/manifest_fetcher.h"
-#include "third_party/blink/renderer/platform/weborigin/kurl.h"
-
-namespace blink {
-
-WebManifestFetcher::WebManifestFetcher(const WebURL& url)
-    : private_(MakeGarbageCollected<ManifestFetcher>(url)) {}
-
-void WebManifestFetcher::Start(WebDocument* web_document,
-                               bool use_credentials,
-                               Callback callback) {
-  DCHECK(!IsNull());
-
-  Document* document = web_document->Unwrap<Document>();
-  private_->Start(*document, use_credentials, std::move(callback));
-}
-
-void WebManifestFetcher::Cancel() {
-  DCHECK(!IsNull());
-  private_->Cancel();
-}
-
-void WebManifestFetcher::Reset() {
-  private_.Reset();
-}
-
-}  // namespace blink

third_party/blink/renderer/modules/manifest/BUILD.gn

@@ -8,8 +8,6 @@ blink_modules_sources("manifest") {
   sources = [
     "image_resource_type_converters.cc",
     "image_resource_type_converters.h",
-    "manifest_fetcher.cc",
-    "manifest_fetcher.h",
     "manifest_parser.cc",
     "manifest_parser.h",
     "manifest_uma_util.cc",

third_party/blink/renderer/modules/manifest/manifest_fetcher.cc

-// Copyright 2019 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include "third_party/blink/renderer/modules/manifest/manifest_fetcher.h"
-
-#include "third_party/blink/renderer/core/dom/document.h"
-#include "third_party/blink/renderer/core/loader/threadable_loader.h"
-#include "third_party/blink/renderer/platform/exported/wrapped_resource_response.h"
-
-namespace blink {
-
-ManifestFetcher::ManifestFetcher(const KURL& url)
-    : url_(url), completed_(false) {}
-
-ManifestFetcher::~ManifestFetcher() {
-  if (!completed_)
-    Cancel();
-}
-
-void ManifestFetcher::Start(Document& document,
-                            bool use_credentials,
-                            WebManifestFetcher::Callback callback) {
-  callback_ = std::move(callback);
-
-  ResourceRequest request(url_);
-  request.SetRequestContext(mojom::RequestContextType::MANIFEST);
-  request.SetFetchRequestMode(network::mojom::FetchRequestMode::kCors);
-  // See https://w3c.github.io/manifest/. Use "include" when use_credentials is
-  // true, and "omit" otherwise.
-  request.SetFetchCredentialsMode(
-      use_credentials ? network::mojom::FetchCredentialsMode::kInclude
-                      : network::mojom::FetchCredentialsMode::kOmit);
-
-  ResourceLoaderOptions resource_loader_options;
-  resource_loader_options.data_buffering_policy = kDoNotBufferData;
-
-  loader_ = MakeGarbageCollected<ThreadableLoader>(document, this,
-                                                   resource_loader_options);
-  loader_->Start(request);
-}
-
-void ManifestFetcher::Cancel() {
-  if (!loader_)
-    return;
-
-  DCHECK(!completed_);
-
-  ThreadableLoader* loader = loader_.Release();
-  loader->Cancel();
-}
-
-void ManifestFetcher::DidReceiveResponse(uint64_t,
-                                         const ResourceResponse& response) {
-  response_ = response;
-}
-
-void ManifestFetcher::DidReceiveData(const char* data, unsigned length) {
-  if (!length)
-    return;
-
-  data_.Append(data, length);
-}
-
-void ManifestFetcher::DidFinishLoading(uint64_t) {
-  DCHECK(!completed_);
-  completed_ = true;
-
-  WrappedResourceResponse wrapped_response(response_);
-  std::move(callback_).Run(wrapped_response, data_.ToString());
-  data_.Clear();
-}
-
-void ManifestFetcher::DidFail(const ResourceError& error) {
-  if (!callback_)
-    return;
-
-  data_.Clear();
-
-  WrappedResourceResponse wrapped_response(response_);
-  std::move(callback_).Run(wrapped_response, String());
-}
-
-void ManifestFetcher::DidFailRedirectCheck() {
-  DidFail(ResourceError::Failure(NullURL()));
-}
-
-}  // namespace blink

third_party/blink/renderer/modules/manifest/manifest_fetcher.h

-// Copyright 2019 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#ifndef THIRD_PARTY_BLINK_RENDERER_MODULES_MANIFEST_MANIFEST_FETCHER_H_
-#define THIRD_PARTY_BLINK_RENDERER_MODULES_MANIFEST_MANIFEST_FETCHER_H_
-
-#include <memory>
-
-#include "base/callback.h"
-#include "third_party/blink/public/web/web_manifest_fetcher.h"
-#include "third_party/blink/renderer/core/loader/threadable_loader.h"
-#include "third_party/blink/renderer/core/loader/threadable_loader_client.h"
-#include "third_party/blink/renderer/platform/heap/garbage_collected.h"
-#include "third_party/blink/renderer/platform/heap/member.h"
-#include "third_party/blink/renderer/platform/loader/fetch/resource_response.h"
-#include "third_party/blink/renderer/platform/wtf/text/string_builder.h"
-
-namespace blink {
-
-class Document;
-class KURL;
-
-// Helper class to download a Web Manifest. When an instance is created, the
-// caller need to call Start() and wait for the passed callback to be executed.
-// If the fetch fails, the callback will be called with two empty objects.
-class ManifestFetcher final : public GarbageCollectedFinalized<ManifestFetcher>,
-                              public ThreadableLoaderClient {
-  USING_GARBAGE_COLLECTED_MIXIN(ManifestFetcher);
-
- public:
-  explicit ManifestFetcher(const KURL& url);
-  ~ManifestFetcher() override;
-
-  void Start(Document& document,
-             bool use_credentials,
-             WebManifestFetcher::Callback callback);
-  void Cancel();
-
-  // ThreadableLoaderClient
-  void DidReceiveResponse(uint64_t, const ResourceResponse&) override;
-  void DidReceiveData(const char*, unsigned) override;
-  void DidFinishLoading(uint64_t) override;
-  void DidFail(const ResourceError&) override;
-  void DidFailRedirectCheck() override;
-
-  void Trace(Visitor* visitor) override { visitor->Trace(loader_); }
-
- private:
-  KURL url_;
-  bool completed_;
-  WebManifestFetcher::Callback callback_;
-  ResourceResponse response_;
-  StringBuilder data_;
-  Member<ThreadableLoader> loader_;
-
-  DISALLOW_COPY_AND_ASSIGN(ManifestFetcher);
-};
-
-}  // namespace blink
-
-#endif  // THIRD_PARTY_BLINK_RENDERER_MODULES_MANIFEST_MANIFEST_FETCHER_H_