Blacklist handling of the netflix plugin.
The netflix plugin is triggering a bug in bsdiff, so adding a blacklist
of bsdiff testing for this file until bsdiff is fixed.
BUG=chromium-os:28552
TEST=Built/ran new unittests. Did NOT diff a ChromeOS image (yet).
Change-Id: Ib2ba71ab815105fb4db88e0b24335d34693b96f7
Reviewed-on: https://gerrit.chromium.org/gerrit/19205
Reviewed-by: Jay Srinivasan <jaysri@chromium.org>
Commit-Ready: Don Garrett <dgarrett@chromium.org>
Tested-by: Don Garrett <dgarrett@chromium.org>
diff --git a/delta_diff_generator.cc b/delta_diff_generator.cc
index 67b917f..f5f8bec 100644
--- a/delta_diff_generator.cc
+++ b/delta_diff_generator.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium OS Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium OS Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
@@ -102,8 +102,19 @@
string old_path = (old_root == kNonexistentPath) ? kNonexistentPath :
old_root + path;
+ // TODO(dgarrett): chromium-os:15274 Wire up this file all of the way to
+ // command line.
+ static const char* black_files[] = {
+ "/opt/google/chrome/pepper/libnetflixidd.so"
+ };
+
+ std::set<string> bsdiff_blacklist = std::set<string>(black_files,
+ black_files +
+ arraysize(black_files));
+
TEST_AND_RETURN_FALSE(DeltaDiffGenerator::ReadFileToDiff(old_path,
new_root + path,
+ bsdiff_blacklist,
&data,
&operation,
true));
@@ -400,6 +411,7 @@
vector<char> data;
TEST_AND_RETURN_FALSE(DeltaDiffGenerator::ReadFileToDiff(old_kernel_part,
new_kernel_part,
+ std::set<string>(),
&data,
op,
false));
@@ -482,6 +494,7 @@
bool DeltaDiffGenerator::ReadFileToDiff(
const string& old_filename,
const string& new_filename,
+ const std::set<string>& bsdiff_blacklist,
vector<char>* out_data,
DeltaArchiveManifest_InstallOperation* out_op,
bool gather_extents) {
@@ -511,13 +524,14 @@
// Do we have an original file to consider?
struct stat old_stbuf;
- bool no_original = old_filename.empty();
- if (!no_original && 0 != stat(old_filename.c_str(), &old_stbuf)) {
+ bool original = !old_filename.empty();
+ if (original && 0 != stat(old_filename.c_str(), &old_stbuf)) {
// If stat-ing the old file fails, it should be because it doesn't exist.
TEST_AND_RETURN_FALSE(errno == ENOTDIR || errno == ENOENT);
- no_original = true;
+ original = false;
}
- if (!no_original) {
+
+ if (original) {
// Read old data
vector<char> old_data;
TEST_AND_RETURN_FALSE(utils::ReadFile(old_filename, &old_data));
@@ -527,16 +541,20 @@
current_best_size = 0;
data.clear();
} else {
- // Try bsdiff of old to new data
- vector<char> bsdiff_delta;
- TEST_AND_RETURN_FALSE(
- BsdiffFiles(old_filename, new_filename, &bsdiff_delta));
- CHECK_GT(bsdiff_delta.size(), static_cast<vector<char>::size_type>(0));
- if (bsdiff_delta.size() < current_best_size) {
- operation.set_type(DeltaArchiveManifest_InstallOperation_Type_BSDIFF);
- current_best_size = bsdiff_delta.size();
+ if (bsdiff_blacklist.find(old_filename) ==
+ bsdiff_blacklist.end()) {
+ // If the source file hasn't been bsdiff blacklisted, then try to see
+ // if bsdiff can find a smaller operation.
+ vector<char> bsdiff_delta;
+ TEST_AND_RETURN_FALSE(
+ BsdiffFiles(old_filename, new_filename, &bsdiff_delta));
+ CHECK_GT(bsdiff_delta.size(), static_cast<vector<char>::size_type>(0));
+ if (bsdiff_delta.size() < current_best_size) {
+ operation.set_type(DeltaArchiveManifest_InstallOperation_Type_BSDIFF);
+ current_best_size = bsdiff_delta.size();
- data = bsdiff_delta;
+ data = bsdiff_delta;
+ }
}
}
}
diff --git a/delta_diff_generator.h b/delta_diff_generator.h
index 79683e5..3997df2 100644
--- a/delta_diff_generator.h
+++ b/delta_diff_generator.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium OS Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
@@ -99,6 +99,7 @@
// Returns true on success.
static bool ReadFileToDiff(const std::string& old_filename,
const std::string& new_filename,
+ const std::set<std::string>& bsdiff_blacklist,
std::vector<char>* out_data,
DeltaArchiveManifest_InstallOperation* out_op,
bool gather_extents);
diff --git a/delta_diff_generator_unittest.cc b/delta_diff_generator_unittest.cc
index bd0b48e..632b8e7 100644
--- a/delta_diff_generator_unittest.cc
+++ b/delta_diff_generator_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium OS Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium OS Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
@@ -70,6 +70,7 @@
DeltaArchiveManifest_InstallOperation op;
EXPECT_TRUE(DeltaDiffGenerator::ReadFileToDiff(old_path(),
new_path(),
+ std::set<string>(),
&data,
&op,
true));
@@ -99,6 +100,7 @@
DeltaArchiveManifest_InstallOperation op;
EXPECT_TRUE(DeltaDiffGenerator::ReadFileToDiff(old_path(),
new_path(),
+ std::set<string>(),
&data,
&op,
true));
@@ -117,6 +119,97 @@
EXPECT_EQ(1, BlocksInExtents(op.dst_extents()));
}
+TEST_F(DeltaDiffGeneratorTest, RunAsRootBsdiffBlacklistNoMatchTest) {
+ EXPECT_TRUE(utils::WriteFile(old_path().c_str(),
+ reinterpret_cast<const char*>(kRandomString),
+ sizeof(kRandomString) - 1));
+ EXPECT_TRUE(utils::WriteFile(new_path().c_str(),
+ reinterpret_cast<const char*>(kRandomString),
+ sizeof(kRandomString)));
+ vector<char> data;
+ DeltaArchiveManifest_InstallOperation op;
+
+ std::set<string> bsdiff_blacklist;
+ bsdiff_blacklist.insert("fuzzy_bunny"); // Not the new_path
+
+ EXPECT_TRUE(DeltaDiffGenerator::ReadFileToDiff(old_path(),
+ new_path(),
+ bsdiff_blacklist,
+ &data,
+ &op,
+ true));
+ EXPECT_FALSE(data.empty());
+
+ EXPECT_TRUE(op.has_type());
+ EXPECT_EQ(DeltaArchiveManifest_InstallOperation_Type_BSDIFF, op.type());
+ EXPECT_FALSE(op.has_data_offset());
+ EXPECT_FALSE(op.has_data_length());
+ EXPECT_EQ(1, op.src_extents_size());
+ EXPECT_EQ(sizeof(kRandomString) - 1, op.src_length());
+ EXPECT_EQ(1, op.dst_extents_size());
+ EXPECT_EQ(sizeof(kRandomString), op.dst_length());
+ EXPECT_EQ(BlocksInExtents(op.src_extents()),
+ BlocksInExtents(op.dst_extents()));
+ EXPECT_EQ(1, BlocksInExtents(op.dst_extents()));
+}
+
+
+TEST_F(DeltaDiffGeneratorTest, RunAsRootBsdiffBlacklistMatchTest) {
+ EXPECT_TRUE(utils::WriteFile(old_path().c_str(),
+ reinterpret_cast<const char*>(kRandomString),
+ sizeof(kRandomString) - 1));
+ EXPECT_TRUE(utils::WriteFile(new_path().c_str(),
+ reinterpret_cast<const char*>(kRandomString),
+ sizeof(kRandomString)));
+ vector<char> data;
+ DeltaArchiveManifest_InstallOperation op;
+
+ std::set<string> bsdiff_blacklist;
+ bsdiff_blacklist.insert("fuzzy_bunny");
+ bsdiff_blacklist.insert(old_path());
+
+ EXPECT_TRUE(DeltaDiffGenerator::ReadFileToDiff(old_path(),
+ new_path(),
+ bsdiff_blacklist,
+ &data,
+ &op,
+ true));
+ EXPECT_FALSE(data.empty());
+
+ // The point of this test is that we don't use BSDIFF the way the above
+ // did. The rest of the details are to be caught in other tests.
+ EXPECT_TRUE(op.has_type());
+ EXPECT_NE(DeltaArchiveManifest_InstallOperation_Type_BSDIFF, op.type());
+}
+
+TEST_F(DeltaDiffGeneratorTest, RunAsRootBsdiffBlacklistMoveMatchTest) {
+ EXPECT_TRUE(utils::WriteFile(old_path().c_str(),
+ reinterpret_cast<const char*>(kRandomString),
+ sizeof(kRandomString)));
+ EXPECT_TRUE(utils::WriteFile(new_path().c_str(),
+ reinterpret_cast<const char*>(kRandomString),
+ sizeof(kRandomString)));
+ vector<char> data;
+ DeltaArchiveManifest_InstallOperation op;
+
+ std::set<string> bsdiff_blacklist;
+ bsdiff_blacklist.insert("fuzzy_bunny");
+ bsdiff_blacklist.insert(old_path());
+
+ EXPECT_TRUE(DeltaDiffGenerator::ReadFileToDiff(old_path(),
+ new_path(),
+ bsdiff_blacklist,
+ &data,
+ &op,
+ true));
+ EXPECT_TRUE(data.empty());
+
+ // The point of this test is that we can still use a MOVE for a file
+ // that is blacklisted.
+ EXPECT_TRUE(op.has_type());
+ EXPECT_EQ(DeltaArchiveManifest_InstallOperation_Type_MOVE, op.type());
+}
+
TEST_F(DeltaDiffGeneratorTest, RunAsRootReplaceSmallTest) {
vector<char> new_data;
for (int i = 0; i < 2; i++) {
@@ -130,6 +223,7 @@
DeltaArchiveManifest_InstallOperation op;
EXPECT_TRUE(DeltaDiffGenerator::ReadFileToDiff(old_path(),
new_path(),
+ std::set<string>(),
&data,
&op,
true));
@@ -161,6 +255,7 @@
DeltaArchiveManifest_InstallOperation op;
EXPECT_TRUE(DeltaDiffGenerator::ReadFileToDiff(old_path(),
new_path(),
+ std::set<string>(),
&data,
&op,
false));