Subresource WebBundle: Output console warnings for WebBundle errors

Display a console warning when web bundle cannot be parsed, or requested
resource is not included in the bundle. Errors are reported through a
callback passed to CreateWebBundleSubresourceLoaderFactory().

Bug: 1082020
Change-Id: I77414239d17140fb18a22d74454e02ba1036b8f7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2342326Reviewed-by: Tsuyoshi Horo <horo@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Hayato Ito <hayato@chromium.org>
Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797552}

third_party/blink/renderer/core/html/link_web_bundle.cc

@@ -6,12 +6,14 @@
 
 #include "third_party/blink/public/mojom/fetch/fetch_api_request.mojom-blink.h"
 #include "third_party/blink/renderer/core/html/html_link_element.h"
+#include "third_party/blink/renderer/core/inspector/console_message.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/loader/cors/cors.h"
 #include "third_party/blink/renderer/platform/loader/fetch/bytes_consumer.h"
 #include "third_party/blink/renderer/platform/loader/fetch/resource_fetcher.h"
 #include "third_party/blink/renderer/platform/loader/fetch/url_loader/web_bundle_subresource_loader.h"
+#include "third_party/blink/renderer/platform/wtf/cross_thread_functional.h"
 
 namespace blink {
 
@@ -68,9 +70,10 @@ class WebBundleLoader : public GarbageCollected<WebBundleLoader>,
   void DidStartLoadingResponseBody(BytesConsumer& consumer) override {
     DCHECK(pending_factory_receiver_);
     CreateWebBundleSubresourceLoaderFactory(
-        std::move(pending_factory_receiver_), consumer.DrainAsDataPipe());
-    // TODO(crbug.com/1082020): Set |failed_| to true on metadata parse error,
-    // so that "error" event is dispatched.
+        std::move(pending_factory_receiver_), consumer.DrainAsDataPipe(),
+        ConvertToBaseRepeatingCallback(
+            CrossThreadBindRepeating(&WebBundleLoader::OnWebBundleError,
+                                     WrapCrossThreadWeakPersistent(this))));
   }
 
   void DidFinishLoading(uint64_t) override { link_web_bundle_->NotifyLoaded(); }
@@ -87,12 +90,19 @@ class WebBundleLoader : public GarbageCollected<WebBundleLoader>,
       // |pending_factory_receiver_| are processed (and fail).
       CreateWebBundleSubresourceLoaderFactory(
           std::move(pending_factory_receiver_),
-          mojo::ScopedDataPipeConsumerHandle());
+          mojo::ScopedDataPipeConsumerHandle(), base::DoNothing());
     }
     failed_ = true;
     link_web_bundle_->NotifyLoaded();
   }
 
+  void OnWebBundleError(WebBundleErrorType type, const String& message) {
+    // TODO(crbug.com/1082020): Dispatch "error" event on metadata parse error.
+    // Simply setting |failed_| here does not work because DidFinishLoading()
+    // may already be called.
+    link_web_bundle_->OnWebBundleError(url_.ElidedString() + ": " + message);
+  }
+
   Member<LinkWebBundle> link_web_bundle_;
   Member<ThreadableLoader> loader_;
   mojo::Remote<network::mojom::blink::URLLoaderFactory> loader_factory_;
@@ -116,6 +126,17 @@ void LinkWebBundle::NotifyLoaded() {
     owner_->ScheduleEvent();
 }
 
