mojo filesystem: Simplify full file reading/writing.

components/filesystem/public/cpp/prefs/filesystem_json_pref_store.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/filesystem/public/cpp/prefs/filesystem_json_pref_store.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 #include <utility>
@@ skipping 327 matching lines @@
   if (err != FileError::OK) {
     // Do real error checking.
     NOTIMPLEMENTED();
     return;
   }
 
   callback.Run();
 }
 
 void FilesystemJsonPrefStore::OnTempFileWriteStart() {
-  // Open up a temporary file and truncate it.
-  directory_->OpenFile(
-      "tmp", GetProxy(&temporary_file_), kFlagWrite | kFlagCreate,
-      Bind(&FilesystemJsonPrefStore::OnTempFileOpened, AsWeakPtr()));
-}
-
-void FilesystemJsonPrefStore::OnTempFileOpened(FileError err) {
-  // TODO(erg): Error handling. The JsonPrefStore code assumes that writing the
-  // file can never fail.
-  CHECK_EQ(FileError::OK, err);
-
   // Calculate what we want to write, and then write to the temporary file.
   pending_lossy_write_ = false;
 
   if (pref_filter_)
     pref_filter_->FilterSerializeData(prefs_.get());
 
   std::string output;
   JSONStringValueSerializer serializer(&output);
   serializer.set_pretty_print(false);
   serializer.Serialize(*prefs_);
 
-  temporary_file_->Write(
-      mojo::Array<uint8_t>::From(output), 0, Whence::FROM_CURRENT,
+  directory_->WriteFile(
+      "tmp",
+      mojo::Array<uint8_t>::From(output),
       Bind(&FilesystemJsonPrefStore::OnTempFileWrite, AsWeakPtr()));
 }
 
-void FilesystemJsonPrefStore::OnTempFileWrite(FileError err,
-                                              uint32_t num_bytes_written) {
+void FilesystemJsonPrefStore::OnTempFileWrite(FileError err) {
   // TODO(erg): Error handling. The JsonPrefStore code assumes that writing the
   // file can never fail.
   CHECK_EQ(FileError::OK, err);
 
-  // Now that we've written the file, close it.
-  temporary_file_->Close(
-      Bind(&FilesystemJsonPrefStore::OnTempFileClosed, AsWeakPtr()));
-}
-
-void FilesystemJsonPrefStore::OnTempFileClosed(FileError err) {
-  // TODO(erg): Error handling. The JsonPrefStore code assumes that writing the
-  // file can never fail.
-  CHECK_EQ(FileError::OK, err);
-
-  temporary_file_.reset();
   directory_->Rename(
       "tmp", path_,
       Bind(&FilesystemJsonPrefStore::OnTempFileRenamed, AsWeakPtr()));
 }
 
-void FilesystemJsonPrefStore::OnTempFileRenamed(FileError err) {}
+void FilesystemJsonPrefStore::OnTempFileRenamed(FileError err) {
+  CHECK_EQ(FileError::OK, err);

[sky, 2016/01/27 01:08:40]

Seems like we shouldn't crash here, but have some sort of error handling.

[Elliot Glaysher, 2016/01/27 01:13:42]

There's no real error checking here because there's no real error checking on
write in the JsonPrefStore. While I could add a call to
error_delegate_->OnError(), this would be a divergence in the behaviour.

(I agree with you that it is bizarre that the PrefService system doesn't seem to
verify that it's successfully written preferences to disk.)

[sky, 2016/01/27 02:48:13]

Error checking is different than crashing if there is an error. Shouldn't this
ignore the error, like the rest of code does?

+}
 
 void FilesystemJsonPrefStore::OnPreferencesReadStart() {
-  // TODO(erg): implement me.
-  directory_->OpenFile(
-      path_, GetProxy(&preferences_file_), kFlagRead | kFlagOpen,
-      Bind(&FilesystemJsonPrefStore::OnPreferencesFileOpened, AsWeakPtr()));
-}
-
-void FilesystemJsonPrefStore::OnPreferencesFileOpened(FileError err) {
-  // TODO(erg): Error handling.
-  if (err == FileError::OK) {
-    preferences_file_->ReadEntireFile(
-        Bind(&FilesystemJsonPrefStore::OnPreferencesFileRead, AsWeakPtr()));
-  } else {
-    OnPreferencesFileRead(err, mojo::Array<uint8_t>());
-  }
+  directory_->ReadEntireFile(
+      path_,
+      Bind(&FilesystemJsonPrefStore::OnPreferencesFileRead, AsWeakPtr()));
 }
 
 void FilesystemJsonPrefStore::OnPreferencesFileRead(
     FileError err,
     mojo::Array<uint8_t> contents) {
   scoped_ptr<FilesystemJsonPrefStore::ReadResult> read_result(
       new FilesystemJsonPrefStore::ReadResult);
   // TODO(erg): Needs even better error handling.
   switch (err) {
+    case FileError::FAILED:
     case FileError::IN_USE:
-    case FileError::ACCESS_DENIED: {
+    case FileError::ACCESS_DENIED:
+    case FileError::NOT_A_FILE: {
       read_only_ = true;
       break;
     }
-    case FileError::FAILED:
     case FileError::NOT_FOUND: {
       // If the file just doesn't exist, maybe this is the first run. Just
       // don't pass a value.
       read_result->error = PREF_READ_ERROR_NO_FILE;
       break;
     }
     default: {
       int error_code;
       std::string error_msg;
       JSONStringValueDeserializer deserializer(base::StringPiece(
           reinterpret_cast<char*>(&contents.front()), contents.size()));
       read_result->value = deserializer.Deserialize(&error_code, &error_msg);
       read_result->error = HandleReadErrors(read_result->value.get());
     }
   }
 
-  preferences_file_.reset();
-
   OnFileRead(std::move(read_result));
 }
 
 }  // namespace filesystem