Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(680)

Unified Diff: chrome/browser/extensions/api/image_writer_private/operation.cc

Issue 170123002: Revert of Significantly cleans up the ImageWriter Operation class and subclasses. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 6 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/extensions/api/image_writer_private/operation.cc
diff --git a/chrome/browser/extensions/api/image_writer_private/operation.cc b/chrome/browser/extensions/api/image_writer_private/operation.cc
index e58ca43ead964cb863311c2d56342397a19a26bb..a37c81c4e6755c4b553f9f820b04bd53e9d103da 100644
--- a/chrome/browser/extensions/api/image_writer_private/operation.cc
+++ b/chrome/browser/extensions/api/image_writer_private/operation.cc
@@ -15,36 +15,34 @@
using content::BrowserThread;
+namespace {
+
const int kMD5BufferSize = 1024;
-#if defined(OS_CHROMEOS)
-// Chrome OS only has a 1 GB temporary partition. This is too small to hold our
-// unzipped image. Fortunately we mount part of the temporary partition under
-// /var/tmp.
-const char kChromeOSTempRoot[] = "/var/tmp";
-#endif
+
+void RemoveTempDirectory(const base::FilePath path) {
+ base::DeleteFile(path, true);
+}
+
+} // namespace
Operation::Operation(base::WeakPtr<OperationManager> manager,
const ExtensionId& extension_id,
- const std::string& device_path)
+ const std::string& storage_unit_id)
: manager_(manager),
extension_id_(extension_id),
-#if defined(OS_WIN)
- device_path_(base::FilePath::FromUTF8Unsafe(device_path)),
-#else
- device_path_(device_path),
-#endif
-#if defined(OS_LINUX) && !defined(CHROMEOS)
- image_file_(base::kInvalidPlatformFileValue),
- device_file_(base::kInvalidPlatformFileValue),
-#endif
+ storage_unit_id_(storage_unit_id),
+ verify_write_(true),
stage_(image_writer_api::STAGE_UNKNOWN),
progress_(0) {
}
-Operation::~Operation() {}
+Operation::~Operation() {
+}
void Operation::Cancel() {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+
+ DVLOG(1) << "Cancelling image writing operation for ext: " << extension_id_;
stage_ = image_writer_api::STAGE_NONE;
@@ -61,82 +59,6 @@
image_writer_api::Stage Operation::GetStage() {
return stage_;
-}
-
-void Operation::Start() {
-#if defined(OS_CHROMEOS)
- if (!temp_dir_.CreateUniqueTempDirUnderPath(
- base::FilePath(kChromeOSTempRoot))) {
-#else
- if (!temp_dir_.CreateUniqueTempDir()) {
-#endif
- Error(error::kTempDirError);
- return;
- }
-
- AddCleanUpFunction(
- base::Bind(base::IgnoreResult(&base::ScopedTempDir::Delete),
- base::Unretained(&temp_dir_)));
-
- StartImpl();
-}
-
-void Operation::Unzip(const base::Closure& continuation) {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
- if (IsCancelled()) {
- return;
- }
-
- if (image_path_.Extension() != FILE_PATH_LITERAL(".zip")) {
- BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE, continuation);
- return;
- }
-
- SetStage(image_writer_api::STAGE_UNZIP);
-
- if (!(zip_reader_.Open(image_path_) && zip_reader_.AdvanceToNextEntry() &&
- zip_reader_.OpenCurrentEntryInZip())) {
- Error(error::kUnzipGenericError);
- return;
- }
-
- if (zip_reader_.HasMore()) {
- Error(error::kUnzipInvalidArchive);
- return;
- }
-
- // Create a new target to unzip to. The original file is opened by the
- // zip_reader_.
- zip::ZipReader::EntryInfo* entry_info = zip_reader_.current_entry_info();
- if (entry_info) {
- image_path_ = temp_dir_.path().Append(entry_info->file_path().BaseName());
- } else {
- Error(error::kTempDirError);
- return;
- }
-
- zip_reader_.ExtractCurrentEntryToFilePathAsync(
- image_path_,
- base::Bind(&Operation::OnUnzipSuccess, this, continuation),
- base::Bind(&Operation::OnUnzipFailure, this),
- base::Bind(&Operation::OnUnzipProgress,
- this,
- zip_reader_.current_entry_info()->original_size()));
-}
-
-void Operation::Finish() {
- if (!BrowserThread::CurrentlyOn(BrowserThread::FILE)) {
- BrowserThread::PostTask(
- BrowserThread::FILE, FROM_HERE, base::Bind(&Operation::Finish, this));
- return;
- }
-
- CleanUp();
-
- BrowserThread::PostTask(
- BrowserThread::UI,
- FROM_HERE,
- base::Bind(&OperationManager::OnComplete, manager_, extension_id_));
}
void Operation::Error(const std::string& error_message) {
@@ -171,10 +93,6 @@
return;
}
- if (progress <= progress_) {
- return;
- }
-
if (IsCancelled()) {
return;
}
@@ -219,51 +137,124 @@
progress_));
}
+void Operation::Finish() {
+ if (!BrowserThread::CurrentlyOn(BrowserThread::FILE)) {
+ BrowserThread::PostTask(BrowserThread::FILE,
+ FROM_HERE,
+ base::Bind(&Operation::Finish, this));
+ return;
+ }
+ DVLOG(1) << "Write operation complete.";
+
+ CleanUp();
+
+ BrowserThread::PostTask(
+ BrowserThread::UI,
+ FROM_HERE,
+ base::Bind(&OperationManager::OnComplete,
+ manager_,
+ extension_id_));
+}
+
bool Operation::IsCancelled() {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
return stage_ == image_writer_api::STAGE_NONE;
}
-void Operation::AddCleanUpFunction(const base::Closure& callback) {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+void Operation::AddCleanUpFunction(base::Closure callback) {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+
cleanup_functions_.push_back(callback);
}
+void Operation::CleanUp() {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+ for (std::vector<base::Closure>::iterator it = cleanup_functions_.begin();
+ it != cleanup_functions_.end();
+ ++it) {
+ it->Run();
+ }
+ cleanup_functions_.clear();
+}
+
+void Operation::UnzipStart(scoped_ptr<base::FilePath> zip_path) {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+ if (IsCancelled()) {
+ return;
+ }
+
+ DVLOG(1) << "Starting unzip stage for " << zip_path->value();
+
+ SetStage(image_writer_api::STAGE_UNZIP);
+
+ base::FilePath tmp_dir;
+ if (!base::CreateTemporaryDirInDir(zip_path->DirName(),
+ FILE_PATH_LITERAL("image_writer"),
+ &tmp_dir)) {
+ Error(error::kTempDirError);
+ return;
+ }
+
+ AddCleanUpFunction(base::Bind(&RemoveTempDirectory, tmp_dir));
+
+ if (!base::CreateTemporaryFileInDir(tmp_dir, &image_path_)) {
+ DLOG(ERROR) << "Failed create temporary unzip target in "
+ << tmp_dir.value();
+ Error(error::kTempDirError);
+ return;
+ }
+
+ if (!(zip_reader_.Open(*zip_path) &&
+ zip_reader_.AdvanceToNextEntry() &&
+ zip_reader_.OpenCurrentEntryInZip())) {
+ DLOG(ERROR) << "Failed to open zip file.";
+ Error(error::kUnzipGenericError);
+ return;
+ }
+
+ if (zip_reader_.HasMore()) {
+ DLOG(ERROR) << "Image zip has more than one file.";
+ Error(error::kUnzipInvalidArchive);
+ return;
+ }
+
+ zip_reader_.ExtractCurrentEntryToFilePathAsync(
+ image_path_,
+ base::Bind(&Operation::OnUnzipSuccess, this),
+ base::Bind(&Operation::OnUnzipFailure, this),
+ base::Bind(&Operation::OnUnzipProgress,
+ this,
+ zip_reader_.current_entry_info()->original_size()));
+}
+
void Operation::GetMD5SumOfFile(
- const base::FilePath& file_path,
+ scoped_ptr<base::FilePath> file_path,
int64 file_size,
int progress_offset,
int progress_scale,
- const base::Callback<void(const std::string&)>& callback) {
+ const base::Callback<void(scoped_ptr<std::string>)>& callback) {
if (IsCancelled()) {
return;
}
base::MD5Init(&md5_context_);
-
- base::PlatformFile file = base::CreatePlatformFile(
- file_path,
- base::PLATFORM_FILE_OPEN | base::PLATFORM_FILE_READ,
- NULL,
- NULL);
- if (file == base::kInvalidPlatformFileValue) {
+ scoped_ptr<image_writer_utils::ImageReader> reader(
+ new image_writer_utils::ImageReader());
+
+ if (!reader->Open(*file_path)) {
Error(error::kImageOpenError);
return;
}
-
if (file_size <= 0) {
- if (!base::GetFileSize(file_path, &file_size)) {
- Error(error::kImageOpenError);
- return;
- }
+ file_size = reader->GetSize();
}
BrowserThread::PostTask(BrowserThread::FILE,
FROM_HERE,
base::Bind(&Operation::MD5Chunk,
this,
- file,
+ base::Passed(&reader),
0,
file_size,
progress_offset,
@@ -272,64 +263,68 @@
}
void Operation::MD5Chunk(
- const base::PlatformFile& file,
+ scoped_ptr<image_writer_utils::ImageReader> reader,
int64 bytes_processed,
int64 bytes_total,
int progress_offset,
int progress_scale,
- const base::Callback<void(const std::string&)>& callback) {
- if (IsCancelled()) {
- base::ClosePlatformFile(file);
- return;
- }
-
- CHECK_LE(bytes_processed, bytes_total);
-
- scoped_ptr<char[]> buffer(new char[kMD5BufferSize]);
- int read_size = std::min(bytes_total - bytes_processed,
- static_cast<int64>(kMD5BufferSize));
-
- if (read_size == 0) {
- // Nothing to read, we are done.
- base::MD5Digest digest;
- base::MD5Final(&digest, &md5_context_);
- callback.Run(base::MD5DigestToBase16(digest));
+ const base::Callback<void(scoped_ptr<std::string>)>& callback) {
+ if (IsCancelled()) {
+ reader->Close();
+ return;
+ }
+
+ char buffer[kMD5BufferSize];
+ int len;
+
+ if (bytes_total - bytes_processed <= kMD5BufferSize) {
+ len = reader->Read(buffer, bytes_total - bytes_processed);
} else {
- int len =
- base::ReadPlatformFile(file, bytes_processed, buffer.get(), read_size);
-
- if (len == read_size) {
- // Process data.
- base::MD5Update(&md5_context_, base::StringPiece(buffer.get(), len));
- int percent_curr =
- ((bytes_processed + len) * progress_scale) / bytes_total +
- progress_offset;
- SetProgress(percent_curr);
-
- BrowserThread::PostTask(BrowserThread::FILE,
- FROM_HERE,
- base::Bind(&Operation::MD5Chunk,
- this,
- file,
- bytes_processed + len,
- bytes_total,
- progress_offset,
- progress_scale,
- callback));
- // Skip closing the file.
- return;
+ len = reader->Read(buffer, kMD5BufferSize);
+ }
+
+ if (len > 0) {
+ base::MD5Update(&md5_context_, base::StringPiece(buffer, len));
+ int percent_prev = (bytes_processed * progress_scale + progress_offset) /
+ (bytes_total);
+ int percent_curr = ((bytes_processed + len) * progress_scale +
+ progress_offset) /
+ (bytes_total);
+ if (percent_curr > percent_prev) {
+ SetProgress(progress_);
+ }
+
+ BrowserThread::PostTask(BrowserThread::FILE,
+ FROM_HERE,
+ base::Bind(&Operation::MD5Chunk,
+ this,
+ base::Passed(&reader),
+ bytes_processed + len,
+ bytes_total,
+ progress_offset,
+ progress_scale,
+ callback));
+ } else if (len == 0) {
+ if (bytes_processed + len < bytes_total) {
+ reader->Close();
+ Error(error::kHashReadError);
} else {
- // We didn't read the bytes we expected.
- Error(error::kHashReadError);
+ base::MD5Digest digest;
+ base::MD5Final(&digest, &md5_context_);
+ scoped_ptr<std::string> hash(
+ new std::string(base::MD5DigestToBase16(digest)));
+ callback.Run(hash.Pass());
}
- }
- base::ClosePlatformFile(file);
-}
-
-void Operation::OnUnzipSuccess(const base::Closure& continuation) {
+ } else { // len < 0
+ reader->Close();
+ Error(error::kHashReadError);
+ }
+}
+
+void Operation::OnUnzipSuccess() {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
SetProgress(kProgressComplete);
- BrowserThread::PostTask(BrowserThread::FILE, FROM_HERE, continuation);
+ WriteStart();
}
void Operation::OnUnzipFailure() {
@@ -344,15 +339,5 @@
SetProgress(progress_percent);
}
-void Operation::CleanUp() {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
- for (std::vector<base::Closure>::iterator it = cleanup_functions_.begin();
- it != cleanup_functions_.end();
- ++it) {
- it->Run();
- }
- cleanup_functions_.clear();
-}
-
} // namespace image_writer
} // namespace extensions

Powered by Google App Engine
This is Rietveld 408576698