diff --git a/chrome/browser/ui/app_list/search/ranking/ranker_delegate.cc b/chrome/browser/ui/app_list/search/ranking/ranker_delegate.cc index ae6d67fbe367e..43ebbfa9710c4 100644 --- a/chrome/browser/ui/app_list/search/ranking/ranker_delegate.cc +++ b/chrome/browser/ui/app_list/search/ranking/ranker_delegate.cc @@ -15,9 +15,34 @@ #include "chrome/browser/ui/app_list/search/util/score_normalizer.pb.h" namespace app_list { +namespace { + +// A standard write delay used for protos without time-sensitive writes. This is +// intended to be slightly longer than the longest conceivable latency for a +// search. +constexpr base::TimeDelta kStandardWriteDelay = base::Seconds(3); + +// No write delay for protos with time-sensitive writes. +constexpr base::TimeDelta kNoWriteDelay = base::Seconds(0); + +} // namespace RankerDelegate::RankerDelegate(Profile* profile, SearchController* controller) { - // TODO(crbug.com/1274853): Ranking removed due to stability concerns. + const auto state_dir = RankerStateDirectory(profile); + + // Main result and category ranking. + AddRanker(std::make_unique( + PersistentProto( + state_dir.AppendASCII("score_norm.pb"), kStandardWriteDelay))); + + // Result post-processing. + AddRanker(std::make_unique()); + AddRanker(std::make_unique()); + + // Result removal. + AddRanker(std::make_unique( + PersistentProto( + state_dir.AppendASCII("removed_results.pb"), kNoWriteDelay))); } RankerDelegate::~RankerDelegate() {} diff --git a/chrome/browser/ui/app_list/search/util/persistent_proto.h b/chrome/browser/ui/app_list/search/util/persistent_proto.h index 0fb78ed299b87..18de4ad5c4a0e 100644 --- a/chrome/browser/ui/app_list/search/util/persistent_proto.h +++ b/chrome/browser/ui/app_list/search/util/persistent_proto.h @@ -22,20 +22,6 @@ #include "base/time/time.h" namespace app_list { -namespace { - -template -class MessageStorage; - -template -class RootMessageStorage; - -template -class EmbeddedMessageStorage; - -} // namespace - -// API ------------------------------------------------------------------------ // The result of reading a backing file from disk. These values persist to logs. // Entries should not be renumbered and numeric values should never be reused. @@ -44,7 +30,8 @@ enum class ReadStatus { kMissing = 1, kReadError = 2, kParseError = 3, - // Provided when an embedded message completes a no-op 'read'. + // kNoop is currently unused, but was previously used when no read was + // required. kNoop = 4, kMaxValue = kNoop, }; @@ -58,130 +45,6 @@ enum class WriteStatus { kMaxValue = kSerializationError, }; -using PersistentProtoReadCallback = base::OnceCallback; -using PersistentProtoWriteCallback = base::RepeatingCallback; - -// PersistentProto wraps a proto class and persists it to disk. Usage summary: -// -// - PProtos are constructed with a file path and a write delay. No logic -// happens at construction, instead Init must be called. -// -// - Callbacks registered with RegisterOnRead and RegisterOnWrite will be -// called when appropriate. It is recommended to register these before -// calling Init. -// -// - Init is asynchronous, and usage before RegisterOnRead callbacks are -// called will crash. -// -// - After initialization, a PProto acts identically to a regular proto. Use -// my_pproto->method() to access methods. -// -// - Call QueueWrite or StartWrite to write to disk. -// -// READING -// The backing file is read asynchronously from disk once at initialization, -// and RegisterOnRead callbacks are run once this is done. Until they are -// called, has_value is false and get() will always return nullptr. If no proto -// file exists on disk, or it is invalid, a blank proto is constructed and -// immediately written to disk. -// -// WRITING -// Writes must be triggered manually. Two methods are available: -// - QueueWrite delays writing to disk for |write_delay| time, in order to -// batch successive writes. -// - StartWrite writes to disk as soon as the task scheduler allows. -// RegisterOnWrite callbacks are run each time a write has completed. -// -// EMBEDDED MESSAGES -// To pass around a sub-message of a PProto that can do writes, use the Wrap -// method as follows. -// -// PersistentProto my_message = -// pproto.Wrap(pproto->mutable_my_message()); -// -// These are a drop-in replacement for a regular PProto, except that: -// -// - Calling Init is not necessary, because a Wrap'd PProto must have been -// created from an existing, initialized PProto. If Init is called, it -// immediately calls all RegisterOnRead callbacks and is otherwise a -// no-op. -// -// - The root PProto and embedded message PProtos form a tree. Writes called -// on any node of the tree trickle up to the root, and cause a write of the -// entire tree. This will trigger write callbacks that were registered from -// anywhere in the tree. Take care that the data in the embedded message is -// safe to be serialized at any moment. -// -// Warning: The top-level PProto object must outlive all PProtos generated with -// Wrap. -// -// Warning: Because a Wrap'd PProto object only retains a pointer to a message, -// reassigning that field in the parent PProto will silently invalidate it. -// This includes calling Purge. -// -// Warning: It is possible to Wrap a message that is part of another proto, eg. -// pproto_1.Wrap(proto_2->...). Don't do this! -template -class PersistentProto { - public: - PersistentProto(const base::FilePath& path, - const base::TimeDelta write_delay) - : storage_(std::make_unique>(path, write_delay)) {} - ~PersistentProto() {} - - PersistentProto(PersistentProto&& other) { - storage_ = std::move(other.storage_); - } - - PersistentProto& operator=(PersistentProto&& other) { - storage_ = std::move(other.storage_); - } - - PersistentProto(const PersistentProto&) = delete; - PersistentProto& operator=(const PersistentProto&) = delete; - - template - PersistentProto Wrap(U* message) { - DCHECK(initialized()); - return PersistentProto( - std::make_unique>(storage_.get(), message)); - } - - // Initialization. - void Init() { storage_->Init(); } - void RegisterOnRead(PersistentProtoReadCallback on_read) { - storage_->RegisterOnRead(std::move(on_read)); - } - void RegisterOnWrite(PersistentProtoWriteCallback on_write) { - storage_->RegisterOnWrite(std::move(on_write)); - } - - // Getters. - T* get() { return storage_->get(); } - T* operator->() { return storage_->get(); } - const T* operator->() const { return storage_->get(); } - T operator*() { return *(storage_->get()); } - - // Testing initialization and value. - bool initialized() const { return storage_ && storage_->initialized(); } - constexpr bool has_value() const { return storage_ && storage_->has_value(); } - constexpr explicit operator bool() const { return has_value(); } - - // Writing and deletion. - void QueueWrite() { storage_->QueueWrite(); } - void StartWrite() { storage_->StartWrite(); } - void Purge() { storage_->Purge(); } - - private: - template - friend class PersistentProto; - - PersistentProto(std::unique_ptr> storage) - : storage_(std::move(storage)) {} - - std::unique_ptr> storage_; -}; - namespace { template @@ -221,98 +84,94 @@ WriteStatus Write(const base::FilePath& filepath, return WriteStatus::kOk; } -// STORAGE INTERFACES ---------------------------------------------------------- - -// All methods that don't require knowing the underlying proto's type. -class GenericMessageStorage { - public: - virtual bool initialized() const = 0; - virtual constexpr bool has_value() const = 0; - virtual constexpr explicit operator bool() const = 0; - - virtual void RegisterOnRead(PersistentProtoReadCallback on_read) = 0; - virtual void RegisterOnWrite(PersistentProtoWriteCallback on_write) = 0; - - virtual void Init() = 0; - virtual void QueueWrite() = 0; - virtual void StartWrite() = 0; - virtual void Purge() = 0; -}; +} // namespace -// Storage base class that abstracts over both root protos and embedded -// messages. +// PersistentProto wraps a proto class and persists it to disk. Usage summary: +// - Init is asynchronous, usage before |on_read| is called will crash. +// - pproto->Method() will call Method on the underlying proto. +// - Call QueueWrite() to write to disk. +// +// Reading. The backing file is read asynchronously from disk once at +// initialization, and the |on_read| callback is run once this is done. Until +// |on_read| is called, has_value is false and get() will always return nullptr. +// If no proto file exists on disk, or it is invalid, a blank proto is +// constructed and immediately written to disk. +// +// Writing. Writes must be triggered manually. Two methods are available: +// - QueueWrite() delays writing to disk for |write_delay| time, in order to +// batch successive writes. +// - StartWrite() writes to disk as soon as the task scheduler allows. +// The |on_write| callback is run each time a write has completed. template -class MessageStorage : public GenericMessageStorage { +class PersistentProto { public: - MessageStorage() {} - virtual ~MessageStorage() {} + using ReadCallback = base::OnceCallback; + using WriteCallback = base::RepeatingCallback; - MessageStorage(const MessageStorage&) = delete; - MessageStorage& operator=(const MessageStorage&) = delete; - - virtual T* get() = 0; - virtual T* operator->() = 0; - virtual const T* operator->() const = 0; - virtual T operator*() = 0; -}; - -// ROOT MESSAGE STORAGE -------------------------------------------------------- - -// Storage for a top-level proto. This actually controls reading and writing. -template -class RootMessageStorage : public MessageStorage { - public: - RootMessageStorage(const base::FilePath& path, - const base::TimeDelta write_delay) + PersistentProto(const base::FilePath& path, const base::TimeDelta write_delay) : path_(path), write_delay_(write_delay), task_runner_(base::ThreadPool::CreateSequencedTaskRunner( {base::TaskPriority::BEST_EFFORT, base::MayBlock(), base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN})) {} - ~RootMessageStorage() override {} + ~PersistentProto() = default; + + PersistentProto(const PersistentProto&) = delete; + PersistentProto& operator=(const PersistentProto&) = delete; + + PersistentProto(PersistentProto&& other) { + path_ = other.path_; + write_delay_ = other.write_delay_; + initialized_ = other.initialized_; + write_is_queued_ = false; + purge_after_reading_ = other.purge_after_reading_; + read_callbacks_ = std::move(other.read_callbacks_); + write_callbacks_ = std::move(other.write_callbacks_); + task_runner_ = std::move(other.task_runner_); + proto_ = std::move(other.proto_); + } - void Init() override { - DCHECK(!initialized_); + void Init() { task_runner_->PostTaskAndReplyWithResult( FROM_HERE, base::BindOnce(&Read, path_), - base::BindOnce(&RootMessageStorage::OnReadComplete, + base::BindOnce(&PersistentProto::OnReadComplete, weak_factory_.GetWeakPtr())); } - void RegisterOnRead(PersistentProtoReadCallback on_read) override { + void RegisterOnRead(ReadCallback on_read) { read_callbacks_.push_back(std::move(on_read)); } - void RegisterOnWrite(PersistentProtoWriteCallback on_write) override { + void RegisterOnWrite(WriteCallback on_write) { write_callbacks_.push_back(std::move(on_write)); } - T* get() override { return proto_.get(); } + T* get() { return proto_.get(); } - T* operator->() override { + T* operator->() { CHECK(proto_); return proto_.get(); } - const T* operator->() const override { + const T* operator->() const { CHECK(proto_); return proto_.get(); } - T operator*() override { + T operator*() { CHECK(proto_); return *proto_; } - bool initialized() const override { return initialized_; } + bool initialized() const { return initialized_; } - constexpr bool has_value() const override { return proto_.get() != nullptr; } + constexpr bool has_value() const { return proto_.get() != nullptr; } - constexpr explicit operator bool() const override { return has_value(); } + constexpr explicit operator bool() const { return has_value(); } // Write the backing proto to disk after |save_delay_ms_| has elapsed. - void QueueWrite() override { + void QueueWrite() { DCHECK(proto_); if (!proto_) return; @@ -324,13 +183,13 @@ class RootMessageStorage : public MessageStorage { base::SequencedTaskRunnerHandle::Get()->PostDelayedTask( FROM_HERE, - base::BindOnce(&RootMessageStorage::OnQueueWrite, + base::BindOnce(&PersistentProto::OnQueueWrite, weak_factory_.GetWeakPtr()), write_delay_); } // Write the backing proto to disk 'now'. - void StartWrite() override { + void StartWrite() { DCHECK(proto_); if (!proto_) return; @@ -348,7 +207,7 @@ class RootMessageStorage : public MessageStorage { // active. task_runner_->PostTaskAndReplyWithResult( FROM_HERE, base::BindOnce(&Write, path_, proto_str), - base::BindOnce(&RootMessageStorage::OnWriteComplete, + base::BindOnce(&PersistentProto::OnWriteComplete, weak_factory_.GetWeakPtr())); } @@ -357,7 +216,7 @@ class RootMessageStorage : public MessageStorage { // backing file is read from disk. In this case, the file is overwritten after // it has been read. In either case, the file is written as soon as possible, // skipping the |save_delay_ms_| wait time. - void Purge() override { + void Purge() { if (proto_) { proto_.reset(); proto_ = std::make_unique(); @@ -389,6 +248,7 @@ class RootMessageStorage : public MessageStorage { for (auto& cb : read_callbacks_) { std::move(cb).Run(result.first); } + read_callbacks_.clear(); } void OnWriteComplete(const WriteStatus status) { @@ -424,76 +284,18 @@ class RootMessageStorage : public MessageStorage { bool purge_after_reading_ = false; // Run when the cache finishes reading from disk. - std::vector read_callbacks_; + std::vector read_callbacks_; // Run when the cache finishes writing to disk. - std::vector write_callbacks_; + std::vector write_callbacks_; // The proto itself. std::unique_ptr proto_; scoped_refptr task_runner_; - base::WeakPtrFactory weak_factory_{this}; -}; - -// EMBEDDED MESSAGE STORAGE ---------------------------------------------------- - -// Storage for an embedded message, which delegates writes to another storage -// object. -template -class EmbeddedMessageStorage : public MessageStorage { - public: - EmbeddedMessageStorage(GenericMessageStorage* storage, T* message) - : storage_(storage), message_(message) {} - - ~EmbeddedMessageStorage() override {} - - void Init() override { - // Initialization is a no-op for embedded messages because, to be - // constructed, the root message must have already been initialized. So - // immediately call all read callbacks. - for (auto& cb : read_callbacks_) { - std::move(cb).Run(ReadStatus::kNoop); - } - } - - void RegisterOnRead(PersistentProtoReadCallback on_read) override { - read_callbacks_.push_back(std::move(on_read)); - } - - void RegisterOnWrite(PersistentProtoWriteCallback on_write) override { - // Delegate calling the write callback to the storage. - storage_->RegisterOnWrite(std::move(on_write)); - } - - T* get() override { return message_; } - T* operator->() override { return message_; } - const T* operator->() const override { return message_; } - T operator*() override { return *message_; } - - // For this have been constructed, the parent proto must have been initialized - // and have a value. So all init-querying methods can trivially return true. - bool initialized() const override { return true; } - constexpr bool has_value() const override { return true; } - constexpr explicit operator bool() const override { return true; } - - void QueueWrite() override { storage_->QueueWrite(); } - void StartWrite() override { storage_->StartWrite(); } - - void Purge() override { - message_->Clear(); - StartWrite(); - } - - private: - GenericMessageStorage* storage_; - T* message_; - - // All callbacks passed to |RegisterOnRead|. - std::vector read_callbacks_; + base::WeakPtrFactory weak_factory_{this}; }; -} // namespace } // namespace app_list #endif // CHROME_BROWSER_UI_APP_LIST_SEARCH_UTIL_PERSISTENT_PROTO_H_ diff --git a/chrome/browser/ui/app_list/search/util/persistent_proto_test.proto b/chrome/browser/ui/app_list/search/util/persistent_proto_test.proto index 423c433363b80..d67d2d47c3d1e 100644 --- a/chrome/browser/ui/app_list/search/util/persistent_proto_test.proto +++ b/chrome/browser/ui/app_list/search/util/persistent_proto_test.proto @@ -8,14 +8,7 @@ option optimize_for = LITE_RUNTIME; package app_list; -// Test protos used for the PersistentProto unit tests. - +// A proto used with the PersistentProto class in unit tests. message TestProto { optional int64 value = 1; - optional TestProto msg_one = 2; - optional AnotherTestProto msg_two = 3; -} - -message AnotherTestProto { - optional int64 value = 1; } diff --git a/chrome/browser/ui/app_list/search/util/persistent_proto_unittest.cc b/chrome/browser/ui/app_list/search/util/persistent_proto_unittest.cc index 3cfbb72f989ae..eb889e7ab249e 100644 --- a/chrome/browser/ui/app_list/search/util/persistent_proto_unittest.cc +++ b/chrome/browser/ui/app_list/search/util/persistent_proto_unittest.cc @@ -22,18 +22,6 @@ namespace { // Populate |proto| with some test data. void PopulateTestProto(TestProto* proto) { proto->set_value(12345); - - // A sub-proto - TestProto* child = proto->mutable_msg_one(); - child->set_value(54321); - - // A sub-proto of the sub-proto. - TestProto* grandchild = child->mutable_msg_one(); - grandchild->set_value(54321); - - // A sibling to the sub-proto. - AnotherTestProto* sibling = proto->mutable_msg_two(); - sibling->set_value(15243); } // Make a proto with test data. @@ -45,30 +33,9 @@ TestProto MakeTestProto() { // Returns whether |actual| and |expected| are equal. bool ProtoEquals(const TestProto* actual, const TestProto* expected) { - bool ok = true; - - if (!actual->has_value() || !expected->has_value()) { - ok &= actual->has_value() == expected->has_value(); - } else { - ok &= actual->value() == expected->value(); - } - - if (!actual->has_msg_one() || !expected->has_msg_one()) { - ok &= actual->has_msg_one() == expected->has_msg_one(); - } else { - ok &= ProtoEquals(&actual->msg_one(), &expected->msg_one()); - } - - if (!actual->has_msg_two()) { - ok &= !expected->has_msg_two(); - } else if (!actual->msg_two().has_value() || - !expected->msg_two().has_value()) { - ok &= expected->msg_two().has_value() == actual->msg_two().has_value(); - } else { - ok &= actual->msg_two().value() == expected->msg_two().value(); - } - - return ok; + if (!actual->has_value()) + return !expected->has_value(); + return actual->value() == expected->value(); } base::TimeDelta WriteDelay() { @@ -101,31 +68,28 @@ class PersistentProtoTest : public testing::Test { ASSERT_TRUE(base::WriteFile(GetPath(), proto.SerializeAsString())); } - void RegisterOnRead(const ReadStatus status) { + void OnRead(const ReadStatus status) { read_status_ = status; ++read_count_; } base::OnceCallback ReadCallback() { - return base::BindOnce(&PersistentProtoTest::RegisterOnRead, - base::Unretained(this)); + return base::BindOnce(&PersistentProtoTest::OnRead, base::Unretained(this)); } - void RegisterOnWrite(const WriteStatus status) { + void OnWrite(const WriteStatus status) { ASSERT_EQ(status, WriteStatus::kOk); ++write_count_; } base::RepeatingCallback WriteCallback() { - return base::BindRepeating(&PersistentProtoTest::RegisterOnWrite, + return base::BindRepeating(&PersistentProtoTest::OnWrite, base::Unretained(this)); } void Wait() { task_environment_.RunUntilIdle(); } // Records the information passed to the callbacks for later expectation. - // |read_status_| is left intentionally unassigned so all states can be - // distinguished. ReadStatus read_status_; int read_count_ = 0; int write_count_ = 0; @@ -140,17 +104,19 @@ class PersistentProtoTest : public testing::Test { // after that. TEST_F(PersistentProtoTest, Initialization) { PersistentProto pproto(GetPath(), WriteDelay()); + pproto.RegisterOnRead(ReadCallback()); + pproto.RegisterOnWrite(WriteCallback()); pproto.Init(); EXPECT_EQ(pproto.get(), nullptr); - EXPECT_FALSE(pproto.initialized()); Wait(); - EXPECT_TRUE(pproto.initialized()); EXPECT_NE(pproto.get(), nullptr); } // Test bool conversion and has_value. TEST_F(PersistentProtoTest, BoolTests) { PersistentProto pproto(GetPath(), WriteDelay()); + pproto.RegisterOnRead(ReadCallback()); + pproto.RegisterOnWrite(WriteCallback()); pproto.Init(); EXPECT_EQ(pproto.get(), nullptr); EXPECT_FALSE(pproto); @@ -164,6 +130,8 @@ TEST_F(PersistentProtoTest, BoolTests) { // Test -> and *. TEST_F(PersistentProtoTest, Getters) { PersistentProto pproto(GetPath(), WriteDelay()); + pproto.RegisterOnRead(ReadCallback()); + pproto.RegisterOnWrite(WriteCallback()); pproto.Init(); Wait(); // We're really just checking these don't crash. @@ -174,23 +142,27 @@ TEST_F(PersistentProtoTest, Getters) { EXPECT_EQ(val.value(), 1); } -// Test that the pproto correctly loads an on-disk proto into memory. +// Test that the pproto correctly saves the in-memory proto to disk. TEST_F(PersistentProtoTest, Read) { - const auto test_proto = MakeTestProto(); - WriteToDisk(test_proto); - PersistentProto pproto(GetPath(), WriteDelay()); pproto.RegisterOnRead(ReadCallback()); pproto.RegisterOnWrite(WriteCallback()); pproto.Init(); + // Underlying proto should be nullptr until read is complete. EXPECT_EQ(pproto.get(), nullptr); Wait(); - EXPECT_EQ(read_status_, ReadStatus::kOk); + EXPECT_EQ(read_status_, ReadStatus::kMissing); EXPECT_EQ(read_count_, 1); - EXPECT_EQ(write_count_, 0); - EXPECT_NE(pproto.get(), nullptr); - EXPECT_TRUE(ProtoEquals(pproto.get(), &test_proto)); + EXPECT_EQ(write_count_, 1); + + PopulateTestProto(pproto.get()); + pproto.StartWrite(); + Wait(); + EXPECT_EQ(write_count_, 2); + + TestProto written = ReadFromDisk(); + EXPECT_TRUE(ProtoEquals(&written, pproto.get())); } // Test that invalid files on disk are handled correctly. @@ -207,27 +179,23 @@ TEST_F(PersistentProtoTest, ReadInvalidProto) { EXPECT_EQ(write_count_, 1); } -// Test that the pproto correctly saves the in-memory proto to disk. +// Test that the pproto correctly loads an on-disk proto into memory. TEST_F(PersistentProtoTest, Write) { + const auto test_proto = MakeTestProto(); + WriteToDisk(test_proto); + PersistentProto pproto(GetPath(), WriteDelay()); pproto.RegisterOnRead(ReadCallback()); pproto.RegisterOnWrite(WriteCallback()); pproto.Init(); - // Underlying proto should be nullptr until read is complete. EXPECT_EQ(pproto.get(), nullptr); Wait(); - EXPECT_EQ(read_status_, ReadStatus::kMissing); + EXPECT_EQ(read_status_, ReadStatus::kOk); EXPECT_EQ(read_count_, 1); - EXPECT_EQ(write_count_, 1); - - PopulateTestProto(pproto.get()); - pproto.StartWrite(); - Wait(); - EXPECT_EQ(write_count_, 2); - - TestProto written = ReadFromDisk(); - EXPECT_TRUE(ProtoEquals(&written, pproto.get())); + EXPECT_EQ(write_count_, 0); + EXPECT_NE(pproto.get(), nullptr); + EXPECT_TRUE(ProtoEquals(pproto.get(), &test_proto)); } // Test that several saves all happen correctly. @@ -277,94 +245,4 @@ TEST_F(PersistentProtoTest, QueueWrites) { EXPECT_EQ(write_count_, 1); } -// Test that a call to Purge deletes a proto from memory and disk. -TEST_F(PersistentProtoTest, Purge) { - WriteToDisk(MakeTestProto()); - - PersistentProto pproto(GetPath(), WriteDelay()); - pproto.Init(); - Wait(); - - // Values read from disk. - EXPECT_TRUE(pproto); - EXPECT_EQ(pproto->value(), 12345); - - pproto.Purge(); - - // The backing proto should now be initialized but blank. - EXPECT_TRUE(pproto); - EXPECT_FALSE(pproto->has_value()); - - // The on-disk proto should have also been replaced with a blank copy after - // writes have been completed. - Wait(); - EXPECT_FALSE(ReadFromDisk().has_value()); -} - -// Tests that an embedded message can be created and used. -TEST_F(PersistentProtoTest, EmbeddedMessage) { - WriteToDisk(MakeTestProto()); - - PersistentProto pproto(GetPath(), WriteDelay()); - pproto.Init(); - Wait(); - - // Check that the embedded message has the correct data. - PersistentProto msg_one = pproto.Wrap(pproto->mutable_msg_one()); - EXPECT_EQ(msg_one->value(), 54321); - - // Check that modifying the embedded message also modifies the parent. - msg_one->set_value(1123); - EXPECT_EQ(pproto->msg_one().value(), 1123); - - // Check that calling write on the message object actually results in a write. - msg_one->set_value(3211); - msg_one.StartWrite(); - Wait(); - EXPECT_EQ(ReadFromDisk().msg_one().value(), 3211); - - // Check that calling write on a sibling message object results in a write. - PersistentProto msg_two = - pproto.Wrap(pproto->mutable_msg_two()); - msg_one->set_value(1231); - msg_two.StartWrite(); - Wait(); - EXPECT_EQ(ReadFromDisk().msg_one().value(), 1231); -} - -// Tests that callbacks on embedded messages are handled correctly. -TEST_F(PersistentProtoTest, EmbeddedMessageCallbacks) { - PersistentProto pproto(GetPath(), WriteDelay()); - pproto.Init(); - Wait(); - - auto msg_one = pproto.Wrap(pproto->mutable_msg_one()); - msg_one.RegisterOnRead(ReadCallback()); - msg_one.RegisterOnWrite(WriteCallback()); - - // Running Init on the embedded message should immediately run read callbacks. - msg_one.Init(); - EXPECT_EQ(read_status_, ReadStatus::kNoop); - EXPECT_EQ(read_count_, 1); - - // Write callbacks registered on the embedded message should be triggered from - // a write on the... - - // ... parent pproto. - pproto.StartWrite(); - Wait(); - EXPECT_EQ(write_count_, 1); - - // ... embedded message. - msg_one.StartWrite(); - Wait(); - EXPECT_EQ(write_count_, 2); - - // ... embedded message within embedded message. - auto grandchild = msg_one.Wrap(msg_one->mutable_msg_one()); - grandchild.StartWrite(); - Wait(); - EXPECT_EQ(write_count_, 3); -} - } // namespace app_list diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml index daf4f9b7de895..6bce7a2ea74f6 100644 --- a/tools/metrics/histograms/enums.xml +++ b/tools/metrics/histograms/enums.xml @@ -68612,7 +68612,7 @@ Called by update_net_trust_anchors.py.--> - + diff --git a/tools/metrics/histograms/metadata/apps/histograms.xml b/tools/metrics/histograms/metadata/apps/histograms.xml index 3a1379c22defd..98d312d59b821 100644 --- a/tools/metrics/histograms/metadata/apps/histograms.xml +++ b/tools/metrics/histograms/metadata/apps/histograms.xml @@ -836,8 +836,7 @@ chromium-metrics-reviews@google.com. Various error states on reading a backing file from disk, in the score normalizer used by the Chrome OS launcher. Emitted once when a persistent proto initially reads from disk. Some number of Missing values are expected, - eg. because it is logged for each new user added to a device. Some number of - No-op values are expected if embedded messages are used. + eg. because it is logged for each new user added to a device.