Revert "BundledExchangesParser: Update the bundle format [1/2]"

This reverts commit d8c863a6.

Reason for revert: broke build, example: https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8905916412276160432/+/steps/compile__with_patch_/0/stdout

Original change's description:
> BundledExchangesParser: Update the bundle format [1/2]
> 
> This updates BundledExchangesParser to parse the new bundle format [1].
> 
> Changes:
> - Updated the magic header bytes [2]
> - Version field is added (we use implementation-specific version string
>   "b1\0\0", which matches gen-bundle's output [3])
> - Fallback URL (== Primary URL) field is added
> - ParseMetadata() returns error type ("format error" or "version error")
>   and fallback URL if available
> - Updated spec ref comments
> 
> The structure change of the index section is not reflected yet. It will
> be updated in the next CL.
> 
> The test bundle file (hello.wbn) is generated with gen-bundle of
> github.com/WICG/webpackage revision a3cef2c, which supports the new
> bundle format except for the new index section structure.
> 
> [1] https://github.com/WICG/webpackage/pull/450
> [2] https://github.com/WICG/webpackage/pull/454
> [3] https://github.com/WICG/webpackage/pull/458
> 
> Bug: 969596
> Change-Id: I95744ed00fdd09d369bb8648d1cdf62ca181d0dd
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1715487
> Reviewed-by: Robert Sesek <rsesek@chromium.org>
> Reviewed-by: Tsuyoshi Horo <horo@chromium.org>
> Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
> Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#684200}

TBR=horo@chromium.org,kinuko@chromium.org,toyoshim@chromium.org,ksakamoto@chromium.org,rsesek@chromium.org

Change-Id: Ifce2b07c15a57a2030887cbfaad5853cbc5af992
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 969596
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1737883Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Commit-Queue: Andrey Kosyakov <caseq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#684207}

content/browser/web_package/bundled_exchanges_reader_unittest.cc

@@ -163,9 +163,7 @@ class BundledExchangesReaderTest : public testing::Test {
     items.push_back(std::move(item));
 
     data_decoder::mojom::BundleMetadataPtr metadata =
-        data_decoder::mojom::BundleMetadata::New(GURL() /* primary_url */,
-                                                 std::move(items),
-                                                 GURL() /* manifest_url */);
+        data_decoder::mojom::BundleMetadata::New(std::move(items), GURL());
     factory_->RunMetadataCallback(std::move(metadata));
     run_loop.Run();
   }

services/data_decoder/bundled_exchanges_parser.cc

@@ -38,17 +38,8 @@ constexpr uint64_t kMaxResponseHeaderLength = 512 * 1024;
 // The initial buffer size for reading an item from the response section.
 constexpr uint64_t kInitialBufferSizeForResponse = 4096;
 
-// Step 2. of
-// https://wicg.github.io/webpackage/draft-yasskin-wpack-bundled-exchanges.html#load-metadata
 const uint8_t kBundleMagicBytes[] = {
-    0x86, 0x48, 0xF0, 0x9F, 0x8C, 0x90, 0xF0, 0x9F, 0x93, 0xA6,
-};
-
-// Step 7. of
-// https://wicg.github.io/webpackage/draft-yasskin-wpack-bundled-exchanges.html#load-metadata
-// We use an implementation-specific version string "b1\0\0".
-const uint8_t kVersionB1MagicBytes[] = {
-    0x44, 0x62, 0x31, 0x00, 0x00,
+    0x84, 0x48, 0xF0, 0x9F, 0x8C, 0x90, 0xF0, 0x9F, 0x93, 0xA6,
 };
 
 // Section names.
@@ -58,7 +49,6 @@ constexpr char kResponsesSection[] = "responses";
 // https://tools.ietf.org/html/draft-ietf-cbor-7049bis-05#section-3.1
 enum class CBORType {
   kByteString = 2,
-  kTextString = 3,
   kArray = 4,
 };
 
@@ -183,17 +173,6 @@ class InputReader {
     return result;
   }
 
-  base::Optional<base::StringPiece> ReadString(size_t n) {
-    auto bytes = ReadBytes(n);
-    if (!bytes)
-      return base::nullopt;
-    base::StringPiece str(reinterpret_cast<const char*>(bytes->data()),
-                          bytes->size());
-    if (!base::IsStringUTF8(str))
-      return base::nullopt;
-    return str;
-  }
-
   // Parses the type and argument of a CBOR item from the input head. If parsed
   // successfully and the type matches |expected_type|, returns the argument.
   // Otherwise returns nullopt.
@@ -290,23 +269,23 @@ class BundledExchangesParser::MetadataParser
   void DidGetSize(uint64_t size) {
     size_ = size;
 
-    // In the next step, we will parse `magic`, `version`, and the CBOR
-    // header of `primary-url`.
+    // In the next step, we will parse `magic`, `section-lengths`, and the CBOR
+    // header of `sections`.
     // https://wicg.github.io/webpackage/draft-yasskin-wpack-bundled-exchanges.html#top-level
-    const uint64_t length = std::min(size, sizeof(kBundleMagicBytes) +
-                                               sizeof(kVersionB1MagicBytes) +
-                                               kMaxCBORItemHeaderSize);
+    const uint64_t length =
+        std::min(size, sizeof(kBundleMagicBytes) + kMaxSectionLengthsCBORSize +
+                           kMaxCBORItemHeaderSize * 2);
     data_source_->Read(0, length,
-                       base::BindOnce(&MetadataParser::ParseMagicBytes,
+                       base::BindOnce(&MetadataParser::ParseBundleHeader,
                                       weak_factory_.GetWeakPtr(), length));
   }
 
-  // Step 1-4 of
+  // Step 1-15 of
   // https://wicg.github.io/webpackage/draft-yasskin-wpack-bundled-exchanges.html#load-metadata
-  void ParseMagicBytes(uint64_t expected_data_length,
-                       const base::Optional<std::vector<uint8_t>>& data) {
+  void ParseBundleHeader(uint64_t expected_data_length,
+                         const base::Optional<std::vector<uint8_t>>& data) {
     if (!data || data->size() != expected_data_length) {
-      RunErrorCallbackAndDestroy("Error reading bundle magic bytes.");
+      RunErrorCallbackAndDestroy("Error reading bundle header.");
       return;
     }
 
@@ -315,9 +294,9 @@ class BundledExchangesParser::MetadataParser
     InputReader input(*data);
 
     // Step 2. "If reading 10 bytes from stream returns an error or doesn't
-    // return the bytes with hex encoding "86 48 F0 9F 8C 90 F0 9F 93 A6"
-    // (the CBOR encoding of the 6-item array initial byte and 8-byte bytestring
-    // initial byte, followed by 🌐📦 in UTF-8), return a "format error"."
+    // return the bytes with hex encoding "84 48 F0 9F 8C 90 F0 9F 93 A6"
+    // (the CBOR encoding of the 4-item array initial byte and 8-byte bytestring
+    // initial byte, followed by 🌐📦 in UTF-8), return an error."
     const auto magic = input.ReadBytes(sizeof(kBundleMagicBytes));
     if (!magic ||
         !std::equal(magic->begin(), magic->end(), std::begin(kBundleMagicBytes),
@@ -326,125 +305,43 @@ class BundledExchangesParser::MetadataParser
       return;
     }
 
-    // Step 3. "Let version be the result of reading 5 bytes from stream. If
-    // this is an error, return a "format error"."
-    const auto version = input.ReadBytes(sizeof(kVersionB1MagicBytes));
-    if (!version) {
-      RunErrorCallbackAndDestroy("Cannot read version bytes.");
-      return;
-    }
-    if (!std::equal(version->begin(), version->end(),
-                    std::begin(kVersionB1MagicBytes),
-                    std::end(kVersionB1MagicBytes))) {
-      version_mismatch_ = true;
-      // Continue parsing until Step 7 where we get a fallback URL, and
-      // then return "version error" with the fallback URL.
-    }
-
-    // Step 4. "Let urlType and urlLength be the result of reading the type and
-    // argument of a CBOR item from stream (Section 3.5.3). If this is an error
-    // or urlType is not 3 (a CBOR text string), return a "format error"."
-    const auto url_length = input.ReadCBORHeader(CBORType::kTextString);
-    if (!url_length) {
-      RunErrorCallbackAndDestroy("Cannot parse the size of fallback URL.");
-      return;
-    }
-
-    // In the next step, we will parse the content of `primary-url`,
-    // `section-lengths`, and the CBOR header of `sections`.
-    const uint64_t length = std::min(
-        size_ - input.CurrentOffset(),
-        *url_length + kMaxSectionLengthsCBORSize + kMaxCBORItemHeaderSize * 2);
-    data_source_->Read(input.CurrentOffset(), length,
-                       base::BindOnce(&MetadataParser::ParseBundleHeader,
-                                      weak_factory_.GetWeakPtr(), length,
-                                      *url_length, input.CurrentOffset()));
-  }
-
-  // Step 5-20 of
-  // https://wicg.github.io/webpackage/draft-yasskin-wpack-bundled-exchanges.html#load-metadata
-  void ParseBundleHeader(uint64_t expected_data_length,
-                         uint64_t url_length,
-                         uint64_t offset_in_stream,
-                         const base::Optional<std::vector<uint8_t>>& data) {
-    if (!data || data->size() != expected_data_length) {
-      RunErrorCallbackAndDestroy("Error reading bundle header.");
-      return;
-    }
-    InputReader input(*data);
-
-    // Step 5. "Let fallbackUrlBytes be the result of reading urlLength bytes
-    // from stream. If this is an error, return a "format error"."
-    const auto fallback_url_string = input.ReadString(url_length);
-    if (!fallback_url_string) {
-      RunErrorCallbackAndDestroy("Cannot read fallback URL.");
-      return;
-    }
-
-    // Step 6. "Let fallbackUrl be the result of parsing ([URL]) the UTF-8
-    // decoding of fallbackUrlBytes with no base URL. If either the UTF-8
-    // decoding or parsing fails, return a "format error"."
-    // Note: ReadString() ensures that |fallback_url_string| is a valid UTF-8.
-    GURL fallback_url(*fallback_url_string);
-    if (!fallback_url.is_valid()) {
-      RunErrorCallbackAndDestroy("Cannot parse fallback URL.");
-      return;
-    }
-
-    // "Note: From this point forward, errors also include the fallback URL to
-    // help clients recover."
-    fallback_url_ = std::move(fallback_url);
-
-    // Step 7. "If version does not have the hex encoding "44 31 00 00 00" (the
-    // CBOR encoding of a 4-byte byte string holding an ASCII "1" followed by
-    // three 0 bytes), return a "version error" with fallbackUrl. "
-    // Note: We use an implementation-specific version string
-    // kVersionB1MagicBytes.
-    if (version_mismatch_) {
-      RunErrorCallbackAndDestroy(
-          "Version error: this implementation only supports "
-          "bundle format of version b1.",
-          mojom::BundleParseErrorType::kVersionError);
-      return;
-    }
-
-    // Step 8. "Let sectionLengthsLength be the result of getting the length of
+    // Step 3. "Let sectionLengthsLength be the result of getting the length of
     // the CBOR bytestring header from stream (Section 3.5.2). If this is an
-    // error, return a "format error" with fallbackUrl."
+    // error, return that error."
     const auto section_lengths_length =
         input.ReadCBORHeader(CBORType::kByteString);
     if (!section_lengths_length) {
       RunErrorCallbackAndDestroy("Cannot parse the size of section-lengths.");
       return;
     }
-    // Step 9. "If sectionLengthsLength is 8192 (8*1024) or greater, return a
-    // "format error" with fallbackUrl."
+    // Step 4. "If sectionLengthsLength is 8192 (8*1024) or greater, return an
+    // error."
     if (*section_lengths_length >= kMaxSectionLengthsCBORSize) {
       RunErrorCallbackAndDestroy(
           "The section-lengths CBOR must be smaller than 8192 bytes.");
       return;
     }
 
-    // Step 10. "Let sectionLengthsBytes be the result of reading
+    // Step 5. "Let sectionLengthsBytes be the result of reading
     // sectionLengthsLength bytes from stream. If sectionLengthsBytes is an
-    // error, return a "format error" with fallbackUrl."
+    // error, return that error."
     const auto section_lengths_bytes = input.ReadBytes(*section_lengths_length);
     if (!section_lengths_bytes) {
       RunErrorCallbackAndDestroy("Cannot read section-lengths.");
       return;
     }
 
-    // Step 11. "Let sectionLengths be the result of parsing one CBOR item
+    // Step 6. "Let sectionLengths be the result of parsing one CBOR item
     // (Section 3.5) from sectionLengthsBytes, matching the section-lengths
     // rule in the CDDL ([I-D.ietf-cbor-cddl]) above. If sectionLengths is an
-    // error, return a "format error" with fallbackUrl."
+    // error, return an error."
     const auto section_lengths = ParseSectionLengths(*section_lengths_bytes);
     if (!section_lengths) {
       RunErrorCallbackAndDestroy("Cannot parse section-lengths.");
       return;
     }
 
-    // Step 12. "Let (sectionsType, numSections) be the result of parsing the
+    // Step 7. "Let (sectionsType, numSections) be the result of parsing the
     // type and argument of a CBOR item from stream (Section 3.5.3)."
     const auto num_sections = input.ReadCBORHeader(CBORType::kArray);
     if (!num_sections) {
@@ -452,46 +349,44 @@ class BundledExchangesParser::MetadataParser
       return;
     }
 
-    // Step 13. "If sectionsType is not 4 (a CBOR array) or numSections is not
-    // half of the length of sectionLengths, return a "format error" with
-    // fallbackUrl."
+    // Step 8. "If sectionsType is not 4 (a CBOR array) or numSections is not
+    // half of the length of sectionLengths, return an error."
     if (*num_sections != section_lengths->size()) {
       RunErrorCallbackAndDestroy("Unexpected number of sections.");
       return;
     }
 
-    // Step 14. "Let sectionsStart be the current offset within stream."
-    // Note: This doesn't exceed |size_|.
-    const uint64_t sections_start = offset_in_stream + input.CurrentOffset();
+    // Step 9. "Let sectionsStart be the current offset within stream."
+    const uint64_t sections_start = input.CurrentOffset();
 
-    // Step 15. "Let knownSections be the subset of the Section 6.2 that this
+    // Step 10. "Let knownSections be the subset of the Section 6.2 that this
     // client has implemented."
-    // Step 16. "Let ignoredSections be an empty set."
+    // Step 11. "Let ignoredSections be an empty set."
 
     // This implementation doesn't use knownSections nor ignoredSections.
 
-    // Step 17. "Let sectionOffsets be an empty map ([INFRA]) from section names
+    // Step 12. "Let sectionOffsets be an empty map ([INFRA]) from section names
     // to (offset, length) pairs. These offsets are relative to the start of
     // stream."
 
     // |section_offsets_| is defined as a class member field.
 
-    // Step 18. "Let currentOffset be sectionsStart."
+    // Step 13. "Let currentOffset be sectionsStart."
     uint64_t current_offset = sections_start;
 
-    // Step 19. "For each ("name", length) pair of adjacent elements in
+    // Step 14. "For each ("name", length) pair of adjacent elements in
     // sectionLengths:"
     for (const auto& pair : *section_lengths) {
       const std::string& name = pair.first;
       const uint64_t length = pair.second;
-      // Step 19.1. "If "name"'s specification in knownSections says not to
+      // Step 14.1. "If "name"'s specification in knownSections says not to
       // process other sections, add those sections' names to ignoredSections."
 
       // There're no such sections at the moment.
 
-      // Step 19.2. "If sectionOffsets["name"] exists, return a "format error"
-      // with fallbackUrl. That is, duplicate sections are forbidden."
-      // Step 19.3. "Set sectionOffsets["name"] to (currentOffset, length)."
+      // Step 14.2. "If sectionOffsets["name"] exists, return an error. That is,
+      // duplicate sections are forbidden."
+      // Step 14.3. "Set sectionOffsets["name"] to (currentOffset, length)."
       bool added = section_offsets_
                        .insert(std::make_pair(
                            name, std::make_pair(current_offset, length)))
@@ -501,7 +396,7 @@ class BundledExchangesParser::MetadataParser
         return;
       }
 
-      // Step 19.4. "Set currentOffset to currentOffset + length."
+      // Step 14.4. "Set currentOffset to currentOffset + length."
       if (!base::CheckAdd(current_offset, length)
                .AssignIfValid(&current_offset) ||
           current_offset > size_) {
@@ -510,10 +405,9 @@ class BundledExchangesParser::MetadataParser
       }
     }
 
-    // Step 20. "If the "responses" section is not last in sectionLengths,
-    // return a "format error" with fallbackUrl. This allows a streaming parser
-    // to assume that it'll know the requests by the time their responses
-    // arrive."
+    // Step 15. "If the "responses" section is not last in sectionLengths,
+    // return an error. This allows a streaming parser to assume that it'll
+    // know the requests by the time their responses arrive."
     if (section_lengths->empty() ||
         section_lengths->back().first != kResponsesSection) {
       RunErrorCallbackAndDestroy(
@@ -724,8 +618,8 @@ class BundledExchangesParser::MetadataParser
     }
 
     // We're done.
-    RunSuccessCallbackAndDestroy(mojom::BundleMetadata::New(
-        fallback_url_, std::move(items_), manifest_url_));
+    RunSuccessCallbackAndDestroy(
+        mojom::BundleMetadata::New(std::move(items_), manifest_url_));
   }
 
   void RunSuccessCallbackAndDestroy(mojom::BundleMetadataPtr metadata) {
@@ -733,14 +627,9 @@ class BundledExchangesParser::MetadataParser
     delete this;
   }
 
-  void RunErrorCallbackAndDestroy(
-      const base::Optional<std::string>& error_message,
-      mojom::BundleParseErrorType error_type =
-          mojom::BundleParseErrorType::kFormatError) {
-    mojom::BundleMetadataParseErrorPtr err =
-        mojom::BundleMetadataParseError::New(error_type, fallback_url_,
-                                             *error_message);
-    std::move(callback_).Run(nullptr, std::move(err));
+  void RunErrorCallbackAndDestroy(const std::string& message) {
+    std::move(callback_).Run(nullptr,
+                             mojom::BundleMetadataParseError::New(message));
     delete this;
   }
 
@@ -752,8 +641,6 @@ class BundledExchangesParser::MetadataParser
   scoped_refptr<SharedBundleDataSource> data_source_;
   ParseMetadataCallback callback_;
   uint64_t size_;
-  bool version_mismatch_ = false;
-  GURL fallback_url_;
   // name -> (offset, length)
   std::map<std::string, std::pair<uint64_t, uint64_t>> section_offsets_;
   std::vector<mojom::BundleIndexItemPtr> items_;
@@ -927,12 +814,9 @@ class BundledExchangesParser::ResponseParser
     delete this;
   }
 
-  void RunErrorCallbackAndDestroy(
-      const std::string& message,
-      mojom::BundleParseErrorType error_type =
-          mojom::BundleParseErrorType::kFormatError) {
-    std::move(callback_).Run(
-        nullptr, mojom::BundleResponseParseError::New(error_type, message));
+  void RunErrorCallbackAndDestroy(const std::string& message) {
+    std::move(callback_).Run(nullptr,
+                             mojom::BundleResponseParseError::New(message));
     delete this;
   }
 

services/data_decoder/bundled_exchanges_parser_unittest.cc

@@ -18,8 +18,6 @@ namespace data_decoder {
 
 namespace {
 
-constexpr char kFallbackUrl[] = "https://test.example.com/";
-
 std::string GetTestFileContents(const base::FilePath& path) {
   base::FilePath test_data_dir;
   base::PathService::Get(base::DIR_SOURCE_ROOT, &test_data_dir);
@@ -64,10 +62,7 @@ class TestDataSource : public mojom::BundleDataSource {
   mojo::ReceiverSet<mojom::BundleDataSource> receivers_;
 };
 
-using ParseBundleResult =
-    std::pair<mojom::BundleMetadataPtr, mojom::BundleMetadataParseErrorPtr>;
-
-ParseBundleResult ParseBundle(TestDataSource* data_source) {
+mojom::BundleMetadataPtr ParseBundle(TestDataSource* data_source) {
   mojo::PendingRemote<mojom::BundleDataSource> source_remote;
   data_source->AddReceiver(source_remote.InitWithNewPipeAndPassReceiver());
 
@@ -77,25 +72,17 @@ ParseBundleResult ParseBundle(TestDataSource* data_source) {
   data_decoder::mojom::BundledExchangesParser& parser = parser_impl;
 
   base::RunLoop run_loop;
-  ParseBundleResult result;
+  mojom::BundleMetadataPtr result;
   parser.ParseMetadata(base::BindLambdaForTesting(
       [&result, &run_loop](mojom::BundleMetadataPtr metadata,
                            mojom::BundleMetadataParseErrorPtr error) {
-        result = std::make_pair(std::move(metadata), std::move(error));
+        result = std::move(metadata);
         run_loop.QuitClosure().Run();
       }));
   run_loop.Run();
-  EXPECT_TRUE((result.first && !result.second) ||
-              (!result.first && result.second));
   return result;
 }
 
-void ExpectFormatErrorWithFallbackURL(ParseBundleResult result) {
-  ASSERT_TRUE(result.second);
-  EXPECT_EQ(result.second->type, mojom::BundleParseErrorType::kFormatError);
-  EXPECT_EQ(result.second->fallback_url, kFallbackUrl);
-}
-
 mojom::BundleResponsePtr ParseResponse(TestDataSource* data_source,
                                        const mojom::BundleIndexItemPtr& item) {
   mojo::PendingRemote<mojom::BundleDataSource> source_remote;
@@ -124,16 +111,12 @@ class BundleBuilder {
  public:
   using Headers = std::vector<std::pair<std::string, std::string>>;
 
-  explicit BundleBuilder(const std::string& fallback_url)
-      : fallback_url_(fallback_url) {
-    writer_config_.allow_invalid_utf8_for_testing = true;
-  }
-
   void AddExchange(const Headers& request_headers,
                    const Headers& response_headers,
                    base::StringPiece payload) {
     cbor::Value::ArrayValue response_array;
-    response_array.emplace_back(Encode(CreateHeaderMap(response_headers)));
+    response_array.emplace_back(
+        *cbor::Writer::Write(CreateHeaderMap(response_headers)));
     response_array.emplace_back(CreateByteString(payload));
     cbor::Value response(response_array);
 
@@ -151,7 +134,7 @@ class BundleBuilder {
   std::vector<uint8_t> CreateBundle() {
     AddSection("index", cbor::Value(index_));
     AddSection("responses", cbor::Value(responses_));
-    return Encode(CreateTopLevel());
+    return *cbor::Writer::Write(CreateTopLevel());
   }
 
  private:
@@ -171,25 +154,16 @@ class BundleBuilder {
     toplevel_array.emplace_back(
         CreateByteString(u8"\U0001F310\U0001F4E6"));  // "🌐📦"
     toplevel_array.emplace_back(
-        CreateByteString(base::StringPiece("b1\0\0", 4)));
-    toplevel_array.emplace_back(
-        cbor::Value::InvalidUTF8StringValueForTesting(fallback_url_));
-    toplevel_array.emplace_back(Encode(cbor::Value(section_lengths_)));
+        *cbor::Writer::Write(cbor::Value(section_lengths_)));
     toplevel_array.emplace_back(sections_);
     toplevel_array.emplace_back(CreateByteString(""));  // length (ignored)
     return cbor::Value(toplevel_array);
   }
 
-  std::vector<uint8_t> Encode(const cbor::Value& value) {
-    return *cbor::Writer::Write(value, writer_config_);
+  static int64_t EncodedLength(const cbor::Value& value) {
+    return cbor::Writer::Write(value)->size();
   }
 
-  int64_t EncodedLength(const cbor::Value& value) {
-    return Encode(value).size();
-  }
-
-  cbor::Writer::Config writer_config_;
-  std::string fallback_url_;
   cbor::Value::ArrayValue section_lengths_;
   cbor::Value::ArrayValue sections_;
   cbor::Value::ArrayValue index_;
@@ -204,10 +178,10 @@ class BundledExchangeParserTest : public testing::Test {
 };
 
 TEST_F(BundledExchangeParserTest, EmptyBundle) {
-  BundleBuilder builder(kFallbackUrl);
+  BundleBuilder builder;
   TestDataSource data_source(builder.CreateBundle());
 
-  mojom::BundleMetadataPtr metadata = ParseBundle(&data_source).first;
+  mojom::BundleMetadataPtr metadata = ParseBundle(&data_source);
   ASSERT_TRUE(metadata);
   const auto& index = metadata->index;
 
@@ -215,68 +189,40 @@ TEST_F(BundledExchangeParserTest, EmptyBundle) {
 }
 
 TEST_F(BundledExchangeParserTest, WrongMagic) {
-  BundleBuilder builder(kFallbackUrl);
+  BundleBuilder builder;
   std::vector<uint8_t> bundle = builder.CreateBundle();
   bundle[3] ^= 1;
   TestDataSource data_source(bundle);
 
-  mojom::BundleMetadataParseErrorPtr error = ParseBundle(&data_source).second;
-  ASSERT_TRUE(error);
-  EXPECT_EQ(error->type, mojom::BundleParseErrorType::kFormatError);
-  EXPECT_TRUE(error->fallback_url.is_empty());
-}
-
-TEST_F(BundledExchangeParserTest, UnknownVersion) {
-  BundleBuilder builder(kFallbackUrl);
-  std::vector<uint8_t> bundle = builder.CreateBundle();
-  // Modify the version string from "b1\0\0" to "q1\0\0".
-  ASSERT_EQ(bundle[11], 'b');
-  bundle[11] = 'q';
-  TestDataSource data_source(bundle);
-
-  mojom::BundleMetadataParseErrorPtr error = ParseBundle(&data_source).second;
-  ASSERT_TRUE(error);
-  EXPECT_EQ(error->type, mojom::BundleParseErrorType::kVersionError);
-  EXPECT_EQ(error->fallback_url, kFallbackUrl);
-}
-
-TEST_F(BundledExchangeParserTest, FallbackURLIsNotUTF8) {
-  BundleBuilder builder("https://test.example.com/\xcc");
-  std::vector<uint8_t> bundle = builder.CreateBundle();
-  TestDataSource data_source(bundle);
-
-  mojom::BundleMetadataParseErrorPtr error = ParseBundle(&data_source).second;
-  ASSERT_TRUE(error);
-  EXPECT_EQ(error->type, mojom::BundleParseErrorType::kFormatError);
-  EXPECT_TRUE(error->fallback_url.is_empty());
+  ASSERT_FALSE(ParseBundle(&data_source));
 }
 
 TEST_F(BundledExchangeParserTest, SectionLengthsTooLarge) {
-  BundleBuilder builder(kFallbackUrl);
+  BundleBuilder builder;
   std::string too_long_section_name(8192, 'x');
   builder.AddSection(too_long_section_name, cbor::Value(0));
   TestDataSource data_source(builder.CreateBundle());
 
-  ExpectFormatErrorWithFallbackURL(ParseBundle(&data_source));
+  ASSERT_FALSE(ParseBundle(&data_source));
 }
 
 TEST_F(BundledExchangeParserTest, DuplicateSectionName) {
-  BundleBuilder builder(kFallbackUrl);
+  BundleBuilder builder;
   builder.AddSection("foo", cbor::Value(0));
   builder.AddSection("foo", cbor::Value(0));
   TestDataSource data_source(builder.CreateBundle());
 
-  ExpectFormatErrorWithFallbackURL(ParseBundle(&data_source));
+  ASSERT_FALSE(ParseBundle(&data_source));
 }
 
 TEST_F(BundledExchangeParserTest, SingleEntry) {
-  BundleBuilder builder(kFallbackUrl);
+  BundleBuilder builder;
   builder.AddExchange(
       {{":method", "GET"}, {":url", "https://test.example.com/"}},
       {{":status", "200"}, {"content-type", "text/plain"}}, "payload");
   TestDataSource data_source(builder.CreateBundle());
 
-  mojom::BundleMetadataPtr metadata = ParseBundle(&data_source).first;
+  mojom::BundleMetadataPtr metadata = ParseBundle(&data_source);
   ASSERT_TRUE(metadata);
   const auto& index = metadata->index;
 
@@ -294,57 +240,62 @@ TEST_F(BundledExchangeParserTest, SingleEntry) {
 }
 
 TEST_F(BundledExchangeParserTest, InvalidRequestURL) {
-  BundleBuilder builder(kFallbackUrl);
+  BundleBuilder builder;
   builder.AddExchange({{":method", "GET"}, {":url", ""}},
                       {{":status", "200"}, {"content-type", "text/plain"}},
                       "payload");
   TestDataSource data_source(builder.CreateBundle());
 
-  ExpectFormatErrorWithFallbackURL(ParseBundle(&data_source));
+  mojom::BundleMetadataPtr metadata = ParseBundle(&data_source);
+  ASSERT_FALSE(metadata);
 }
 
 TEST_F(BundledExchangeParserTest, RequestURLHasCredentials) {
-  BundleBuilder builder(kFallbackUrl);
+  BundleBuilder builder;
   builder.AddExchange(
       {{":method", "GET"}, {":url", "https://user:passwd@test.example.com/"}},
       {{":status", "200"}, {"content-type", "text/plain"}}, "payload");
   TestDataSource data_source(builder.CreateBundle());
 
-  ExpectFormatErrorWithFallbackURL(ParseBundle(&data_source));
+  mojom::BundleMetadataPtr metadata = ParseBundle(&data_source);
+  ASSERT_FALSE(metadata);
 }
 
 TEST_F(BundledExchangeParserTest, RequestURLHasFragment) {
-  BundleBuilder builder(kFallbackUrl);
+  BundleBuilder builder;
   builder.AddExchange(
       {{":method", "GET"}, {":url", "https://test.example.com/#fragment"}},
       {{":status", "200"}, {"content-type", "text/plain"}}, "payload");
   TestDataSource data_source(builder.CreateBundle());
 
-  ExpectFormatErrorWithFallbackURL(ParseBundle(&data_source));
+  mojom::BundleMetadataPtr metadata = ParseBundle(&data_source);
+  ASSERT_FALSE(metadata);
 }
 
 TEST_F(BundledExchangeParserTest, NoMethodInRequestHeaders) {
-  BundleBuilder builder(kFallbackUrl);
+  BundleBuilder builder;
   builder.AddExchange(
       {{":url", "https://test.example.com/"}},  // ":method" is missing.
       {{":status", "200"}, {"content-type", "text/plain"}}, "payload");
   TestDataSource data_source(builder.CreateBundle());
 
-  ExpectFormatErrorWithFallbackURL(ParseBundle(&data_source));
+  mojom::BundleMetadataPtr metadata = ParseBundle(&data_source);
+  ASSERT_FALSE(metadata);
 }
 
 TEST_F(BundledExchangeParserTest, MethodIsNotGET) {
-  BundleBuilder builder(kFallbackUrl);
+  BundleBuilder builder;
   builder.AddExchange(
       {{":method", "POST"}, {":url", "https://test.example.com/"}},
       {{":status", "200"}, {"content-type", "text/plain"}}, "payload");
   TestDataSource data_source(builder.CreateBundle());
 
-  ExpectFormatErrorWithFallbackURL(ParseBundle(&data_source));
+  mojom::BundleMetadataPtr metadata = ParseBundle(&data_source);
+  ASSERT_FALSE(metadata);
 }
 
 TEST_F(BundledExchangeParserTest, ExtraPseudoInRequestHeaders) {
-  BundleBuilder builder(kFallbackUrl);
+  BundleBuilder builder;
   builder.AddExchange({{":method", "GET"},
                        {":url", "https://test.example.com/"},
                        {":scheme", "https"}},
@@ -352,17 +303,17 @@ TEST_F(BundledExchangeParserTest, ExtraPseudoInRequestHeaders) {
                       "payload");
   TestDataSource data_source(builder.CreateBundle());
 
-  ExpectFormatErrorWithFallbackURL(ParseBundle(&data_source));
+  ASSERT_FALSE(ParseBundle(&data_source));
 }
 
 TEST_F(BundledExchangeParserTest, NoStatusInResponseHeaders) {
-  BundleBuilder builder(kFallbackUrl);
+  BundleBuilder builder;
   builder.AddExchange(
       {{":method", "GET"}, {":url", "https://test.example.com/"}},
       {{"content-type", "text/plain"}}, "payload");  // ":status" is missing.
   TestDataSource data_source(builder.CreateBundle());
 
-  mojom::BundleMetadataPtr metadata = ParseBundle(&data_source).first;
+  mojom::BundleMetadataPtr metadata = ParseBundle(&data_source);
   ASSERT_TRUE(metadata);
   ASSERT_EQ(metadata->index.size(), 1u);
 
@@ -370,13 +321,13 @@ TEST_F(BundledExchangeParserTest, NoStatusInResponseHeaders) {
 }
 
 TEST_F(BundledExchangeParserTest, InvalidResponseStatus) {
-  BundleBuilder builder(kFallbackUrl);
+  BundleBuilder builder;
   builder.AddExchange(
       {{":method", "GET"}, {":url", "https://test.example.com/"}},
       {{":status", "0200"}, {"content-type", "text/plain"}}, "payload");
   TestDataSource data_source(builder.CreateBundle());
 
-  mojom::BundleMetadataPtr metadata = ParseBundle(&data_source).first;
+  mojom::BundleMetadataPtr metadata = ParseBundle(&data_source);
   ASSERT_TRUE(metadata);
   ASSERT_EQ(metadata->index.size(), 1u);
 
@@ -384,14 +335,14 @@ TEST_F(BundledExchangeParserTest, InvalidResponseStatus) {
 }
 
 TEST_F(BundledExchangeParserTest, ExtraPseudoInResponseHeaders) {
-  BundleBuilder builder(kFallbackUrl);
+  BundleBuilder builder;
   builder.AddExchange(
       {{":method", "GET"}, {":url", "https://test.example.com/"}},
       {{":status", "200"}, {":foo", ""}, {"content-type", "text/plain"}},
       "payload");
   TestDataSource data_source(builder.CreateBundle());
 
-  mojom::BundleMetadataPtr metadata = ParseBundle(&data_source).first;
+  mojom::BundleMetadataPtr metadata = ParseBundle(&data_source);
   ASSERT_TRUE(metadata);
   ASSERT_EQ(metadata->index.size(), 1u);
 
@@ -399,13 +350,13 @@ TEST_F(BundledExchangeParserTest, ExtraPseudoInResponseHeaders) {
 }
 
 TEST_F(BundledExchangeParserTest, UpperCaseCharacterInHeaderName) {
-  BundleBuilder builder(kFallbackUrl);
+  BundleBuilder builder;
   builder.AddExchange(
       {{":method", "GET"}, {":url", "https://test.example.com/"}},
       {{":status", "200"}, {"Content-Type", "text/plain"}}, "payload");
   TestDataSource data_source(builder.CreateBundle());
 
-  mojom::BundleMetadataPtr metadata = ParseBundle(&data_source).first;
+  mojom::BundleMetadataPtr metadata = ParseBundle(&data_source);
   ASSERT_TRUE(metadata);
   ASSERT_EQ(metadata->index.size(), 1u);
 
@@ -413,13 +364,13 @@ TEST_F(BundledExchangeParserTest, UpperCaseCharacterInHeaderName) {
 }
 
 TEST_F(BundledExchangeParserTest, InvalidHeaderValue) {
-  BundleBuilder builder(kFallbackUrl);
+  BundleBuilder builder;
   builder.AddExchange(
       {{":method", "GET"}, {":url", "https://test.example.com/"}},
       {{":status", "200"}, {"content-type", "\n"}}, "payload");
   TestDataSource data_source(builder.CreateBundle());
 
-  mojom::BundleMetadataPtr metadata = ParseBundle(&data_source).first;
+  mojom::BundleMetadataPtr metadata = ParseBundle(&data_source);
   ASSERT_TRUE(metadata);
   ASSERT_EQ(metadata->index.size(), 1u);
 
@@ -429,7 +380,7 @@ TEST_F(BundledExchangeParserTest, InvalidHeaderValue) {
 TEST_F(BundledExchangeParserTest, ParseGoldenFile) {
   TestDataSource data_source(base::FilePath(FILE_PATH_LITERAL("hello.wbn")));
 
-  mojom::BundleMetadataPtr metadata = ParseBundle(&data_source).first;
+  mojom::BundleMetadataPtr metadata = ParseBundle(&data_source);
   ASSERT_TRUE(metadata);
   ASSERT_EQ(metadata->index.size(), 4u);
 

services/data_decoder/public/cpp/safe_bundled_exchanges_parser.cc

@@ -60,9 +60,7 @@ void SafeBundledExchangesParser::ParseMetadata(
   // simultaneous request is fine enough.
   if (disconnected_ || !metadata_callback_.is_null()) {
     std::move(callback).Run(
-        nullptr, mojom::BundleMetadataParseError::New(
-                     mojom::BundleParseErrorType::kParserInternalError,
-                     GURL() /* fallback_url */, kConnectionError));
+        nullptr, mojom::BundleMetadataParseError::New(kConnectionError));
     return;
   }
   metadata_callback_ = std::move(callback);
@@ -80,9 +78,7 @@ void SafeBundledExchangesParser::ParseResponse(
   if (disconnected_ ||
       response_callbacks_.contains(response_callback_next_id_)) {
     std::move(callback).Run(
-        nullptr, mojom::BundleResponseParseError::New(
-                     mojom::BundleParseErrorType::kParserInternalError,
-                     kConnectionError));
+        nullptr, mojom::BundleResponseParseError::New(kConnectionError));
     return;
   }
   size_t callback_id = response_callback_next_id_++;
@@ -97,14 +93,10 @@ void SafeBundledExchangesParser::OnDisconnect() {
   disconnected_ = true;
   if (!metadata_callback_.is_null())
     std::move(metadata_callback_)
-        .Run(nullptr, mojom::BundleMetadataParseError::New(
-                          mojom::BundleParseErrorType::kParserInternalError,
-                          GURL() /* fallback_url */, kConnectionError));
+        .Run(nullptr, mojom::BundleMetadataParseError::New(kConnectionError));
   for (auto& callback : response_callbacks_)
     std::move(callback.second)
-        .Run(nullptr, mojom::BundleResponseParseError::New(
-                          mojom::BundleParseErrorType::kParserInternalError,
-                          kConnectionError));
+        .Run(nullptr, mojom::BundleResponseParseError::New(kConnectionError));
   response_callbacks_.clear();
 }
 

services/data_decoder/public/mojom/bundled_exchanges_parser.mojom

@@ -38,26 +38,16 @@ interface BundleDataSource {
   Read(uint64 offset, uint64 length) => (array<uint8>? buffer);
 };
 
-enum BundleParseErrorType {
-  kParserInternalError,
-  kFormatError,
-  kVersionError,
-};
-
 struct BundleMetadataParseError {
-  BundleParseErrorType type;
-  url.mojom.Url fallback_url;
   string message;
+  // TODO(crbug.com/969596): Add fields for error type and fallback URL.
 };
 
 struct BundleResponseParseError {
-  BundleParseErrorType type;
   string message;
 };
 
-// https://wicg.github.io/webpackage/draft-yasskin-wpack-bundled-exchanges.html#semantics-load-metadata
 struct BundleMetadata {
-  url.mojom.Url primary_url;
   array<BundleIndexItem> index;
   url.mojom.Url manifest_url;
 };

services/test/data/bundled_exchanges/generate-test-wbns.sh

@@ -12,8 +12,7 @@ if ! command -v gen-bundle > /dev/null 2>&1; then
     exit 1
 fi
 
-gen-bundle -version b1 \
-           -baseURL https://test.example.org/ \
+gen-bundle -baseURL https://test.example.org/ \
            -dir hello/ \
            -manifestURL manifest.webmanifest \
            -o hello.wbn