Show WebBundles parse error message in DevTools Console

Bug: 969596
Change-Id: I7c7f209b7bad770fe94ae6d7baeaec57e3d329a2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1864754Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Kunihiko Sakamoto <ksakamoto@chromium.org>
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#706808}

content/browser/frame_host/navigation_request.cc

@@ -2063,14 +2063,15 @@ void NavigationRequest::OnStartChecksComplete(
                switches::kTrustableBundledExchangesFileUrl));
     bundled_exchanges_handle_ =
         bundled_exchanges_handle_tracker_->MaybeCreateBundledExchangesHandle(
-            common_params_->url);
+            common_params_->url, frame_tree_node_->frame_tree_node_id());
   }
   if (!bundled_exchanges_handle_ && bundled_exchanges_navigation_info_) {
     DCHECK(base::FeatureList::IsEnabled(features::kWebBundles) ||
            base::CommandLine::ForCurrentProcess()->HasSwitch(
                switches::kTrustableBundledExchangesFileUrl));
     bundled_exchanges_handle_ = BundledExchangesHandle::CreateForNavigationInfo(
-        bundled_exchanges_navigation_info_->Clone());
+        bundled_exchanges_navigation_info_->Clone(),
+        frame_tree_node_->frame_tree_node_id());
   }
   if (!bundled_exchanges_handle_) {
     if (bundled_exchanges_utils::CanLoadAsTrustableBundledExchangesFile(
@@ -2081,11 +2082,13 @@ void NavigationRequest::OnStartChecksComplete(
       // invalid character.
       if (source) {
         bundled_exchanges_handle_ =
-            BundledExchangesHandle::CreateForTrustableFile(std::move(source));
+            BundledExchangesHandle::CreateForTrustableFile(
+                std::move(source), frame_tree_node_->frame_tree_node_id());
       }
     } else if (bundled_exchanges_utils::CanLoadAsBundledExchangesFile(
                    common_params_->url)) {
-      bundled_exchanges_handle_ = BundledExchangesHandle::CreateForFile();
+      bundled_exchanges_handle_ = BundledExchangesHandle::CreateForFile(
+          frame_tree_node_->frame_tree_node_id());
     }
   }
 

content/browser/web_package/bundled_exchanges_browsertest.cc

@@ -363,14 +363,23 @@ IN_PROC_BROWSER_TEST_F(BundledExchangesTrustableFileNotFoundBrowserTest,
   if (!original_client_)
     return;
 
+  WebContents* web_contents = shell()->web_contents();
+  ConsoleObserverDelegate console_delegate(web_contents, "*");
+  web_contents->SetDelegate(&console_delegate);
   base::RunLoop run_loop;
-  FinishNavigationObserver finish_navigation_observer(shell()->web_contents(),
+  FinishNavigationObserver finish_navigation_observer(web_contents,
                                                       run_loop.QuitClosure());
-  EXPECT_FALSE(NavigateToURL(shell()->web_contents(), test_data_url()));
+  EXPECT_FALSE(NavigateToURL(web_contents, test_data_url()));
   run_loop.Run();
   ASSERT_TRUE(finish_navigation_observer.error_code());
   EXPECT_EQ(net::ERR_INVALID_BUNDLED_EXCHANGES,
             *finish_navigation_observer.error_code());
+  if (console_delegate.messages().empty())
+    console_delegate.Wait();
+
+  EXPECT_FALSE(console_delegate.messages().empty());
+  EXPECT_EQ("Failed to read metadata of Web Bundle file: FILE_ERROR_FAILED",
+            console_delegate.message());
 }
 
 class BundledExchangesFileBrowserTest
@@ -478,14 +487,25 @@ IN_PROC_BROWSER_TEST_P(BundledExchangesFileBrowserTest,
   const GURL test_data_url =
       GetTestUrlForFile(GetTestDataPath("invalid_bundled_exchanges.wbn"));
 
+  WebContents* web_contents = shell()->web_contents();
+  ConsoleObserverDelegate console_delegate(web_contents, "*");
+  web_contents->SetDelegate(&console_delegate);
+
   base::RunLoop run_loop;
-  FinishNavigationObserver finish_navigation_observer(shell()->web_contents(),
+  FinishNavigationObserver finish_navigation_observer(web_contents,
                                                       run_loop.QuitClosure());
-  EXPECT_FALSE(NavigateToURL(shell()->web_contents(), test_data_url));
+  EXPECT_FALSE(NavigateToURL(web_contents, test_data_url));
   run_loop.Run();
   ASSERT_TRUE(finish_navigation_observer.error_code());
   EXPECT_EQ(net::ERR_INVALID_BUNDLED_EXCHANGES,
             *finish_navigation_observer.error_code());
+
+  if (console_delegate.messages().empty())
+    console_delegate.Wait();
+
+  EXPECT_FALSE(console_delegate.messages().empty());
+  EXPECT_EQ("Failed to read metadata of Web Bundle file: Wrong magic bytes.",
+            console_delegate.message());
 }
 
 IN_PROC_BROWSER_TEST_P(BundledExchangesFileBrowserTest, NoLocalFileScheme) {

content/browser/web_package/bundled_exchanges_handle.cc

@@ -18,6 +18,8 @@
 #include "content/browser/web_package/bundled_exchanges_utils.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
+#include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/web_contents.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
 #include "mojo/public/cpp/bindings/receiver.h"
@@ -64,6 +66,19 @@ const net::NetworkTrafficAnnotationTag kTrafficAnnotation =
     "by Blink, but based on a user initiated navigation."
   )");
 
+void AddMetadataParseErrorMessageToConsole(
+    int frame_tree_node_id,
+    const data_decoder::mojom::BundleMetadataParseErrorPtr& metadata_error) {
+  WebContents* web_contents =
+      WebContents::FromFrameTreeNodeId(frame_tree_node_id);
+  if (!web_contents)
+    return;
+  web_contents->GetMainFrame()->AddMessageToConsole(
+      blink::mojom::ConsoleMessageLevel::kError,
+      std::string("Failed to read metadata of Web Bundle file: ") +
+          metadata_error->message);
+}
+
 // A class to provide a network::mojom::URLLoader interface to redirect a
 // request to the BundledExchanges to the main resource url.
 class PrimaryURLRedirectLoader final : public network::mojom::URLLoader {
@@ -129,8 +144,10 @@ class PrimaryURLRedirectLoader final : public network::mojom::URLLoader {
 //     StartResponse() to create the loader for the main resource.
 class InterceptorForFile final : public NavigationLoaderInterceptor {
  public:
-  explicit InterceptorForFile(DoneCallback done_callback)
-      : done_callback_(std::move(done_callback)) {}
+  explicit InterceptorForFile(DoneCallback done_callback,
+                              int frame_tree_node_id)
+      : done_callback_(std::move(done_callback)),
+        frame_tree_node_id_(frame_tree_node_id) {}
   ~InterceptorForFile() override {
     DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   }
@@ -188,6 +205,8 @@ class InterceptorForFile final : public NavigationLoaderInterceptor {
       data_decoder::mojom::BundleMetadataParseErrorPtr metadata_error) {
     DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
     if (metadata_error) {
+      AddMetadataParseErrorMessageToConsole(frame_tree_node_id_,
+                                            metadata_error);
       forwarding_client_->OnComplete(network::URLLoaderCompletionStatus(
           net::ERR_INVALID_BUNDLED_EXCHANGES));
       forwarding_client_.reset();
@@ -220,6 +239,7 @@ class InterceptorForFile final : public NavigationLoaderInterceptor {
   }
 
   DoneCallback done_callback_;
+  const int frame_tree_node_id_;
   scoped_refptr<BundledExchangesReader> reader_;
   GURL primary_url_;
   std::unique_ptr<BundledExchangesURLLoaderFactory> url_loader_factory_;
@@ -252,10 +272,12 @@ class InterceptorForFile final : public NavigationLoaderInterceptor {
 class InterceptorForTrustableFile final : public NavigationLoaderInterceptor {
  public:
   InterceptorForTrustableFile(std::unique_ptr<BundledExchangesSource> source,
-                              DoneCallback done_callback)
+                              DoneCallback done_callback,
+                              int frame_tree_node_id)
       : source_(std::move(source)),
         reader_(base::MakeRefCounted<BundledExchangesReader>(source_->Clone())),
-        done_callback_(std::move(done_callback)) {
+        done_callback_(std::move(done_callback)),
+        frame_tree_node_id_(frame_tree_node_id) {
     reader_->ReadMetadata(
         base::BindOnce(&InterceptorForTrustableFile::OnMetadataReady,
                        weak_factory_.GetWeakPtr()));
@@ -322,6 +344,7 @@ class InterceptorForTrustableFile final : public NavigationLoaderInterceptor {
     DCHECK(!url_loader_factory_);
 
     if (error) {
+      AddMetadataParseErrorMessageToConsole(frame_tree_node_id_, error);
       metadata_error_ = std::move(error);
     } else {
       primary_url_ = reader_->GetPrimaryURL();
@@ -339,6 +362,7 @@ class InterceptorForTrustableFile final : public NavigationLoaderInterceptor {
   std::unique_ptr<BundledExchangesSource> source_;
   scoped_refptr<BundledExchangesReader> reader_;
   DoneCallback done_callback_;
+  const int frame_tree_node_id_;
 
   network::ResourceRequest pending_resource_request_;
   network::mojom::URLLoaderRequest pending_request_;
@@ -514,11 +538,13 @@ class InterceptorForNavigationInfo final : public NavigationLoaderInterceptor {
  public:
   InterceptorForNavigationInfo(
       std::unique_ptr<BundledExchangesNavigationInfo> navigation_info,
-      DoneCallback done_callback)
+      DoneCallback done_callback,
+      int frame_tree_node_id)
       : reader_(base::MakeRefCounted<BundledExchangesReader>(
             navigation_info->source().Clone())),
         target_inner_url_(navigation_info->target_inner_url()),
-        done_callback_(std::move(done_callback)) {
+        done_callback_(std::move(done_callback)),
+        frame_tree_node_id_(frame_tree_node_id) {
     reader_->ReadMetadata(
         base::BindOnce(&InterceptorForNavigationInfo::OnMetadataReady,
                        weak_factory_.GetWeakPtr()));
@@ -571,6 +597,7 @@ class InterceptorForNavigationInfo final : public NavigationLoaderInterceptor {
     DCHECK(!url_loader_factory_);
 
     if (error) {
+      AddMetadataParseErrorMessageToConsole(frame_tree_node_id_, error);
       metadata_error_ = std::move(error);
     } else {
       url_loader_factory_ = std::make_unique<BundledExchangesURLLoaderFactory>(
@@ -587,6 +614,7 @@ class InterceptorForNavigationInfo final : public NavigationLoaderInterceptor {
   scoped_refptr<BundledExchangesReader> reader_;
   const GURL target_inner_url_;
   DoneCallback done_callback_;
+  const int frame_tree_node_id_;
 
   network::ResourceRequest pending_resource_request_;
   network::mojom::URLLoaderRequest pending_request_;
@@ -605,25 +633,28 @@ class InterceptorForNavigationInfo final : public NavigationLoaderInterceptor {
 }  // namespace
 
 // static
-std::unique_ptr<BundledExchangesHandle>
-BundledExchangesHandle::CreateForFile() {
+std::unique_ptr<BundledExchangesHandle> BundledExchangesHandle::CreateForFile(
+    int frame_tree_node_id) {
   auto handle = base::WrapUnique(new BundledExchangesHandle());
   handle->SetInterceptor(std::make_unique<InterceptorForFile>(
       base::BindOnce(&BundledExchangesHandle::OnBundledExchangesFileLoaded,
-                     handle->weak_factory_.GetWeakPtr())));
+                     handle->weak_factory_.GetWeakPtr()),
+      frame_tree_node_id));
   return handle;
 }
 
 // static
 std::unique_ptr<BundledExchangesHandle>
 BundledExchangesHandle::CreateForTrustableFile(
-    std::unique_ptr<BundledExchangesSource> source) {
+    std::unique_ptr<BundledExchangesSource> source,
+    int frame_tree_node_id) {
   DCHECK(source->is_trusted());
   auto handle = base::WrapUnique(new BundledExchangesHandle());
   handle->SetInterceptor(std::make_unique<InterceptorForTrustableFile>(
       std::move(source),
       base::BindOnce(&BundledExchangesHandle::OnBundledExchangesFileLoaded,
-                     handle->weak_factory_.GetWeakPtr())));
+                     handle->weak_factory_.GetWeakPtr()),
+      frame_tree_node_id));
   return handle;
 }
 
@@ -653,12 +684,14 @@ BundledExchangesHandle::CreateForTrackedNavigation(
 // static
 std::unique_ptr<BundledExchangesHandle>
 BundledExchangesHandle::CreateForNavigationInfo(
-    std::unique_ptr<BundledExchangesNavigationInfo> navigation_info) {
+    std::unique_ptr<BundledExchangesNavigationInfo> navigation_info,
+    int frame_tree_node_id) {
   auto handle = base::WrapUnique(new BundledExchangesHandle());
   handle->SetInterceptor(std::make_unique<InterceptorForNavigationInfo>(
       std::move(navigation_info),
       base::BindOnce(&BundledExchangesHandle::OnBundledExchangesFileLoaded,
-                     handle->weak_factory_.GetWeakPtr())));
+                     handle->weak_factory_.GetWeakPtr()),
+      frame_tree_node_id));
   return handle;
 }
 

content/browser/web_package/bundled_exchanges_handle.h

@@ -27,13 +27,16 @@ class NavigationLoaderInterceptor;
 // loading. Running on the UI thread.
 class BundledExchangesHandle {
  public:
-  static std::unique_ptr<BundledExchangesHandle> CreateForFile();
+  static std::unique_ptr<BundledExchangesHandle> CreateForFile(
+      int frame_tree_node_id);
   static std::unique_ptr<BundledExchangesHandle> CreateForTrustableFile(
-      std::unique_ptr<BundledExchangesSource> source);
+      std::unique_ptr<BundledExchangesSource> source,
+      int frame_tree_node_id);
   static std::unique_ptr<BundledExchangesHandle> CreateForTrackedNavigation(
       scoped_refptr<BundledExchangesReader> reader);
   static std::unique_ptr<BundledExchangesHandle> CreateForNavigationInfo(
-      std::unique_ptr<BundledExchangesNavigationInfo> navigation_info);
+      std::unique_ptr<BundledExchangesNavigationInfo> navigation_info,
+      int frame_tree_node_id);
 
   ~BundledExchangesHandle();
 

content/browser/web_package/bundled_exchanges_handle_tracker.cc

@@ -23,7 +23,8 @@ BundledExchangesHandleTracker::~BundledExchangesHandleTracker() = default;
 
 std::unique_ptr<BundledExchangesHandle>
 BundledExchangesHandleTracker::MaybeCreateBundledExchangesHandle(
-    const GURL& url) {
+    const GURL& url,
+    int frame_tree_node_id) {
   if (reader_->HasEntry(url))
     return BundledExchangesHandle::CreateForTrackedNavigation(reader_);
   if (!reader_->source().is_trusted() &&
@@ -33,7 +34,8 @@ BundledExchangesHandleTracker::MaybeCreateBundledExchangesHandle(
     // reloaded.
     return BundledExchangesHandle::CreateForNavigationInfo(
         std::make_unique<BundledExchangesNavigationInfo>(
-            reader_->source().Clone(), target_inner_url_));
+            reader_->source().Clone(), target_inner_url_),
+        frame_tree_node_id);
   }
   return nullptr;
 }

content/browser/web_package/bundled_exchanges_handle_tracker.h

@@ -25,7 +25,8 @@ class BundledExchangesHandleTracker {
   // if the bundled exchanges file contains the matching response. Otherwise
   // returns null.
   std::unique_ptr<BundledExchangesHandle> MaybeCreateBundledExchangesHandle(
-      const GURL& url);
+      const GURL& url,
+      int frame_tree_node_id);
 
  private:
   scoped_refptr<BundledExchangesReader> reader_;