Convert //mojo/file_utils to use SynchronousInterfacePtr. (Also convert a use of ApplicationImpl::ConnectToServiceDeprecated().) R=vardhan@google.com Review URL: https://codereview.chromium.org/1923573002 .
diff --git a/mojo/file_utils/BUILD.gn b/mojo/file_utils/BUILD.gn index 511d4a2..0f44f6e 100644 --- a/mojo/file_utils/BUILD.gn +++ b/mojo/file_utils/BUILD.gn
@@ -13,7 +13,8 @@ ] public_deps = [ - "//mojo/services/files/interfaces:interfaces", + "//mojo/services/files/interfaces", + "//mojo/services/files/interfaces:interfaces_sync", ] }
diff --git a/mojo/file_utils/file_util.cc b/mojo/file_utils/file_util.cc index 28d7f1d..18fa83b 100644 --- a/mojo/file_utils/file_util.cc +++ b/mojo/file_utils/file_util.cc
@@ -6,20 +6,18 @@ #include <stdint.h> -#include "mojo/services/files/interfaces/directory.mojom.h" -#include "mojo/services/files/interfaces/file.mojom.h" #include "mojo/services/files/interfaces/types.mojom.h" namespace file_utils { -mojo::files::FilePtr CreateTemporaryFileInDir( - mojo::files::DirectoryPtr* directory, +mojo::InterfaceHandle<mojo::files::File> CreateTemporaryFileInDir( + mojo::SynchronousInterfacePtr<mojo::files::Directory>* directory, std::string* path_name) { std::string internal_path_name; const std::string charset("abcdefghijklmnopqrstuvwxyz1234567890"); const unsigned kSuffixLength = 6; const unsigned kMaxNumAttempts = 10; - mojo::files::FilePtr temp_file; + mojo::InterfaceHandle<mojo::files::File> temp_file; mojo::files::Error error = mojo::files::Error::INTERNAL; uint64_t random_value = 0; for (unsigned attempt = 0; attempt < kMaxNumAttempts; attempt++) { @@ -31,13 +29,13 @@ random_value += (microseconds << 16) ^ (microseconds / 1000000); internal_path_name.push_back(charset[random_value % charset.length()]); } - (*directory) - ->OpenFile(internal_path_name, GetProxy(&temp_file), - mojo::files::kOpenFlagWrite | mojo::files::kOpenFlagRead | - mojo::files::kOpenFlagCreate | - mojo::files::kOpenFlagExclusive, - [&error](mojo::files::Error e) { error = e; }); - if (!directory->WaitForIncomingResponse()) + if (!(*directory) + ->OpenFile(internal_path_name, GetProxy(&temp_file), + mojo::files::kOpenFlagWrite | + mojo::files::kOpenFlagRead | + mojo::files::kOpenFlagCreate | + mojo::files::kOpenFlagExclusive, + &error)) return nullptr; // TODO(smklein): React to error code when ErrnoToError can return something // other than Error::UNKNOWN. We only want to retry when "EEXIST" is thrown.
diff --git a/mojo/file_utils/file_util.h b/mojo/file_utils/file_util.h index 579ff0a..122148a 100644 --- a/mojo/file_utils/file_util.h +++ b/mojo/file_utils/file_util.h
@@ -9,7 +9,9 @@ #include <string> -#include "mojo/services/files/interfaces/directory.mojom.h" +#include "mojo/public/cpp/bindings/interface_handle.h" +#include "mojo/public/cpp/bindings/synchronous_interface_ptr.h" +#include "mojo/services/files/interfaces/directory.mojom-sync.h" #include "mojo/services/files/interfaces/file.mojom.h" namespace file_utils { @@ -20,8 +22,8 @@ // when this function is called. // Returns a FilePtr on success, and null on failure. // On failure, |path_name| is unchanged. -mojo::files::FilePtr CreateTemporaryFileInDir( - mojo::files::DirectoryPtr* directory, +mojo::InterfaceHandle<mojo::files::File> CreateTemporaryFileInDir( + mojo::SynchronousInterfacePtr<mojo::files::Directory>* directory, std::string* path_name); } // namespace file_utils
diff --git a/mojo/file_utils/file_util_unittest.cc b/mojo/file_utils/file_util_unittest.cc index 80ca03a..d3f6cfc 100644 --- a/mojo/file_utils/file_util_unittest.cc +++ b/mojo/file_utils/file_util_unittest.cc
@@ -3,15 +3,20 @@ // found in the LICENSE file. #include <stdint.h> +#include <string.h> + +#include <utility> #include <vector> #include "mojo/file_utils/file_util.h" #include "mojo/public/cpp/application/application_impl.h" #include "mojo/public/cpp/application/application_test_base.h" +#include "mojo/public/cpp/application/connect.h" +#include "mojo/public/cpp/bindings/synchronous_interface_ptr.h" #include "mojo/public/cpp/utility/run_loop.h" -#include "mojo/services/files/interfaces/directory.mojom.h" -#include "mojo/services/files/interfaces/file.mojom.h" -#include "mojo/services/files/interfaces/files.mojom.h" +#include "mojo/services/files/interfaces/directory.mojom-sync.h" +#include "mojo/services/files/interfaces/file.mojom-sync.h" +#include "mojo/services/files/interfaces/files.mojom-sync.h" namespace file_utils { namespace test { @@ -19,138 +24,95 @@ using FileUtilTest = mojo::test::ApplicationTestBase; -// TODO(smklein): Stuff copied from mojo/services/files/c/lib/template_util.h -typedef char YesType; - -struct NoType { - YesType dummy[2]; -}; - -template <typename T> -struct IsMoveOnlyType { - template <typename U> - static YesType Test(const typename U::MoveOnlyTypeForCPP03*); - - template <typename U> - static NoType Test(...); - - static const bool value = - sizeof(Test<T>(0)) == sizeof(YesType) && !std::is_const<T>::value; -}; - -template <typename T> -typename std::enable_if<!IsMoveOnlyType<T>::value, T>::type& Forward(T& t) { - return t; -} - -template <typename T> -typename std::enable_if<IsMoveOnlyType<T>::value, T>::type Forward(T& t) { - return t.Pass(); -} - -template <typename T1> -mojo::Callback<void(T1)> Capture(T1* t1) { - return [t1](T1 got_t1) { *t1 = Forward(got_t1); }; -} - -template <typename T1, typename T2> -mojo::Callback<void(T1, T2)> Capture(T1* t1, T2* t2) { - return [t1, t2](T1 got_t1, T2 got_t2) { - *t1 = Forward(got_t1); - *t2 = Forward(got_t2); - }; -} -// TODO(smklein): (End of stuff copied from template_util.h) - -// Given a FilePtr |file|, write the string |input| and verify -// the output matches the |input|'s size. -void WriteFileHelper(mojo::files::FilePtr* file, const std::string& input) { - std::vector<uint8_t> bytes_to_write; - for (size_t i = 0; i < input.size(); i++) - bytes_to_write.push_back(static_cast<uint8_t>(input[i])); +// Writes |input| to |file| and that the requisite number of bytes was written. +void WriteFileHelper(mojo::SynchronousInterfacePtr<mojo::files::File>* file, + const std::string& input) { + auto bytes_to_write = mojo::Array<uint8_t>::New(input.size()); + memcpy(bytes_to_write.data(), input.data(), input.size()); mojo::files::Error error = mojo::files::Error::INTERNAL; uint32_t num_bytes_written = 0; - (*file)->Write(mojo::Array<uint8_t>::From(bytes_to_write), 0, - mojo::files::Whence::FROM_CURRENT, - Capture(&error, &num_bytes_written)); - ASSERT_TRUE(file->WaitForIncomingResponse()); + ASSERT_TRUE((*file)->Write(std::move(bytes_to_write), 0, + mojo::files::Whence::FROM_CURRENT, &error, + &num_bytes_written)); EXPECT_EQ(mojo::files::Error::OK, error); - EXPECT_EQ(bytes_to_write.size(), num_bytes_written); + EXPECT_EQ(input.size(), num_bytes_written); } -// Given a FilePtr |file|, seek to offset |offset| using whence -// FROM_CURRENT. Verifies the post-seek position is |expected_position|. -void SeekFileHelper(mojo::files::FilePtr* file, +// Seeks to |offset| in |file| (from the current position), verifying that the +// resulting position is |expected_position|. +void SeekFileHelper(mojo::SynchronousInterfacePtr<mojo::files::File>* file, int64_t offset, int64_t expected_position) { int64_t position = -1; mojo::files::Error error = mojo::files::Error::INTERNAL; - (*file)->Seek(offset, mojo::files::Whence::FROM_CURRENT, - Capture(&error, &position)); - ASSERT_TRUE(file->WaitForIncomingResponse()); + ASSERT_TRUE((*file)->Seek(offset, mojo::files::Whence::FROM_CURRENT, &error, + &position)); EXPECT_EQ(mojo::files::Error::OK, error); EXPECT_EQ(expected_position, position); } -// Given a FilePtr |file|, read from the file at offset |offset| (using -// Whence::FROM_CURRENT), and verify the result matches the string -// |expected_output|. -void ReadFileHelper(mojo::files::FilePtr* file, +// Reads |expected_output.size()| bytes from |file| at offset |offset| (using +// Whence::FROM_CURRENT), and verify that the result matches |expected_output|. +void ReadFileHelper(mojo::SynchronousInterfacePtr<mojo::files::File>* file, int64_t offset, const std::string& expected_output) { mojo::Array<uint8_t> bytes_read; mojo::files::Error error = mojo::files::Error::INTERNAL; uint32_t num_bytes_to_read = expected_output.size(); - (*file)->Read(num_bytes_to_read, offset, mojo::files::Whence::FROM_CURRENT, - Capture(&error, &bytes_read)); - ASSERT_TRUE(file->WaitForIncomingResponse()); + ASSERT_TRUE((*file)->Read(num_bytes_to_read, offset, + mojo::files::Whence::FROM_CURRENT, &error, + &bytes_read)); EXPECT_EQ(mojo::files::Error::OK, error); - EXPECT_EQ(num_bytes_to_read, bytes_read.size()); - for (size_t i = 0; i < expected_output.size(); i++) - EXPECT_EQ(static_cast<uint8_t>(expected_output[i]), bytes_read[i]); + ASSERT_EQ(num_bytes_to_read, bytes_read.size()); + EXPECT_EQ( + 0, memcmp(bytes_read.data(), expected_output.data(), num_bytes_to_read)); } -// Open the file named |path_name| in |directory| and verify that no error -// occurs. Returns the newly opened file. -mojo::files::FilePtr OpenFileHelper(mojo::files::DirectoryPtr* directory, - const std::string& path_name) { - mojo::files::FilePtr temp_file; +// Closes |file| and verifies that no error occurs. +void CloseFileHelper(mojo::SynchronousInterfacePtr<mojo::files::File>* file) { mojo::files::Error error = mojo::files::Error::INTERNAL; - (*directory) - ->OpenFile(path_name, GetProxy(&temp_file), - mojo::files::kOpenFlagWrite | mojo::files::kOpenFlagRead, - Capture(&error)); - EXPECT_EQ(true, directory->WaitForIncomingResponse()); + ASSERT_TRUE((*file)->Close(&error)); + EXPECT_EQ(mojo::files::Error::OK, error); +} + +// Opens the file named |path_name| in |directory| and verifies that no error +// occurs. Returns the newly-opened file. +mojo::SynchronousInterfacePtr<mojo::files::File> OpenFileHelper( + mojo::SynchronousInterfacePtr<mojo::files::Directory>* directory, + const std::string& path_name) { + mojo::SynchronousInterfacePtr<mojo::files::File> temp_file; + mojo::files::Error error = mojo::files::Error::INTERNAL; + EXPECT_TRUE( + (*directory) + ->OpenFile(path_name, GetSynchronousProxy(&temp_file), + mojo::files::kOpenFlagWrite | mojo::files::kOpenFlagRead, + &error)); EXPECT_EQ(mojo::files::Error::OK, error); return temp_file; } -// Close the FilePtr |file| and verify that no error occurs. -void CloseFileHelper(mojo::files::FilePtr* file) { - mojo::files::Error error = mojo::files::Error::INTERNAL; - (*file)->Close(Capture(&error)); - ASSERT_TRUE(file->WaitForIncomingResponse()); - EXPECT_EQ(mojo::files::Error::OK, error); -} - TEST_F(FileUtilTest, BasicCreateTemporaryFile) { - mojo::files::FilesPtr files; - application_impl()->ConnectToServiceDeprecated("mojo:files", &files); + mojo::SynchronousInterfacePtr<mojo::files::Files> files; + mojo::ConnectToService(application_impl()->shell(), "mojo:files", + GetSynchronousProxy(&files)); mojo::files::Error error = mojo::files::Error::INTERNAL; - mojo::files::DirectoryPtr directory; - files->OpenFileSystem(nullptr, mojo::GetProxy(&directory), Capture(&error)); - - ASSERT_TRUE(files.WaitForIncomingResponse()); + mojo::SynchronousInterfacePtr<mojo::files::Directory> directory; + ASSERT_TRUE(files->OpenFileSystem( + nullptr, mojo::GetSynchronousProxy(&directory), &error)); EXPECT_EQ(mojo::files::Error::OK, error); - std::string filename1, filename2, filename3; - mojo::files::FilePtr file1, file2, file3; - file1 = CreateTemporaryFileInDir(&directory, &filename1); + std::string filename1; + auto file1 = mojo::SynchronousInterfacePtr<mojo::files::File>::Create( + CreateTemporaryFileInDir(&directory, &filename1)); ASSERT_TRUE(file1); - file2 = CreateTemporaryFileInDir(&directory, &filename2); + std::string filename2; + auto file2 = mojo::SynchronousInterfacePtr<mojo::files::File>::Create( + CreateTemporaryFileInDir(&directory, &filename2)); ASSERT_TRUE(file2); - file3 = CreateTemporaryFileInDir(&directory, &filename3); + std::string filename3; + auto file3 = mojo::SynchronousInterfacePtr<mojo::files::File>::Create( + CreateTemporaryFileInDir(&directory, &filename3)); ASSERT_TRUE(file3); // The temp filenames should not be equal.
diff --git a/services/nacl/nonsfi/BUILD.gn b/services/nacl/nonsfi/BUILD.gn index 7ec07b5..791e5aa 100644 --- a/services/nacl/nonsfi/BUILD.gn +++ b/services/nacl/nonsfi/BUILD.gn
@@ -43,6 +43,8 @@ "//mojo/file_utils", "//mojo/message_pump", "//mojo/nacl/nonsfi:irt_mojo", + "//mojo/services/files/interfaces", + "//mojo/services/files/interfaces:interfaces_sync", "//native_client/src/nonsfi/loader:elf_loader", ] }
diff --git a/services/nacl/nonsfi/content_handler_main_pexe.cc b/services/nacl/nonsfi/content_handler_main_pexe.cc index 33d9467..d590b89 100644 --- a/services/nacl/nonsfi/content_handler_main_pexe.cc +++ b/services/nacl/nonsfi/content_handler_main_pexe.cc
@@ -18,6 +18,8 @@ #include "mojo/public/cpp/application/application_impl.h" #include "mojo/public/cpp/application/connect.h" #include "mojo/public/cpp/bindings/array.h" +#include "mojo/public/cpp/bindings/synchronous_interface_ptr.h" +#include "mojo/services/files/interfaces/directory.mojom-sync.h" #include "mojo/services/files/interfaces/files.mojom.h" #include "services/nacl/nonsfi/pnacl_compile.mojom.h" #include "services/nacl/nonsfi/pnacl_link.mojom.h" @@ -86,7 +88,7 @@ mojo::ConnectToService(app->shell(), "mojo:files", GetProxy(&files_)); mojo::files::Error error = mojo::files::Error::INTERNAL; files_->OpenFileSystem("app_persistent_cache", - GetProxy(&nexe_cache_directory), + GetSynchronousProxy(&nexe_cache_directory_), [&error](mojo::files::Error e) { error = e; }); CHECK(files_.WaitForIncomingResponse()); CHECK_EQ(mojo::files::Error::OK, error); @@ -102,10 +104,8 @@ int AccessFileFromCache(std::string& digest) { mojo::files::Error error = mojo::files::Error::INTERNAL; mojo::files::FilePtr nexe_cache_file; - nexe_cache_directory->OpenFile( - digest, GetProxy(&nexe_cache_file), mojo::files::kOpenFlagRead, - [&error](mojo::files::Error e) { error = e; }); - CHECK(nexe_cache_directory.WaitForIncomingResponse()); + CHECK(nexe_cache_directory_->OpenFile(digest, GetProxy(&nexe_cache_file), + mojo::files::kOpenFlagRead, &error)); if (mojo::files::Error::OK == error) // Copy the mojo cached file into an open temporary file. return ::nacl::MojoFileToTempFileDescriptor(nexe_cache_file.Pass()); @@ -119,8 +119,9 @@ // First, open a "temporary" file. mojo::files::Error error = mojo::files::Error::INTERNAL; std::string temp_file_name; - mojo::files::FilePtr nexe_cache_file = file_utils::CreateTemporaryFileInDir( - &nexe_cache_directory, &temp_file_name); + auto nexe_cache_file = + mojo::files::FilePtr::Create(file_utils::CreateTemporaryFileInDir( + &nexe_cache_directory_, &temp_file_name)); CHECK(nexe_cache_file); // Copy the contents of nexe_fd into the temporary Mojo file. @@ -129,9 +130,7 @@ // The file is named after the hash of the requesting pexe. // This makes it usable by future requests for the same pexe under different // names. It also atomically moves the entire temp file. - nexe_cache_directory->Rename(temp_file_name, digest, - [&error](mojo::files::Error e) { error = e; }); - CHECK(nexe_cache_directory.WaitForIncomingResponse()); + CHECK(nexe_cache_directory_->Rename(temp_file_name, digest, &error)); CHECK_EQ(mojo::files::Error::OK, error); } @@ -203,7 +202,7 @@ } private: - mojo::files::DirectoryPtr nexe_cache_directory; + mojo::SynchronousInterfacePtr<mojo::files::Directory> nexe_cache_directory_; mojo::files::FilesPtr files_; mojo::ContentHandlerFactory content_handler_factory_; mojo::nacl::PexeCompilerInitPtr compiler_init_;