iwa: Add `IwaSource*WithMode::WithFileOp`, rework source inheritance
This CL does two things:
1. The new `IwaSource(Bundle)WithMode::WithFileOp` method closes the
missing link between sources with mode and sources with mode and file
op.
2. We previously inherited all bundle-based sources from
`IwaSourceBundle`. This was nice for sharing code and allowing
implicit conversions, but allowed you to write code like the
following, which doesn't really make sense:
```
IwaSourceBundleProdModeWithFileOp bundle;
IwaSourceBundleDevModeWithFileOp new_bundle =
bundle.WithDevModeFileOp(
IwaSourceBundleDevFileOp::kReference
);
```
With the changes in this CL, you'd have to be more explicit if you
_really_ wanted to achieve this:
```
IwaSourceBundleProdModeWithFileOp bundle;
IwaSourceBundleDevModeWithFileOp new_bundle(
bundle.path(),
IwaSourceBundleDevFileOp::kReference
);
```
Bug: None
Change-Id: Ibb202bc34f4a56a396270618fca5ba72e53ed123
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5372845
Commit-Queue: Robbie McElrath <rmcelrath@chromium.org>
Auto-Submit: Christian Flach <cmfcmf@chromium.org>
Reviewed-by: Robbie McElrath <rmcelrath@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1273492}
diff --git a/chrome/browser/web_applications/isolated_web_apps/isolated_web_app_source.cc b/chrome/browser/web_applications/isolated_web_apps/isolated_web_app_source.cc
index fe75e0c..5a419d4b 100644
--- a/chrome/browser/web_applications/isolated_web_apps/isolated_web_app_source.cc
+++ b/chrome/browser/web_applications/isolated_web_apps/isolated_web_app_source.cc
@@ -93,10 +93,26 @@
}
}
-IwaSourceBundle::IwaSourceBundle(base::FilePath path)
+namespace internal {
+
+IwaSourceBundleBase::IwaSourceBundleBase(base::FilePath path)
: path_(std::move(path)) {}
+IwaSourceBundleBase::~IwaSourceBundleBase() = default;
+
+bool IwaSourceBundleBase::operator==(const IwaSourceBundleBase&) const =
+ default;
+
+} // namespace internal
+
+IwaSourceBundle::IwaSourceBundle(base::FilePath path)
+ : internal::IwaSourceBundleBase(std::move(path)) {}
IwaSourceBundle::~IwaSourceBundle() = default;
+IwaSourceBundle::IwaSourceBundle(IwaSourceBundleWithMode other)
+ : internal::IwaSourceBundleBase(std::move(other.path_)) {}
+IwaSourceBundle::IwaSourceBundle(IwaSourceBundleWithModeAndFileOp other)
+ : internal::IwaSourceBundleBase(std::move(other.path_)) {}
+
bool IwaSourceBundle::operator==(const IwaSourceBundle& other) const = default;
IwaSourceBundleWithModeAndFileOp IwaSourceBundle::WithModeAndFileOp(
@@ -125,7 +141,7 @@
IwaSourceBundleWithMode::IwaSourceBundleWithMode(base::FilePath path,
bool dev_mode)
- : IwaSourceBundle(std::move(path)), dev_mode_(dev_mode) {}
+ : internal::IwaSourceBundleBase(std::move(path)), dev_mode_(dev_mode) {}
IwaSourceBundleWithMode::~IwaSourceBundleWithMode() = default;
IwaSourceBundleWithMode::IwaSourceBundleWithMode(IwaSourceBundleDevMode other)
@@ -139,6 +155,19 @@
bool IwaSourceBundleWithMode::operator==(
const IwaSourceBundleWithMode& other) const = default;
+[[nodiscard]] IwaSourceBundleWithModeAndFileOp
+IwaSourceBundleWithMode::WithFileOp(
+ IwaSourceBundleProdFileOp prod_file_op,
+ IwaSourceBundleDevFileOp dev_file_op) const {
+ if (dev_mode_) {
+ return IwaSourceBundleWithModeAndFileOp(path_,
+ ToBundleModeAndFileOp(dev_file_op));
+ } else {
+ return IwaSourceBundleWithModeAndFileOp(
+ path_, ToBundleModeAndFileOp(prod_file_op));
+ }
+}
+
base::Value IwaSourceBundleWithMode::ToDebugValue() const {
return base::Value(base::Value::Dict()
.Set("path", base::FilePathToValue(path_))
@@ -151,7 +180,7 @@
}
IwaSourceBundleDevMode::IwaSourceBundleDevMode(base::FilePath path)
- : IwaSourceBundle(std::move(path)) {}
+ : internal::IwaSourceBundleBase(std::move(path)) {}
IwaSourceBundleDevMode::~IwaSourceBundleDevMode() = default;
bool IwaSourceBundleDevMode::operator==(
@@ -174,7 +203,7 @@
}
IwaSourceBundleProdMode::IwaSourceBundleProdMode(base::FilePath path)
- : IwaSourceBundle(std::move(path)) {}
+ : internal::IwaSourceBundleBase(std::move(path)) {}
IwaSourceBundleProdMode::~IwaSourceBundleProdMode() = default;
bool IwaSourceBundleProdMode::operator==(
@@ -199,16 +228,17 @@
IwaSourceBundleWithModeAndFileOp::IwaSourceBundleWithModeAndFileOp(
base::FilePath path,
ModeAndFileOp mode_and_file_op)
- : IwaSourceBundle(std::move(path)), mode_and_file_op_(mode_and_file_op) {}
+ : internal::IwaSourceBundleBase(std::move(path)),
+ mode_and_file_op_(mode_and_file_op) {}
IwaSourceBundleWithModeAndFileOp::~IwaSourceBundleWithModeAndFileOp() = default;
IwaSourceBundleWithModeAndFileOp::IwaSourceBundleWithModeAndFileOp(
IwaSourceBundleDevModeWithFileOp other)
- : IwaSourceBundle(std::move(other.path_)),
+ : internal::IwaSourceBundleBase(std::move(other.path_)),
mode_and_file_op_(ToBundleModeAndFileOp(other.file_op_)) {}
IwaSourceBundleWithModeAndFileOp::IwaSourceBundleWithModeAndFileOp(
IwaSourceBundleProdModeWithFileOp other)
- : IwaSourceBundle(std::move(other.path_)),
+ : internal::IwaSourceBundleBase(std::move(other.path_)),
mode_and_file_op_(ToBundleModeAndFileOp(other.file_op_)) {}
bool IwaSourceBundleWithModeAndFileOp::operator==(
@@ -364,6 +394,21 @@
bool IwaSourceWithMode::operator==(const IwaSourceWithMode& other) const =
default;
+[[nodiscard]] IwaSourceWithModeAndFileOp IwaSourceWithMode::WithFileOp(
+ IwaSourceBundleProdFileOp prod_file_op,
+ IwaSourceBundleDevFileOp dev_file_op) const {
+ return absl::visit(
+ base::Overloaded{
+ [&](const IwaSourceBundleWithMode& source)
+ -> IwaSourceWithModeAndFileOp::Variant {
+ return source.WithFileOp(prod_file_op, dev_file_op);
+ },
+ [&](const IwaSourceProxy& source)
+ -> IwaSourceWithModeAndFileOp::Variant { return source; },
+ },
+ variant_);
+}
+
bool IwaSourceWithMode::dev_mode() const {
return absl::visit(
base::Overloaded{[](const auto& source) { return source.dev_mode(); }},
diff --git a/chrome/browser/web_applications/isolated_web_apps/isolated_web_app_source.h b/chrome/browser/web_applications/isolated_web_apps/isolated_web_app_source.h
index 59874b58..26174d2 100644
--- a/chrome/browser/web_applications/isolated_web_apps/isolated_web_app_source.h
+++ b/chrome/browser/web_applications/isolated_web_apps/isolated_web_app_source.h
@@ -113,14 +113,34 @@
inline constexpr IwaSourceBundleDevFileOp kDefaultBundleDevFileOp =
IwaSourceBundleDevFileOp::kReference;
-class IwaSourceBundle {
+namespace internal {
+
+class IwaSourceBundleBase {
+ public:
+ explicit IwaSourceBundleBase(base::FilePath);
+ ~IwaSourceBundleBase();
+
+ bool operator==(const IwaSourceBundleBase&) const;
+
+ const base::FilePath& path() const { return path_; }
+
+ protected:
+ base::FilePath path_;
+};
+
+} // namespace internal
+
+class IwaSourceBundle : public internal::IwaSourceBundleBase {
public:
explicit IwaSourceBundle(base::FilePath);
~IwaSourceBundle();
- bool operator==(const IwaSourceBundle&) const;
+ // NOLINTNEXTLINE(google-explicit-constructor)
+ /* implicit */ IwaSourceBundle(IwaSourceBundleWithMode other);
+ // NOLINTNEXTLINE(google-explicit-constructor)
+ /* implicit */ IwaSourceBundle(IwaSourceBundleWithModeAndFileOp other);
- const base::FilePath& path() const { return path_; }
+ bool operator==(const IwaSourceBundle&) const;
[[nodiscard]] IwaSourceBundleWithModeAndFileOp WithModeAndFileOp(
IwaSourceBundleModeAndFileOp mode_and_file_op) const;
@@ -130,14 +150,13 @@
IwaSourceBundleProdFileOp file_op) const;
base::Value ToDebugValue() const;
-
- protected:
- base::FilePath path_;
};
std::ostream& operator<<(std::ostream& os, const IwaSourceBundle& source);
-class IwaSourceBundleWithMode : public IwaSourceBundle {
+class IwaSourceBundleWithMode : public internal::IwaSourceBundleBase {
public:
+ friend class IwaSourceBundle;
+
IwaSourceBundleWithMode(base::FilePath path, bool dev_mode);
~IwaSourceBundleWithMode();
@@ -151,6 +170,12 @@
bool operator==(const IwaSourceBundleWithMode&) const;
+ // Depending on whether `this` is a dev mode or prod mode source, returns a
+ // source with either the `prod_file_op` or `dev_file_op` file operation.
+ [[nodiscard]] IwaSourceBundleWithModeAndFileOp WithFileOp(
+ IwaSourceBundleProdFileOp prod_file_op,
+ IwaSourceBundleDevFileOp dev_file_op) const;
+
bool dev_mode() const { return dev_mode_; }
base::Value ToDebugValue() const;
@@ -161,7 +186,7 @@
std::ostream& operator<<(std::ostream& os,
const IwaSourceBundleWithMode& source);
-class IwaSourceBundleDevMode : public IwaSourceBundle {
+class IwaSourceBundleDevMode : public internal::IwaSourceBundleBase {
public:
friend class IwaSourceBundleWithMode;
@@ -178,7 +203,7 @@
std::ostream& operator<<(std::ostream& os,
const IwaSourceBundleDevMode& source);
-class IwaSourceBundleProdMode : public IwaSourceBundle {
+class IwaSourceBundleProdMode : public internal::IwaSourceBundleBase {
public:
friend class IwaSourceBundleWithMode;
@@ -195,8 +220,9 @@
std::ostream& operator<<(std::ostream& os,
const IwaSourceBundleProdMode& source);
-class IwaSourceBundleWithModeAndFileOp : public IwaSourceBundle {
+class IwaSourceBundleWithModeAndFileOp : public internal::IwaSourceBundleBase {
public:
+ friend class IwaSourceBundle;
friend class IwaSourceBundleWithMode;
using ModeAndFileOp = IwaSourceBundleModeAndFileOp;
@@ -328,6 +354,12 @@
bool operator==(const IwaSourceWithMode&) const;
+ // Depending on whether `this` is a dev mode or prod mode source, returns a
+ // source with either the `prod_file_op` or `dev_file_op` file operation.
+ [[nodiscard]] IwaSourceWithModeAndFileOp WithFileOp(
+ IwaSourceBundleProdFileOp prod_file_op,
+ IwaSourceBundleDevFileOp dev_file_op) const;
+
bool dev_mode() const;
const Variant& variant() const { return variant_; }
diff --git a/chrome/browser/web_applications/isolated_web_apps/isolated_web_app_source_unittest.cc b/chrome/browser/web_applications/isolated_web_apps/isolated_web_app_source_unittest.cc
index 07371195..d4421694 100644
--- a/chrome/browser/web_applications/isolated_web_apps/isolated_web_app_source_unittest.cc
+++ b/chrome/browser/web_applications/isolated_web_apps/isolated_web_app_source_unittest.cc
@@ -128,6 +128,25 @@
}
}
+TEST_F(IwaSourceBundleWithModeTest, WithFileOp) {
+ {
+ IwaSourceBundleWithMode bundle{kExamplePath, /*dev_mode=*/false};
+ EXPECT_THAT(
+ bundle.WithFileOp(IwaSourceBundleProdFileOp::kCopy,
+ IwaSourceBundleDevFileOp::kReference),
+ Eq(IwaSourceBundleWithModeAndFileOp{
+ kExamplePath, IwaSourceBundleModeAndFileOp::kProdModeCopy}));
+ }
+ {
+ IwaSourceBundleWithMode bundle{kExamplePath, /*dev_mode=*/true};
+ EXPECT_THAT(
+ bundle.WithFileOp(IwaSourceBundleProdFileOp::kCopy,
+ IwaSourceBundleDevFileOp::kReference),
+ Eq(IwaSourceBundleWithModeAndFileOp{
+ kExamplePath, IwaSourceBundleModeAndFileOp::kDevModeReference}));
+ }
+}
+
using IwaSourceBundleDevModeTest = IwaSourceTestBase;
TEST_F(IwaSourceBundleDevModeTest, Works) {
@@ -355,6 +374,27 @@
}
}
+TEST_F(IwaSourceWithModeTest, WithFileOp) {
+ {
+ IwaSourceWithMode source{
+ IwaSourceBundleWithMode(kExamplePath, /*dev_mode=*/false)};
+ EXPECT_THAT(
+ source.WithFileOp(IwaSourceBundleProdFileOp::kCopy,
+ IwaSourceBundleDevFileOp::kReference),
+ Eq(IwaSourceBundleWithModeAndFileOp{
+ kExamplePath, IwaSourceBundleModeAndFileOp::kProdModeCopy}));
+ }
+ {
+ IwaSourceWithMode source{
+ IwaSourceBundleWithMode(kExamplePath, /*dev_mode=*/true)};
+ EXPECT_THAT(
+ source.WithFileOp(IwaSourceBundleProdFileOp::kCopy,
+ IwaSourceBundleDevFileOp::kReference),
+ Eq(IwaSourceBundleWithModeAndFileOp{
+ kExamplePath, IwaSourceBundleModeAndFileOp::kDevModeReference}));
+ }
+}
+
using IwaSourceDevModeTest = IwaSourceTestBase;
TEST_F(IwaSourceDevModeTest, Works) {