+void LinkWebBundle::OnWebBundleError(const String& message) {
+  if (!owner_)
+    return;
+  ExecutionContext* context = owner_->GetDocument().GetExecutionContext();
+  if (!context)
+    return;
+  context->AddConsoleMessage(MakeGarbageCollected<ConsoleMessage>(
+      mojom::blink::ConsoleMessageSource::kOther,
+      mojom::blink::ConsoleMessageLevel::kWarning, message));
+}
+
 void LinkWebBundle::Process() {
   if (!owner_ || !owner_->GetDocument().GetFrame())
     return;

third_party/blink/renderer/core/html/link_web_bundle.h

@@ -31,6 +31,7 @@ class CORE_EXPORT LinkWebBundle final : public LinkResource,
   void Trace(Visitor* visitor) const override;
 
   void NotifyLoaded();
+  void OnWebBundleError(const String& message);
 
   // LinkResource overrides:
   void Process() override;

third_party/blink/renderer/platform/loader/fetch/url_loader/web_bundle_subresource_loader.cc

@@ -14,6 +14,8 @@
 #include "mojo/public/cpp/system/data_pipe_producer.h"
 #include "services/network/public/mojom/url_loader.mojom.h"
 #include "services/network/public/mojom/url_loader_factory.mojom.h"
+#include "third_party/blink/renderer/platform/scheduler/public/post_cross_thread_task.h"
+#include "third_party/blink/renderer/platform/wtf/cross_thread_functional.h"
 #include "third_party/blink/renderer/platform/wtf/functional.h"
 #include "third_party/blink/renderer/platform/wtf/shared_buffer.h"
 
@@ -251,7 +253,11 @@ class WebBundleSubresourceLoaderFactory
  public:
   WebBundleSubresourceLoaderFactory(
       mojo::PendingReceiver<network::mojom::URLLoaderFactory> receiver,
-      mojo::ScopedDataPipeConsumerHandle bundle_body) {
+      mojo::ScopedDataPipeConsumerHandle bundle_body,
+      scoped_refptr<base::SingleThreadTaskRunner> callback_task_runner,
+      WebBundleErrorCallback error_callback)
+      : callback_task_runner_(callback_task_runner),
+        error_callback_(error_callback) {
     receivers_.Add(this, std::move(receiver));
     receivers_.set_disconnect_handler(base::BindRepeating(
         &WebBundleSubresourceLoaderFactory::OnMojoDisconnect,
@@ -322,6 +328,9 @@ class WebBundleSubresourceLoaderFactory
       return;
     auto it = metadata_->requests.find(loader->Url());
     if (it == metadata_->requests.end()) {
+      RunErrorCallback(WebBundleErrorType::kResourceNotFound,
+                       loader->Url().possibly_invalid_spec() +
+                           " is not found in the WebBundle.");
       loader->OnFail(net::ERR_INVALID_WEB_BUNDLE);
       return;
     }
@@ -340,7 +349,8 @@ class WebBundleSubresourceLoaderFactory
                  "WebBundleSubresourceLoaderFactory::OnMetadataParsed");
     if (error) {
       metadata_error_ = std::move(error);
-      // TODO(crbug.com/1082020): Log the error message to console output.
+      RunErrorCallback(WebBundleErrorType::kMetadataParseError,
+                       metadata_error_->message);
       for (auto loader : pending_loaders_) {
         if (loader)
           loader->OnFail(net::ERR_INVALID_WEB_BUNDLE);
@@ -363,7 +373,7 @@ class WebBundleSubresourceLoaderFactory
     if (!loader)
       return;
     if (error) {
-      // TODO(crbug.com/1082020): Log the error message to console output.
+      RunErrorCallback(WebBundleErrorType::kResponseParseError, error->message);
       loader->OnFail(net::ERR_INVALID_WEB_BUNDLE);
       return;
     }
@@ -383,12 +393,20 @@ class WebBundleSubresourceLoaderFactory
                        loader->GetWeakPtr()));
   }
 
+  void RunErrorCallback(WebBundleErrorType type, const std::string& message) {
+    PostCrossThreadTask(*callback_task_runner_, FROM_HERE,
+                        WTF::CrossThreadBindOnce(error_callback_, type,
+                                                 String(message.c_str())));
+  }
+
   mojo::ReceiverSet<network::mojom::URLLoaderFactory> receivers_;
   std::unique_ptr<LinkWebBundleDataSource> source_;
   mojo::Remote<web_package::mojom::WebBundleParser> parser_;
   web_package::mojom::BundleMetadataPtr metadata_;
   web_package::mojom::BundleMetadataParseErrorPtr metadata_error_;
   std::vector<base::WeakPtr<WebBundleSubresourceLoader>> pending_loaders_;
+  scoped_refptr<base::SingleThreadTaskRunner> callback_task_runner_;
+  WebBundleErrorCallback error_callback_;
 
   base::WeakPtrFactory<WebBundleSubresourceLoaderFactory> weak_ptr_factory_{
       this};
@@ -397,9 +415,12 @@ class WebBundleSubresourceLoaderFactory
 // Runs on a background thread.
 void CreateFactoryOnBackground(
     mojo::PendingReceiver<network::mojom::URLLoaderFactory> receiver,
-    mojo::ScopedDataPipeConsumerHandle bundle_body) {
-  new WebBundleSubresourceLoaderFactory(std::move(receiver),
-                                        std::move(bundle_body));
+    mojo::ScopedDataPipeConsumerHandle bundle_body,
+    scoped_refptr<base::SingleThreadTaskRunner> callback_task_runner,
+    WebBundleErrorCallback error_callback) {
+  new WebBundleSubresourceLoaderFactory(
+      std::move(receiver), std::move(bundle_body), callback_task_runner,
+      std::move(error_callback));
 }
 
 }  // namespace
@@ -407,13 +428,17 @@ void CreateFactoryOnBackground(
 void CreateWebBundleSubresourceLoaderFactory(
     CrossVariantMojoReceiver<network::mojom::URLLoaderFactoryInterfaceBase>
         factory_receiver,
-    mojo::ScopedDataPipeConsumerHandle bundle_body) {
+    mojo::ScopedDataPipeConsumerHandle bundle_body,
+    WebBundleErrorCallback error_callback) {
   auto task_runner = base::ThreadPool::CreateSequencedTaskRunner(
       {base::TaskPriority::USER_VISIBLE,
        base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN});
-  task_runner->PostTask(FROM_HERE, base::BindOnce(&CreateFactoryOnBackground,
-                                                  std::move(factory_receiver),
-                                                  std::move(bundle_body)));
+  task_runner->PostTask(
+      FROM_HERE,
+      base::BindOnce(&CreateFactoryOnBackground, std::move(factory_receiver),
+                     std::move(bundle_body),
+                     base::ThreadTaskRunnerHandle::Get(),
+                     std::move(error_callback)));
 }
 
 }  // namespace blink

third_party/blink/renderer/platform/loader/fetch/url_loader/web_bundle_subresource_loader.h

@@ -10,14 +10,29 @@
 #include "third_party/blink/public/platform/cross_variant_mojo_util.h"
 #include "third_party/blink/renderer/platform/platform_export.h"
 
+namespace WTF {
+class String;
+}  // namespace WTF
+
 namespace blink {
 
+enum class WebBundleErrorType {
+  kMetadataParseError,
+  kResponseParseError,
+  kResourceNotFound,
+};
+
+using WebBundleErrorCallback =
+    base::RepeatingCallback<void(WebBundleErrorType,
+                                 const WTF::String& message)>;
+
 // Creates a network::mojom::URLLoaderFactory that can load resources from a
 // WebBundle, and binds it to |factory_receiver|.
 PLATFORM_EXPORT void CreateWebBundleSubresourceLoaderFactory(
     CrossVariantMojoReceiver<network::mojom::URLLoaderFactoryInterfaceBase>
         factory_receiver,
-    mojo::ScopedDataPipeConsumerHandle bundle_body);
+    mojo::ScopedDataPipeConsumerHandle bundle_body,
+    WebBundleErrorCallback error_callback);
 
 }  // namespace blink
 

