Commit 984c0351 authored by Kunihiko Sakamoto's avatar Kunihiko Sakamoto Committed by Commit Bot

WebBundle: Remove SetWebBundleParserFactoryForTesting

crrev.com/c/1903166 introduced a new testing API for data_decoder_service
which allows tests to inject WebBundleParserFactory for testing.

This patch migrates existing unit tests to use it, and removes
SetWebBundleParserFactoryForTesting() from WebBundleReader and
SafeWebBundleParser.

Bug: 969596
Change-Id: I3e0da13a2202c25c177a24a16c2208fff27683b6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1935778Reviewed-by: default avatarTsuyoshi Horo <horo@chromium.org>
Reviewed-by: default avatarRobert Sesek <rsesek@chromium.org>
Commit-Queue: Kunihiko Sakamoto <ksakamoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#719438}
parent 0999c8d5
...@@ -20,6 +20,7 @@ ...@@ -20,6 +20,7 @@
#include "mojo/public/cpp/bindings/remote.h" #include "mojo/public/cpp/bindings/remote.h"
#include "mojo/public/cpp/bindings/self_owned_receiver.h" #include "mojo/public/cpp/bindings/self_owned_receiver.h"
#include "net/base/filename_util.h" #include "net/base/filename_util.h"
#include "services/data_decoder/public/cpp/test_support/in_process_data_decoder.h"
#include "services/data_decoder/public/mojom/web_bundle_parser.mojom.h" #include "services/data_decoder/public/mojom/web_bundle_parser.mojom.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
...@@ -85,6 +86,12 @@ class MockParserFactory final ...@@ -85,6 +86,12 @@ class MockParserFactory final
MockParserFactory() {} MockParserFactory() {}
~MockParserFactory() override = default; ~MockParserFactory() override = default;
void AddReceiver(
mojo::PendingReceiver<data_decoder::mojom::WebBundleParserFactory>
receiver) {
receivers_.Add(this, std::move(receiver));
}
void WaitUntilParseMetadataCalled(base::OnceClosure closure) { void WaitUntilParseMetadataCalled(base::OnceClosure closure) {
if (parser_) if (parser_)
parser_->WaitUntilParseMetadataCalled(std::move(closure)); parser_->WaitUntilParseMetadataCalled(std::move(closure));
...@@ -134,6 +141,7 @@ class MockParserFactory final ...@@ -134,6 +141,7 @@ class MockParserFactory final
} }
std::unique_ptr<MockParser> parser_; std::unique_ptr<MockParser> parser_;
mojo::ReceiverSet<data_decoder::mojom::WebBundleParserFactory> receivers_;
base::OnceClosure wait_parse_metadata_callback_; base::OnceClosure wait_parse_metadata_callback_;
base::OnceClosure wait_parse_response_callback_; base::OnceClosure wait_parse_response_callback_;
...@@ -147,6 +155,7 @@ class MockWebBundleReaderFactoryImpl final : public MockWebBundleReaderFactory { ...@@ -147,6 +155,7 @@ class MockWebBundleReaderFactoryImpl final : public MockWebBundleReaderFactory {
scoped_refptr<WebBundleReader> CreateReader( scoped_refptr<WebBundleReader> CreateReader(
const std::string& test_file_data) override { const std::string& test_file_data) override {
DCHECK(!factory_);
if (!temp_dir_.CreateUniqueTempDir() || if (!temp_dir_.CreateUniqueTempDir() ||
!CreateTemporaryFileInDir(temp_dir_.GetPath(), &temp_file_path_) || !CreateTemporaryFileInDir(temp_dir_.GetPath(), &temp_file_path_) ||
(test_file_data.size() != (test_file_data.size() !=
...@@ -159,13 +168,10 @@ class MockWebBundleReaderFactoryImpl final : public MockWebBundleReaderFactory { ...@@ -159,13 +168,10 @@ class MockWebBundleReaderFactoryImpl final : public MockWebBundleReaderFactory {
WebBundleSource::MaybeCreateFromTrustedFileUrl( WebBundleSource::MaybeCreateFromTrustedFileUrl(
net::FilePathToFileURL(temp_file_path_))); net::FilePathToFileURL(temp_file_path_)));
std::unique_ptr<MockParserFactory> factory = factory_ = std::make_unique<MockParserFactory>();
std::make_unique<MockParserFactory>(); in_process_data_decoder_.service()
factory_ = factory.get(); .SetWebBundleParserFactoryBinderForTesting(base::BindRepeating(
mojo::Remote<data_decoder::mojom::WebBundleParserFactory> remote; &MockParserFactory::AddReceiver, base::Unretained(factory_.get())));
mojo::MakeSelfOwnedReceiver(std::move(factory),
remote.BindNewPipeAndPassReceiver());
reader->SetWebBundleParserFactoryForTesting(std::move(remote));
return reader; return reader;
} }
...@@ -223,10 +229,11 @@ class MockWebBundleReaderFactoryImpl final : public MockWebBundleReaderFactory { ...@@ -223,10 +229,11 @@ class MockWebBundleReaderFactoryImpl final : public MockWebBundleReaderFactory {
} }
private: private:
data_decoder::test::InProcessDataDecoder in_process_data_decoder_;
std::string test_file_data_; std::string test_file_data_;
base::ScopedTempDir temp_dir_; base::ScopedTempDir temp_dir_;
base::FilePath temp_file_path_; base::FilePath temp_file_path_;
MockParserFactory* factory_ = nullptr; std::unique_ptr<MockParserFactory> factory_;
DISALLOW_COPY_AND_ASSIGN(MockWebBundleReaderFactoryImpl); DISALLOW_COPY_AND_ASSIGN(MockWebBundleReaderFactoryImpl);
}; };
......
...@@ -319,13 +319,6 @@ const WebBundleSource& WebBundleReader::source() const { ...@@ -319,13 +319,6 @@ const WebBundleSource& WebBundleReader::source() const {
return *source_; return *source_;
} }
void WebBundleReader::SetWebBundleParserFactoryForTesting(
mojo::Remote<data_decoder::mojom::WebBundleParserFactory> factory) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
parser_->SetWebBundleParserFactoryForTesting(std::move(factory));
}
void WebBundleReader::ReadMetadataInternal(MetadataCallback callback, void WebBundleReader::ReadMetadataInternal(MetadataCallback callback,
base::File file) { base::File file) {
DCHECK(source_->is_trusted_file() || source_->is_file()); DCHECK(source_->is_trusted_file() || source_->is_file());
......
...@@ -23,7 +23,6 @@ ...@@ -23,7 +23,6 @@
#include "mojo/public/cpp/system/data_pipe_producer.h" #include "mojo/public/cpp/system/data_pipe_producer.h"
#include "net/base/net_errors.h" #include "net/base/net_errors.h"
#include "services/data_decoder/public/cpp/safe_web_bundle_parser.h" #include "services/data_decoder/public/cpp/safe_web_bundle_parser.h"
#include "services/data_decoder/public/mojom/web_bundle_parser.mojom.h"
#include "services/network/public/mojom/url_loader.mojom.h" #include "services/network/public/mojom/url_loader.mojom.h"
#include "url/gurl.h" #include "url/gurl.h"
...@@ -83,9 +82,6 @@ class CONTENT_EXPORT WebBundleReader final ...@@ -83,9 +82,6 @@ class CONTENT_EXPORT WebBundleReader final
// Returns the WebBundleSource. // Returns the WebBundleSource.
const WebBundleSource& source() const; const WebBundleSource& source() const;
void SetWebBundleParserFactoryForTesting(
mojo::Remote<data_decoder::mojom::WebBundleParserFactory> factory);
private: private:
friend class base::RefCounted<WebBundleReader>; friend class base::RefCounted<WebBundleReader>;
......
...@@ -385,6 +385,7 @@ jumbo_static_library("test_support") { ...@@ -385,6 +385,7 @@ jumbo_static_library("test_support") {
"//net:test_support", "//net:test_support",
"//ppapi/c:c", "//ppapi/c:c",
"//services/audio/public/mojom", "//services/audio/public/mojom",
"//services/data_decoder/public/cpp:test_support",
"//services/data_decoder/public/mojom", "//services/data_decoder/public/mojom",
"//services/device/public/mojom", "//services/device/public/mojom",
......
...@@ -131,9 +131,4 @@ void SafeWebBundleParser::OnResponseParsed( ...@@ -131,9 +131,4 @@ void SafeWebBundleParser::OnResponseParsed(
std::move(callback).Run(std::move(response), std::move(error)); std::move(callback).Run(std::move(response), std::move(error));
} }
void SafeWebBundleParser::SetWebBundleParserFactoryForTesting(
mojo::Remote<mojom::WebBundleParserFactory> factory) {
factory_ = std::move(factory);
}
} // namespace data_decoder } // namespace data_decoder
...@@ -44,11 +44,6 @@ class SafeWebBundleParser { ...@@ -44,11 +44,6 @@ class SafeWebBundleParser {
uint64_t response_length, uint64_t response_length,
mojom::WebBundleParser::ParseResponseCallback callback); mojom::WebBundleParser::ParseResponseCallback callback);
// Sets alternative WebBundleParserFactory that will be used to create
// WebBundleParser for testing purpose.
void SetWebBundleParserFactoryForTesting(
mojo::Remote<mojom::WebBundleParserFactory> factory);
// Sets a callback to be called when the data_decoder service connection is // Sets a callback to be called when the data_decoder service connection is
// terminated. // terminated.
void SetDisconnectCallback(base::OnceClosure callback); void SetDisconnectCallback(base::OnceClosure callback);
......
...@@ -66,6 +66,10 @@ class MockFactory final : public mojom::WebBundleParserFactory { ...@@ -66,6 +66,10 @@ class MockFactory final : public mojom::WebBundleParserFactory {
}; };
MockFactory() {} MockFactory() {}
void AddReceiver(
mojo::PendingReceiver<mojom::WebBundleParserFactory> receiver) {
receivers_.Add(this, std::move(receiver));
}
MockParser* GetCreatedParser() { MockParser* GetCreatedParser() {
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
return parser_.get(); return parser_.get();
...@@ -85,6 +89,7 @@ class MockFactory final : public mojom::WebBundleParserFactory { ...@@ -85,6 +89,7 @@ class MockFactory final : public mojom::WebBundleParserFactory {
} }
std::unique_ptr<MockParser> parser_; std::unique_ptr<MockParser> parser_;
mojo::ReceiverSet<data_decoder::mojom::WebBundleParserFactory> receivers_;
DISALLOW_COPY_AND_ASSIGN(MockFactory); DISALLOW_COPY_AND_ASSIGN(MockFactory);
}; };
...@@ -108,19 +113,21 @@ class MockDataSource final : public mojom::BundleDataSource { ...@@ -108,19 +113,21 @@ class MockDataSource final : public mojom::BundleDataSource {
class SafeWebBundleParserTest : public testing::Test { class SafeWebBundleParserTest : public testing::Test {
public: public:
MockFactory* InitializeMockFactory(SafeWebBundleParser* parser) { MockFactory* InitializeMockFactory() {
std::unique_ptr<MockFactory> factory = std::make_unique<MockFactory>(); DCHECK(!factory_);
MockFactory* raw_factory = factory.get(); factory_ = std::make_unique<MockFactory>();
mojo::Remote<mojom::WebBundleParserFactory> remote;
mojo::MakeSelfOwnedReceiver(std::move(factory), in_process_data_decoder_.service()
remote.BindNewPipeAndPassReceiver()); .SetWebBundleParserFactoryBinderForTesting(base::BindRepeating(
parser->SetWebBundleParserFactoryForTesting(std::move(remote)); &MockFactory::AddReceiver, base::Unretained(factory_.get())));
return raw_factory;
return factory_.get();
} }
private: private:
base::test::TaskEnvironment task_environment_; base::test::TaskEnvironment task_environment_;
data_decoder::test::InProcessDataDecoder in_process_data_decoder_; data_decoder::test::InProcessDataDecoder in_process_data_decoder_;
std::unique_ptr<MockFactory> factory_;
}; };
TEST_F(SafeWebBundleParserTest, ParseGoldenFile) { TEST_F(SafeWebBundleParserTest, ParseGoldenFile) {
...@@ -219,7 +226,7 @@ TEST_F(SafeWebBundleParserTest, CallWithoutOpen) { ...@@ -219,7 +226,7 @@ TEST_F(SafeWebBundleParserTest, CallWithoutOpen) {
TEST_F(SafeWebBundleParserTest, UseMockFactory) { TEST_F(SafeWebBundleParserTest, UseMockFactory) {
SafeWebBundleParser parser; SafeWebBundleParser parser;
MockFactory* raw_factory = InitializeMockFactory(&parser); MockFactory* raw_factory = InitializeMockFactory();
EXPECT_FALSE(raw_factory->GetCreatedParser()); EXPECT_FALSE(raw_factory->GetCreatedParser());
base::File test_file = base::File test_file =
...@@ -242,7 +249,7 @@ TEST_F(SafeWebBundleParserTest, UseMockFactory) { ...@@ -242,7 +249,7 @@ TEST_F(SafeWebBundleParserTest, UseMockFactory) {
TEST_F(SafeWebBundleParserTest, ConnectionError) { TEST_F(SafeWebBundleParserTest, ConnectionError) {
SafeWebBundleParser parser; SafeWebBundleParser parser;
MockFactory* raw_factory = InitializeMockFactory(&parser); MockFactory* raw_factory = InitializeMockFactory();
mojo::PendingRemote<mojom::BundleDataSource> remote_data_source; mojo::PendingRemote<mojom::BundleDataSource> remote_data_source;
auto data_source = std::make_unique<MockDataSource>( auto data_source = std::make_unique<MockDataSource>(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment