Skip to content

Commit

Permalink
[Extensions] Add ExtensionBuilder::SetVersion()
Browse files Browse the repository at this point in the history
ExtensionBuilder has SetManifestKey(), but "version" is pretty common
(and fairly core to the extension object), so add a convenience method
for setting it.

Add a quick unittest for the same, and update a smattering of places
that previously used SetManifestKey().

Bug: 832958

Change-Id: I55de79360e698f8f9983c24a0e0d5fdb8b3fff9b
Reviewed-on: https://chromium-review.googlesource.com/1242196
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#594023}
  • Loading branch information
rdcronin authored and Commit Bot committed Sep 25, 2018
1 parent d030b2e commit 5d741ba
Show file tree
Hide file tree
Showing 6 changed files with 26 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
#include "extensions/browser/extension_prefs.h"
#include "extensions/browser/install_flag.h"
#include "extensions/common/extension_builder.h"
#include "extensions/common/manifest_constants.h"
#include "extensions/common/value_builder.h"
#include "ppapi/buildflags/buildflags.h"

Expand Down Expand Up @@ -121,7 +120,7 @@ class ExtensionGarbageCollectorChromeOSUnitTest
const std::string& version,
const base::FilePath& path) {
return ExtensionBuilder("test")
.SetManifestKey(manifest_keys::kVersion, version)
.SetVersion(version)
.SetID(id)
.SetPath(path)
.SetLocation(Manifest::INTERNAL)
Expand Down
8 changes: 4 additions & 4 deletions extensions/browser/updater/update_service_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ class UpdateServiceTest : public ExtensionsTest {

scoped_refptr<Extension> extension1 =
ExtensionBuilder("Foo")
.SetManifestKey("version", "1.0")
.SetVersion("1.0")
.SetID(crx_file::id_util::GenerateId("foo_extension"))
.SetPath(temp_dir.GetPath())
.Build();
Expand Down Expand Up @@ -409,11 +409,11 @@ TEST_F(UpdateServiceTest, UninstallPings) {

// Build 3 extensions.
scoped_refptr<Extension> extension1 =
ExtensionBuilder("1").SetManifestKey("version", "1.2").Build();
ExtensionBuilder("1").SetVersion("1.2").Build();
scoped_refptr<Extension> extension2 =
ExtensionBuilder("2").SetManifestKey("version", "2.3").Build();
ExtensionBuilder("2").SetVersion("2.3").Build();
scoped_refptr<Extension> extension3 =
ExtensionBuilder("3").SetManifestKey("version", "3.4").Build();
ExtensionBuilder("3").SetVersion("3.4").Build();
EXPECT_TRUE(extension1->id() != extension2->id() &&
extension1->id() != extension3->id() &&
extension2->id() != extension3->id());
Expand Down
9 changes: 8 additions & 1 deletion extensions/common/extension_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ struct ExtensionBuilder::ManifestData {
std::vector<std::string> permissions;
base::Optional<ActionType> action;
base::Optional<BackgroundPage> background_page;
base::Optional<std::string> version;

// A ContentScriptEntry includes a string name, and a vector of string
// match patterns.
Expand All @@ -31,7 +32,7 @@ struct ExtensionBuilder::ManifestData {
DictionaryBuilder manifest;
manifest.Set("name", name)
.Set("manifest_version", 2)
.Set("version", "0.1")
.Set("version", version.value_or("0.1"))
.Set("description", "some description");

switch (type) {
Expand Down Expand Up @@ -185,6 +186,12 @@ ExtensionBuilder& ExtensionBuilder::AddContentScript(
return *this;
}

ExtensionBuilder& ExtensionBuilder::SetVersion(const std::string& version) {
CHECK(manifest_data_);
manifest_data_->version = version;
return *this;
}

ExtensionBuilder& ExtensionBuilder::SetPath(const base::FilePath& path) {
path_ = path;
return *this;
Expand Down
5 changes: 5 additions & 0 deletions extensions/common/extension_builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,11 @@ class ExtensionBuilder {
const std::string& script_name,
const std::vector<std::string>& match_patterns);

// Shortcut for setting a specific manifest version. Typically we'd use
// SetManifestKey() or SetManifestPath() for these, but provide a faster
// route for version, since it's so central.
ExtensionBuilder& SetVersion(const std::string& version);

// Shortcuts to setting values on the manifest dictionary without needing to
// go all the way through MergeManifest(). Sample usage:
// ExtensionBuilder("name").SetManifestKey("version", "0.2").Build();
Expand Down
7 changes: 7 additions & 0 deletions extensions/common/extension_builder_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -224,4 +224,11 @@ TEST(ExtensionBuilderTest, AddContentScript) {
}
}

TEST(ExtensionBuilderTest, SetVersion) {
constexpr char kVersion[] = "42.0.99.1";
scoped_refptr<const Extension> extension =
ExtensionBuilder("foo").SetVersion(kVersion).Build();
EXPECT_EQ(kVersion, extension->VersionString());
}

} // namespace extensions
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,7 @@ class ShellSystemLogsFetcherTest : public ExtensionsTest {
scoped_refptr<Extension> BuildExtension(const std::string& name,
const std::string& version,
const std::string& id) {
return ExtensionBuilder(name)
.SetManifestKey("version", version)
.SetID(id)
.Build();
return ExtensionBuilder(name).SetVersion(version).SetID(id).Build();
}

void OnSystemLogsResponse(
Expand Down

0 comments on commit 5d741ba

Please sign in to comment.