third_party/blink/renderer/platform/loader/fetch/url_loader/web_bundle_subresource_loader_test.cc

@@ -13,6 +13,7 @@
 #include "services/network/public/mojom/url_loader_factory.mojom.h"
 #include "services/network/test/test_url_loader_client.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/blink/renderer/platform/wtf/text/wtf_string.h"
 
 namespace blink {
 
@@ -55,7 +56,10 @@ class WebBundleSubresourceLoaderFactoryTest : public ::testing::Test {
     ASSERT_EQ(CreateDataPipe(nullptr, &bundle_data_destination_, &consumer),
               MOJO_RESULT_OK);
     CreateWebBundleSubresourceLoaderFactory(
-        loader_factory_.BindNewPipeAndPassReceiver(), std::move(consumer));
+        loader_factory_.BindNewPipeAndPassReceiver(), std::move(consumer),
+        base::BindRepeating(
+            &WebBundleSubresourceLoaderFactoryTest::OnWebBundleError,
+            base::Unretained(this)));
   }
 
   void WriteBundle(base::span<const uint8_t> data) {
@@ -75,6 +79,19 @@ class WebBundleSubresourceLoaderFactoryTest : public ::testing::Test {
     return StartRequestWithLoaderFactory(loader_factory_, url);
   }
 
+  void RunUntilBundleError() {
+    if (last_bundle_error_.has_value())
+      return;
+    base::RunLoop run_loop;
+    quit_closure_for_bundle_error_ = run_loop.QuitClosure();
+    run_loop.Run();
+  }
+
+  const base::Optional<std::pair<WebBundleErrorType, WTF::String>>&
+  last_bundle_error() const {
+    return last_bundle_error_;
+  }
+
  protected:
   StartRequestResult StartRequestWithLoaderFactory(
       mojo::Remote<network::mojom::URLLoaderFactory>& factory,
@@ -95,8 +112,16 @@ class WebBundleSubresourceLoaderFactoryTest : public ::testing::Test {
   mojo::Remote<network::mojom::URLLoaderFactory> loader_factory_;
 
  private:
+  void OnWebBundleError(WebBundleErrorType type, const WTF::String& message) {
+    last_bundle_error_ = std::make_pair(type, message);
+    if (quit_closure_for_bundle_error_)
+      std::move(quit_closure_for_bundle_error_).Run();
+  }
+
   mojo::ScopedDataPipeProducerHandle bundle_data_destination_;
   base::test::TaskEnvironment task_environment;
+  base::Optional<std::pair<WebBundleErrorType, WTF::String>> last_bundle_error_;
+  base::OnceClosure quit_closure_for_bundle_error_;
 };
 
 TEST_F(WebBundleSubresourceLoaderFactoryTest, Basic) {
@@ -107,6 +132,7 @@ TEST_F(WebBundleSubresourceLoaderFactoryTest, Basic) {
   request.client->RunUntilComplete();
 
   EXPECT_EQ(net::OK, request.client->completion_status().error_code);
+  EXPECT_FALSE(last_bundle_error().has_value());
   std::string body;
   EXPECT_TRUE(mojo::BlockingCopyToString(
       request.client->response_body_release(), &body));
@@ -125,6 +151,7 @@ TEST_F(WebBundleSubresourceLoaderFactoryTest, Clone) {
   request.client->RunUntilComplete();
 
   EXPECT_EQ(net::OK, request.client->completion_status().error_code);
+  EXPECT_FALSE(last_bundle_error().has_value());
 }
 
 TEST_F(WebBundleSubresourceLoaderFactoryTest, MetadataParseError) {
@@ -136,9 +163,13 @@ TEST_F(WebBundleSubresourceLoaderFactoryTest, MetadataParseError) {
   FinishWritingBundle();
 
   request.client->RunUntilComplete();
+  RunUntilBundleError();
 
   EXPECT_EQ(net::ERR_INVALID_WEB_BUNDLE,
             request.client->completion_status().error_code);
+  EXPECT_EQ(last_bundle_error()->first,
+            WebBundleErrorType::kMetadataParseError);
+  EXPECT_EQ(last_bundle_error()->second, "Wrong magic bytes.");
 
   // Requests made after metadata parse error should also fail.
   auto request2 = StartRequest(GURL(kResourceUrl));
@@ -158,9 +189,14 @@ TEST_F(WebBundleSubresourceLoaderFactoryTest, ResponseParseError) {
 
   auto request = StartRequest(GURL(kResourceUrl));
   request.client->RunUntilComplete();
+  RunUntilBundleError();
 
   EXPECT_EQ(net::ERR_INVALID_WEB_BUNDLE,
             request.client->completion_status().error_code);
+  EXPECT_EQ(last_bundle_error()->first,
+            WebBundleErrorType::kResponseParseError);
+  EXPECT_EQ(last_bundle_error()->second,
+            ":status must be 3 ASCII decimal digits.");
 }
 
 TEST_F(WebBundleSubresourceLoaderFactoryTest, ResourceNotFoundInBundle) {
@@ -169,9 +205,14 @@ TEST_F(WebBundleSubresourceLoaderFactoryTest, ResourceNotFoundInBundle) {
 
   auto request = StartRequest(GURL("https://example.com/no-such-resource"));
   request.client->RunUntilComplete();
+  RunUntilBundleError();
 
   EXPECT_EQ(net::ERR_INVALID_WEB_BUNDLE,
             request.client->completion_status().error_code);
+  EXPECT_EQ(last_bundle_error()->first, WebBundleErrorType::kResourceNotFound);
+  EXPECT_EQ(
+      last_bundle_error()->second,
+      "https://example.com/no-such-resource is not found in the WebBundle.");
 }
 
 TEST_F(WebBundleSubresourceLoaderFactoryTest, StartRequestBeforeReadingBundle) {
@@ -261,9 +302,13 @@ TEST_F(WebBundleSubresourceLoaderFactoryTest, TruncatedBundle) {
 
   auto request = StartRequest(GURL(kResourceUrl));
   request.client->RunUntilComplete();
+  RunUntilBundleError();
 
   EXPECT_EQ(net::ERR_INVALID_WEB_BUNDLE,
             request.client->completion_status().error_code);
+  EXPECT_EQ(last_bundle_error()->first,
+            WebBundleErrorType::kResponseParseError);
+  EXPECT_EQ(last_bundle_error()->second, "Error reading response header.");
 }
 
 }  // namespace blink

third_party/blink/web_tests/NeverFixTests

@@ -2147,6 +2147,8 @@ crbug.com/1018640 external/wpt/web-bundle/wbn-from-network/* [ Skip ]
 virtual/wbn-from-network/external/wpt/web-bundle/wbn-from-network/* [ Pass ]
 crbug.com/1082020 external/wpt/web-bundle/subresource-loading/* [ Skip ]
 virtual/subresource-web-bundles/external/wpt/web-bundle/subresource-loading/* [ Pass ]
+crbug.com/1082020 http/tests/loading/wbn/subresource-loading/* [ Skip ]
+virtual/subresource-web-bundles/http/tests/loading/wbn/subresource-loading/* [ Pass ]
 
 # No good way to automate this test yet
 external/wpt/native-file-system/showSaveFilePicker-manual.https.html [ Skip ]

third_party/blink/web_tests/VirtualTestSuites

@@ -743,7 +743,8 @@
   },
   {
     "prefix": "subresource-web-bundles",
-    "bases": [ "external/wpt/web-bundle/subresource-loading" ],
+    "bases": [ "external/wpt/web-bundle/subresource-loading",
+               "http/tests/loading/wbn/subresource-loading" ],
     "args": [ "--enable-blink-features=SubresourceWebBundles" ]
   },
   {

third_party/blink/web_tests/http/tests/loading/wbn/resources/generate-test-wbns.sh

@@ -12,6 +12,13 @@ if ! command -v gen-bundle > /dev/null 2>&1; then
     exit 1
 fi
 
+gen-bundle \
+  -version b1 \
+  -baseURL http://127.0.0.1:8000/loading/wbn/resources/server/hello/ \
+  -primaryURL http://127.0.0.1:8000/loading/wbn/resources/server/hello/script.js \
+  -dir hello/ \
+  -o wbn/hello.wbn
+
 gen-bundle \
   -version b1 \
   -baseURL https://localhost:8443/loading/wbn/resources/server/wbn-subresource-origin-trial/ \

third_party/blink/web_tests/http/tests/loading/wbn/resources/hello/script.js

+console.log('Hello from WebBundle');

third_party/blink/web_tests/http/tests/loading/wbn/resources/server/hello/notfound.js

+console.log('This resource exists only in server, not in bundle');

third_party/blink/web_tests/http/tests/loading/wbn/resources/server/hello/script.js

+console.log('Hello from Server');

third_party/blink/web_tests/http/tests/loading/wbn/subresource-loading/subresource-loading-expected.txt

+main frame - DidStartNavigation
+main frame - ReadyToCommitNavigation
+main frame - didCommitLoadForFrame
+main frame - TitleWasSet: Subresource loading with Web Bundles
+main frame - didFinishDocumentLoadForFrame
+main frame - didHandleOnloadEventsForFrame
+main frame - didFinishLoadForFrame
+CONSOLE MESSAGE: line 1: Hello from WebBundle
+CONSOLE WARNING: http://127.0.0.1:8000/loading/wbn/resources/wbn/hello.wbn: http://127.0.0.1:8000/loading/wbn/resources/server/hello/notfound.js is not found in the WebBundle.
+

third_party/blink/web_tests/http/tests/loading/wbn/subresource-loading/subresource-loading.html

+<!DOCTYPE html>
+<head>
+  <title>Subresource loading with Web Bundles</title>
+  <link rel="webbundle" href="../resources/wbn/hello.wbn"
+        resources="http://127.0.0.1:8000/loading/wbn/resources/server/hello/script.js
+                   http://127.0.0.1:8000/loading/wbn/resources/server/hello/notfound.js">
+</head>
+<body>
+<script>
+(async () => {
+if (window.testRunner) {
+  testRunner.dumpAsText();
+  testRunner.waitUntilDone();
+}
+
+// Delay running the test until "didFinishLoadForFrame" is printed.
+// This is intended to avoid the flakiness of the result outputs.
+await new Promise((resolve) => {
+    window.addEventListener('load', () => setTimeout(resolve, 0));
+  });
+
+if (!document.createElement('link').relList.supports('webbundle')) {
+  console.error("Subresource Web Bundles is not supported");
+  testRunner.notifyDone();
+  return;
+}
+
+await new Promise((resolve, reject) => {
+    const script = document.createElement('script');
+    script.src = 'http://127.0.0.1:8000/loading/wbn/resources/server/hello/script.js';
+    script.onload = resolve;
+    script.onerror = reject;
+    document.body.appendChild(script);
+  });
+
+await new Promise((resolve, reject) => {
+    // This request should fail rather than be loaded from the network.
+    const script = document.createElement('script');
+    script.src = 'http://127.0.0.1:8000/loading/wbn/resources/server/hello/notfound.js';
+    script.onload = reject;
+    script.onerror = resolve;
+    document.body.appendChild(script);
+  });
+
+testRunner.notifyDone();
+})();
+</script>
+</body>