diff --git a/base/BUILD.gn b/base/BUILD.gn index 9a62e82..b1f6bcc 100644 --- a/base/BUILD.gn +++ b/base/BUILD.gn
@@ -1025,6 +1025,8 @@ "unguessable_token.h", "value_conversions.cc", "value_conversions.h", + "value_iterators.cc", + "value_iterators.h", "values.cc", "values.h", "version.cc", @@ -2249,6 +2251,7 @@ "tracked_objects_unittest.cc", "tuple_unittest.cc", "unguessable_token_unittest.cc", + "value_iterators_unittest.cc", "values_unittest.cc", "version_unittest.cc", "vlog_unittest.cc",
diff --git a/base/allocator/partition_allocator/partition_alloc.cc b/base/allocator/partition_allocator/partition_alloc.cc index 7541eed..1c23f10 100644 --- a/base/allocator/partition_allocator/partition_alloc.cc +++ b/base/allocator/partition_allocator/partition_alloc.cc
@@ -1100,7 +1100,11 @@ DCHECK(page->num_unprovisioned_slots < bucket_num_slots); size_t num_slots = bucket_num_slots - page->num_unprovisioned_slots; char slot_usage[max_slot_count]; +#if !defined(OS_WIN) + // The last freelist entry should not be discarded when using OS_WIN. + // DiscardVirtualMemory makes the contents of discarded memory undefined. size_t last_slot = static_cast<size_t>(-1); +#endif memset(slot_usage, 1, num_slots); char* ptr = reinterpret_cast<char*>(PartitionPageToPointer(page)); PartitionFreelistEntry* entry = page->freelist_head; @@ -1111,6 +1115,7 @@ DCHECK(slotIndex < num_slots); slot_usage[slotIndex] = 0; entry = PartitionFreelistMask(entry->next); +#if !defined(OS_WIN) // If we have a slot where the masked freelist entry is 0, we can // actually discard that freelist entry because touching a discarded // page is guaranteed to return original content or 0. @@ -1118,6 +1123,7 @@ // because the masking function is negation.) if (!PartitionFreelistMask(entry)) last_slot = slotIndex; +#endif } // If the slot(s) at the end of the slot span are not in used, we can @@ -1162,7 +1168,9 @@ *entry_ptr = PartitionFreelistMask(entry); entry_ptr = reinterpret_cast<PartitionFreelistEntry**>(entry); num_new_entries++; +#if !defined(OS_WIN) last_slot = slotIndex; +#endif } // Terminate the freelist chain. *entry_ptr = nullptr; @@ -1185,8 +1193,12 @@ // null, we can discard that pointer value too. char* begin_ptr = ptr + (i * slot_size); char* end_ptr = begin_ptr + slot_size; +#if !defined(OS_WIN) if (i != last_slot) begin_ptr += sizeof(PartitionFreelistEntry); +#else + begin_ptr += sizeof(PartitionFreelistEntry); +#endif begin_ptr = reinterpret_cast<char*>( RoundUpToSystemPage(reinterpret_cast<size_t>(begin_ptr))); end_ptr = reinterpret_cast<char*>(
diff --git a/base/allocator/partition_allocator/partition_alloc_unittest.cc b/base/allocator/partition_allocator/partition_alloc_unittest.cc index 77c3823..740e984 100644 --- a/base/allocator/partition_allocator/partition_alloc_unittest.cc +++ b/base/allocator/partition_allocator/partition_alloc_unittest.cc
@@ -1839,14 +1839,22 @@ EXPECT_TRUE(stats); EXPECT_TRUE(stats->is_valid); EXPECT_EQ(0u, stats->decommittable_bytes); +#if defined(OS_WIN) + EXPECT_EQ(0u, stats->discardable_bytes); +#else EXPECT_EQ(kSystemPageSize, stats->discardable_bytes); +#endif EXPECT_EQ(kSystemPageSize, stats->active_bytes); EXPECT_EQ(2 * kSystemPageSize, stats->resident_bytes); } CheckPageInCore(ptr1 - kPointerOffset, true); PartitionPurgeMemoryGeneric(generic_allocator.root(), PartitionPurgeDiscardUnusedSystemPages); +#if defined(OS_WIN) + CheckPageInCore(ptr1 - kPointerOffset, true); +#else CheckPageInCore(ptr1 - kPointerOffset, false); +#endif PartitionFreeGeneric(generic_allocator.root(), ptr2); } @@ -1969,7 +1977,11 @@ EXPECT_TRUE(stats); EXPECT_TRUE(stats->is_valid); EXPECT_EQ(0u, stats->decommittable_bytes); +#if defined(OS_WIN) + EXPECT_EQ(kSystemPageSize, stats->discardable_bytes); +#else EXPECT_EQ(2 * kSystemPageSize, stats->discardable_bytes); +#endif EXPECT_EQ(kSystemPageSize, stats->active_bytes); EXPECT_EQ(4 * kSystemPageSize, stats->resident_bytes); } @@ -1981,7 +1993,11 @@ PartitionPurgeDiscardUnusedSystemPages); EXPECT_EQ(1u, page->num_unprovisioned_slots); CheckPageInCore(ptr1 - kPointerOffset, true); +#if defined(OS_WIN) + CheckPageInCore(ptr1 - kPointerOffset + kSystemPageSize, true); +#else CheckPageInCore(ptr1 - kPointerOffset + kSystemPageSize, false); +#endif CheckPageInCore(ptr1 - kPointerOffset + (kSystemPageSize * 2), true); CheckPageInCore(ptr1 - kPointerOffset + (kSystemPageSize * 3), false);
diff --git a/base/value_iterators.cc b/base/value_iterators.cc new file mode 100644 index 0000000..ba9c730 --- /dev/null +++ b/base/value_iterators.cc
@@ -0,0 +1,228 @@ +// Copyright 2017 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 "base/value_iterators.h" + +namespace base { + +namespace detail { + +// ---------------------------------------------------------------------------- +// dict_iterator. + +dict_iterator::pointer::pointer(const reference& ref) : ref_(ref) {} + +dict_iterator::pointer::pointer(const pointer& ptr) = default; + +dict_iterator::dict_iterator(DictStorage::iterator dict_iter) + : dict_iter_(dict_iter) {} + +dict_iterator::dict_iterator(const dict_iterator& dict_iter) = default; + +dict_iterator& dict_iterator::operator=(const dict_iterator& dict_iter) = + default; + +dict_iterator::~dict_iterator() = default; + +dict_iterator::reference dict_iterator::operator*() { + return {dict_iter_->first, *dict_iter_->second}; +} + +dict_iterator::pointer dict_iterator::operator->() { + return pointer(operator*()); +} + +dict_iterator& dict_iterator::operator++() { + ++dict_iter_; + return *this; +} + +dict_iterator dict_iterator::operator++(int) { + dict_iterator tmp(*this); + ++dict_iter_; + return tmp; +} + +dict_iterator& dict_iterator::operator--() { + --dict_iter_; + return *this; +} + +dict_iterator dict_iterator::operator--(int) { + dict_iterator tmp(*this); + --dict_iter_; + return tmp; +} + +bool operator==(const dict_iterator& lhs, const dict_iterator& rhs) { + return lhs.dict_iter_ == rhs.dict_iter_; +} + +bool operator!=(const dict_iterator& lhs, const dict_iterator& rhs) { + return !(lhs == rhs); +} + +// ---------------------------------------------------------------------------- +// const_dict_iterator. + +const_dict_iterator::pointer::pointer(const reference& ref) : ref_(ref) {} + +const_dict_iterator::pointer::pointer(const pointer& ptr) = default; + +const_dict_iterator::const_dict_iterator(DictStorage::const_iterator dict_iter) + : dict_iter_(dict_iter) {} + +const_dict_iterator::const_dict_iterator(const const_dict_iterator& dict_iter) = + default; + +const_dict_iterator& const_dict_iterator::operator=( + const const_dict_iterator& dict_iter) = default; + +const_dict_iterator::~const_dict_iterator() = default; + +const_dict_iterator::reference const_dict_iterator::operator*() const { + return {dict_iter_->first, *dict_iter_->second}; +} + +const_dict_iterator::pointer const_dict_iterator::operator->() const { + return pointer(operator*()); +} + +const_dict_iterator& const_dict_iterator::operator++() { + ++dict_iter_; + return *this; +} + +const_dict_iterator const_dict_iterator::operator++(int) { + const_dict_iterator tmp(*this); + ++dict_iter_; + return tmp; +} + +const_dict_iterator& const_dict_iterator::operator--() { + --dict_iter_; + return *this; +} + +const_dict_iterator const_dict_iterator::operator--(int) { + const_dict_iterator tmp(*this); + --dict_iter_; + return tmp; +} + +bool operator==(const const_dict_iterator& lhs, + const const_dict_iterator& rhs) { + return lhs.dict_iter_ == rhs.dict_iter_; +} + +bool operator!=(const const_dict_iterator& lhs, + const const_dict_iterator& rhs) { + return !(lhs == rhs); +} + +// ---------------------------------------------------------------------------- +// dict_iterator_proxy. + +dict_iterator_proxy::dict_iterator_proxy(DictStorage* storage) + : storage_(storage) {} + +dict_iterator_proxy::iterator dict_iterator_proxy::begin() { + return iterator(storage_->begin()); +} + +dict_iterator_proxy::const_iterator dict_iterator_proxy::begin() const { + return const_iterator(storage_->begin()); +} + +dict_iterator_proxy::iterator dict_iterator_proxy::end() { + return iterator(storage_->end()); +} + +dict_iterator_proxy::const_iterator dict_iterator_proxy::end() const { + return const_iterator(storage_->end()); +} + +dict_iterator_proxy::reverse_iterator dict_iterator_proxy::rbegin() { + return reverse_iterator(end()); +} + +dict_iterator_proxy::const_reverse_iterator dict_iterator_proxy::rbegin() + const { + return const_reverse_iterator(end()); +} + +dict_iterator_proxy::reverse_iterator dict_iterator_proxy::rend() { + return reverse_iterator(begin()); +} + +dict_iterator_proxy::const_reverse_iterator dict_iterator_proxy::rend() const { + return const_reverse_iterator(begin()); +} + +dict_iterator_proxy::const_iterator dict_iterator_proxy::cbegin() const { + return const_iterator(begin()); +} + +dict_iterator_proxy::const_iterator dict_iterator_proxy::cend() const { + return const_iterator(end()); +} + +dict_iterator_proxy::const_reverse_iterator dict_iterator_proxy::crbegin() + const { + return const_reverse_iterator(rbegin()); +} + +dict_iterator_proxy::const_reverse_iterator dict_iterator_proxy::crend() const { + return const_reverse_iterator(rend()); +} + +// ---------------------------------------------------------------------------- +// const_dict_iterator_proxy. + +const_dict_iterator_proxy::const_dict_iterator_proxy(const DictStorage* storage) + : storage_(storage) {} + +const_dict_iterator_proxy::const_iterator const_dict_iterator_proxy::begin() + const { + return const_iterator(storage_->begin()); +} + +const_dict_iterator_proxy::const_iterator const_dict_iterator_proxy::end() + const { + return const_iterator(storage_->end()); +} + +const_dict_iterator_proxy::const_reverse_iterator +const_dict_iterator_proxy::rbegin() const { + return const_reverse_iterator(end()); +} + +const_dict_iterator_proxy::const_reverse_iterator +const_dict_iterator_proxy::rend() const { + return const_reverse_iterator(begin()); +} + +const_dict_iterator_proxy::const_iterator const_dict_iterator_proxy::cbegin() + const { + return const_iterator(begin()); +} + +const_dict_iterator_proxy::const_iterator const_dict_iterator_proxy::cend() + const { + return const_iterator(end()); +} + +const_dict_iterator_proxy::const_reverse_iterator +const_dict_iterator_proxy::crbegin() const { + return const_reverse_iterator(rbegin()); +} + +const_dict_iterator_proxy::const_reverse_iterator +const_dict_iterator_proxy::crend() const { + return const_reverse_iterator(rend()); +} + +} // namespace detail + +} // namespace base
diff --git a/base/value_iterators.h b/base/value_iterators.h new file mode 100644 index 0000000..2e05127b --- /dev/null +++ b/base/value_iterators.h
@@ -0,0 +1,194 @@ +// Copyright 2017 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. + +#ifndef BASE_VALUE_ITERATORS_H_ +#define BASE_VALUE_ITERATORS_H_ + +#include <memory> +#include <string> +#include <utility> + +#include "base/base_export.h" +#include "base/containers/flat_map.h" +#include "base/macros.h" + +namespace base { + +class Value; + +namespace detail { + +using DictStorage = base::flat_map<std::string, std::unique_ptr<Value>>; + +// This iterator closely resembles DictStorage::iterator, with one +// important exception. It abstracts the underlying unique_ptr away, meaning its +// value_type is std::pair<const std::string, Value>. It's reference type is a +// std::pair<const std::string&, Value&>, so that callers have read-write +// access without incurring a copy. +class BASE_EXPORT dict_iterator { + public: + using difference_type = DictStorage::iterator::difference_type; + using value_type = std::pair<const std::string, Value>; + using reference = std::pair<const std::string&, Value&>; + using iterator_category = std::bidirectional_iterator_tag; + + class pointer { + public: + explicit pointer(const reference& ref); + pointer(const pointer& ptr); + pointer& operator=(const pointer& ptr) = delete; + + reference* operator->() { return &ref_; } + + private: + reference ref_; + }; + + explicit dict_iterator(DictStorage::iterator dict_iter); + dict_iterator(const dict_iterator& dict_iter); + dict_iterator& operator=(const dict_iterator& dict_iter); + ~dict_iterator(); + + reference operator*(); + pointer operator->(); + + dict_iterator& operator++(); + dict_iterator operator++(int); + dict_iterator& operator--(); + dict_iterator operator--(int); + + BASE_EXPORT friend bool operator==(const dict_iterator& lhs, + const dict_iterator& rhs); + BASE_EXPORT friend bool operator!=(const dict_iterator& lhs, + const dict_iterator& rhs); + + private: + DictStorage::iterator dict_iter_; +}; + +// This iterator closely resembles DictStorage::const_iterator, with one +// important exception. It abstracts the underlying unique_ptr away, meaning its +// value_type is std::pair<const std::string, Value>. It's reference type is a +// std::pair<const std::string&, const Value&>, so that callers have read-only +// access without incurring a copy. +class BASE_EXPORT const_dict_iterator { + public: + using difference_type = DictStorage::const_iterator::difference_type; + using value_type = std::pair<const std::string, Value>; + using reference = std::pair<const std::string&, const Value&>; + using iterator_category = std::bidirectional_iterator_tag; + + class pointer { + public: + explicit pointer(const reference& ref); + pointer(const pointer& ptr); + pointer& operator=(const pointer& ptr) = delete; + + const reference* operator->() const { return &ref_; } + + private: + const reference ref_; + }; + + explicit const_dict_iterator(DictStorage::const_iterator dict_iter); + const_dict_iterator(const const_dict_iterator& dict_iter); + const_dict_iterator& operator=(const const_dict_iterator& dict_iter); + ~const_dict_iterator(); + + reference operator*() const; + pointer operator->() const; + + const_dict_iterator& operator++(); + const_dict_iterator operator++(int); + const_dict_iterator& operator--(); + const_dict_iterator operator--(int); + + BASE_EXPORT friend bool operator==(const const_dict_iterator& lhs, + const const_dict_iterator& rhs); + BASE_EXPORT friend bool operator!=(const const_dict_iterator& lhs, + const const_dict_iterator& rhs); + + private: + DictStorage::const_iterator dict_iter_; +}; + +// This class wraps the various |begin| and |end| methods of the underlying +// DictStorage in dict_iterators and const_dict_iterators. This allows callers +// to use this class for easy iteration over the underlying values, granting +// them either read-only or read-write access, depending on the +// const-qualification. +class BASE_EXPORT dict_iterator_proxy { + public: + using key_type = DictStorage::key_type; + using mapped_type = DictStorage::mapped_type::element_type; + using value_type = std::pair<key_type, mapped_type>; + using key_compare = DictStorage::key_compare; + using size_type = DictStorage::size_type; + using difference_type = DictStorage::difference_type; + + using iterator = dict_iterator; + using const_iterator = const_dict_iterator; + using reverse_iterator = std::reverse_iterator<iterator>; + using const_reverse_iterator = std::reverse_iterator<const_iterator>; + + explicit dict_iterator_proxy(DictStorage* storage); + + iterator begin(); + const_iterator begin() const; + iterator end(); + const_iterator end() const; + + reverse_iterator rbegin(); + const_reverse_iterator rbegin() const; + reverse_iterator rend(); + const_reverse_iterator rend() const; + + const_dict_iterator cbegin() const; + const_dict_iterator cend() const; + const_reverse_iterator crbegin() const; + const_reverse_iterator crend() const; + + private: + DictStorage* storage_; +}; + +// This class wraps the various const |begin| and |end| methods of the +// underlying DictStorage in const_dict_iterators. This allows callers to use +// this class for easy iteration over the underlying values, granting them +// either read-only access. +class BASE_EXPORT const_dict_iterator_proxy { + public: + using key_type = const DictStorage::key_type; + using mapped_type = const DictStorage::mapped_type::element_type; + using value_type = std::pair<key_type, mapped_type>; + using key_compare = DictStorage::key_compare; + using size_type = DictStorage::size_type; + using difference_type = DictStorage::difference_type; + + using iterator = const_dict_iterator; + using const_iterator = const_dict_iterator; + using reverse_iterator = std::reverse_iterator<iterator>; + using const_reverse_iterator = std::reverse_iterator<const_iterator>; + + explicit const_dict_iterator_proxy(const DictStorage* storage); + + const_iterator begin() const; + const_iterator end() const; + + const_reverse_iterator rbegin() const; + const_reverse_iterator rend() const; + + const_iterator cbegin() const; + const_iterator cend() const; + const_reverse_iterator crbegin() const; + const_reverse_iterator crend() const; + + private: + const DictStorage* storage_; +}; +} // namespace detail + +} // namespace base + +#endif // BASE_VALUE_ITERATORS_H_
diff --git a/base/value_iterators_unittest.cc b/base/value_iterators_unittest.cc new file mode 100644 index 0000000..63b4ec45 --- /dev/null +++ b/base/value_iterators_unittest.cc
@@ -0,0 +1,345 @@ +// Copyright 2017 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 "base/value_iterators.h" + +#include <type_traits> + +#include "base/memory/ptr_util.h" +#include "base/values.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace base { + +namespace detail { + +namespace { + +// Implementation of std::equal variant that is missing in C++11. +template <class BinaryPredicate, class InputIterator1, class InputIterator2> +bool are_equal(InputIterator1 first1, + InputIterator1 last1, + InputIterator2 first2, + InputIterator2 last2, + BinaryPredicate pred) { + for (; first1 != last1 && first2 != last2; ++first1, ++first2) { + if (!pred(*first1, *first2)) + return false; + } + return first1 == last1 && first2 == last2; +} + +} // namespace + +TEST(ValueIteratorsTest, SameDictStorage) { + static_assert(std::is_same<Value::DictStorage, DictStorage>::value, + "DictStorage differs between Value and Value Iterators."); +} + +TEST(ValueIteratorsTest, IsAssignable) { + static_assert( + !std::is_assignable<dict_iterator::reference::first_type, std::string>(), + "Can assign strings to dict_iterator"); + + static_assert( + std::is_assignable<dict_iterator::reference::second_type, Value>(), + "Can't assign Values to dict_iterator"); + + static_assert(!std::is_assignable<const_dict_iterator::reference::first_type, + std::string>(), + "Can assign strings to const_dict_iterator"); + + static_assert( + !std::is_assignable<const_dict_iterator::reference::second_type, Value>(), + "Can assign Values to const_dict_iterator"); +} + +TEST(ValueIteratorsTest, DictIteratorOperatorStar) { + DictStorage storage; + storage.emplace("0", MakeUnique<Value>(0)); + + using iterator = dict_iterator; + iterator iter(storage.begin()); + EXPECT_EQ("0", (*iter).first); + EXPECT_EQ(Value(0), (*iter).second); + + (*iter).second = Value(1); + EXPECT_EQ(Value(1), *storage["0"]); +} + +TEST(ValueIteratorsTest, DictIteratorOperatorArrow) { + DictStorage storage; + storage.emplace("0", MakeUnique<Value>(0)); + + using iterator = dict_iterator; + iterator iter(storage.begin()); + EXPECT_EQ("0", iter->first); + EXPECT_EQ(Value(0), iter->second); + + iter->second = Value(1); + EXPECT_EQ(Value(1), *storage["0"]); +} + +TEST(ValueIteratorsTest, DictIteratorPreIncrement) { + DictStorage storage; + storage.emplace("0", MakeUnique<Value>(0)); + storage.emplace("1", MakeUnique<Value>(1)); + + using iterator = dict_iterator; + iterator iter(storage.begin()); + EXPECT_EQ("0", iter->first); + EXPECT_EQ(Value(0), iter->second); + + iterator& iter_ref = ++iter; + EXPECT_EQ(&iter, &iter_ref); + + EXPECT_EQ("1", iter_ref->first); + EXPECT_EQ(Value(1), iter_ref->second); +} + +TEST(ValueIteratorsTest, DictIteratorPostIncrement) { + DictStorage storage; + storage.emplace("0", MakeUnique<Value>(0)); + storage.emplace("1", MakeUnique<Value>(1)); + + using iterator = dict_iterator; + iterator iter(storage.begin()); + iterator iter_old = iter++; + + EXPECT_EQ("0", iter_old->first); + EXPECT_EQ(Value(0), iter_old->second); + + EXPECT_EQ("1", iter->first); + EXPECT_EQ(Value(1), iter->second); +} + +TEST(ValueIteratorsTest, DictIteratorPreDecrement) { + DictStorage storage; + storage.emplace("0", MakeUnique<Value>(0)); + storage.emplace("1", MakeUnique<Value>(1)); + + using iterator = dict_iterator; + iterator iter(++storage.begin()); + EXPECT_EQ("1", iter->first); + EXPECT_EQ(Value(1), iter->second); + + iterator& iter_ref = --iter; + EXPECT_EQ(&iter, &iter_ref); + + EXPECT_EQ("0", iter_ref->first); + EXPECT_EQ(Value(0), iter_ref->second); +} + +TEST(ValueIteratorsTest, DictIteratorPostDecrement) { + DictStorage storage; + storage.emplace("0", MakeUnique<Value>(0)); + storage.emplace("1", MakeUnique<Value>(1)); + + using iterator = dict_iterator; + iterator iter(++storage.begin()); + iterator iter_old = iter--; + + EXPECT_EQ("1", iter_old->first); + EXPECT_EQ(Value(1), iter_old->second); + + EXPECT_EQ("0", iter->first); + EXPECT_EQ(Value(0), iter->second); +} + +TEST(ValueIteratorsTest, DictIteratorOperatorEQ) { + DictStorage storage; + using iterator = dict_iterator; + EXPECT_EQ(iterator(storage.begin()), iterator(storage.begin())); + EXPECT_EQ(iterator(storage.end()), iterator(storage.end())); +} + +TEST(ValueIteratorsTest, DictIteratorOperatorNE) { + DictStorage storage_0; + storage_0.emplace("0", MakeUnique<Value>(0)); + + DictStorage storage_1; + storage_1.emplace("0", MakeUnique<Value>(0)); + + using iterator = dict_iterator; + EXPECT_NE(iterator(storage_0.begin()), iterator(storage_0.end())); + EXPECT_NE(iterator(storage_0.begin()), iterator(storage_1.begin())); + EXPECT_NE(iterator(storage_0.end()), iterator(storage_1.end())); + EXPECT_NE(iterator(storage_1.begin()), iterator(storage_1.end())); +} + +TEST(ValueIteratorsTest, ConstDictIteratorOperatorStar) { + DictStorage storage; + storage.emplace("0", MakeUnique<Value>(0)); + + using iterator = const_dict_iterator; + iterator iter(storage.begin()); + EXPECT_EQ("0", (*iter).first); + EXPECT_EQ(Value(0), (*iter).second); +} + +TEST(ValueIteratorsTest, ConstDictIteratorOperatorArrow) { + DictStorage storage; + storage.emplace("0", MakeUnique<Value>(0)); + + using iterator = const_dict_iterator; + iterator iter(storage.begin()); + EXPECT_EQ("0", iter->first); + EXPECT_EQ(Value(0), iter->second); +} + +TEST(ValueIteratorsTest, ConstDictIteratorPreIncrement) { + DictStorage storage; + storage.emplace("0", MakeUnique<Value>(0)); + storage.emplace("1", MakeUnique<Value>(1)); + + using iterator = const_dict_iterator; + iterator iter(storage.begin()); + EXPECT_EQ("0", iter->first); + EXPECT_EQ(Value(0), iter->second); + + iterator& iter_ref = ++iter; + EXPECT_EQ(&iter, &iter_ref); + + EXPECT_EQ("1", iter_ref->first); + EXPECT_EQ(Value(1), iter_ref->second); +} + +TEST(ValueIteratorsTest, ConstDictIteratorPostIncrement) { + DictStorage storage; + storage.emplace("0", MakeUnique<Value>(0)); + storage.emplace("1", MakeUnique<Value>(1)); + + using iterator = const_dict_iterator; + iterator iter(storage.begin()); + iterator iter_old = iter++; + + EXPECT_EQ("0", iter_old->first); + EXPECT_EQ(Value(0), iter_old->second); + + EXPECT_EQ("1", iter->first); + EXPECT_EQ(Value(1), iter->second); +} + +TEST(ValueIteratorsTest, ConstDictIteratorPreDecrement) { + DictStorage storage; + storage.emplace("0", MakeUnique<Value>(0)); + storage.emplace("1", MakeUnique<Value>(1)); + + using iterator = const_dict_iterator; + iterator iter(++storage.begin()); + EXPECT_EQ("1", iter->first); + EXPECT_EQ(Value(1), iter->second); + + iterator& iter_ref = --iter; + EXPECT_EQ(&iter, &iter_ref); + + EXPECT_EQ("0", iter_ref->first); + EXPECT_EQ(Value(0), iter_ref->second); +} + +TEST(ValueIteratorsTest, ConstDictIteratorPostDecrement) { + DictStorage storage; + storage.emplace("0", MakeUnique<Value>(0)); + storage.emplace("1", MakeUnique<Value>(1)); + + using iterator = const_dict_iterator; + iterator iter(++storage.begin()); + iterator iter_old = iter--; + + EXPECT_EQ("1", iter_old->first); + EXPECT_EQ(Value(1), iter_old->second); + + EXPECT_EQ("0", iter->first); + EXPECT_EQ(Value(0), iter->second); +} + +TEST(ValueIteratorsTest, ConstDictIteratorOperatorEQ) { + DictStorage storage; + using iterator = const_dict_iterator; + EXPECT_EQ(iterator(storage.begin()), iterator(storage.begin())); + EXPECT_EQ(iterator(storage.end()), iterator(storage.end())); +} + +TEST(ValueIteratorsTest, ConstDictIteratorOperatorNE) { + DictStorage storage_0; + storage_0.emplace("0", MakeUnique<Value>(0)); + + DictStorage storage_1; + storage_1.emplace("0", MakeUnique<Value>(0)); + + using iterator = const_dict_iterator; + EXPECT_NE(iterator(storage_0.begin()), iterator(storage_0.end())); + EXPECT_NE(iterator(storage_0.begin()), iterator(storage_1.begin())); + EXPECT_NE(iterator(storage_0.end()), iterator(storage_1.end())); + EXPECT_NE(iterator(storage_1.begin()), iterator(storage_1.end())); +} + +TEST(ValueIteratorsTest, DictIteratorProxy) { + DictStorage storage; + storage.emplace("null", MakeUnique<Value>(Value::Type::NONE)); + storage.emplace("bool", MakeUnique<Value>(Value::Type::BOOLEAN)); + storage.emplace("int", MakeUnique<Value>(Value::Type::INTEGER)); + storage.emplace("double", MakeUnique<Value>(Value::Type::DOUBLE)); + storage.emplace("string", MakeUnique<Value>(Value::Type::STRING)); + storage.emplace("blob", MakeUnique<Value>(Value::Type::BINARY)); + storage.emplace("dict", MakeUnique<Value>(Value::Type::DICTIONARY)); + storage.emplace("list", MakeUnique<Value>(Value::Type::LIST)); + + using iterator_proxy = dict_iterator_proxy; + iterator_proxy proxy(&storage); + + auto equal_to = [](const DictStorage::value_type& lhs, + const iterator_proxy::value_type& rhs) { + return std::tie(lhs.first, *lhs.second) == std::tie(rhs.first, rhs.second); + }; + + EXPECT_TRUE(are_equal(storage.begin(), storage.end(), proxy.begin(), + proxy.end(), equal_to)); + + EXPECT_TRUE(are_equal(storage.rbegin(), storage.rend(), proxy.rbegin(), + proxy.rend(), equal_to)); + + EXPECT_TRUE(are_equal(storage.cbegin(), storage.cend(), proxy.cbegin(), + proxy.cend(), equal_to)); + + EXPECT_TRUE(are_equal(storage.crbegin(), storage.crend(), proxy.crbegin(), + proxy.crend(), equal_to)); +} + +TEST(ValueIteratorsTest, ConstDictIteratorProxy) { + DictStorage storage; + storage.emplace("null", MakeUnique<Value>(Value::Type::NONE)); + storage.emplace("bool", MakeUnique<Value>(Value::Type::BOOLEAN)); + storage.emplace("int", MakeUnique<Value>(Value::Type::INTEGER)); + storage.emplace("double", MakeUnique<Value>(Value::Type::DOUBLE)); + storage.emplace("string", MakeUnique<Value>(Value::Type::STRING)); + storage.emplace("blob", MakeUnique<Value>(Value::Type::BINARY)); + storage.emplace("dict", MakeUnique<Value>(Value::Type::DICTIONARY)); + storage.emplace("list", MakeUnique<Value>(Value::Type::LIST)); + + using iterator_proxy = const_dict_iterator_proxy; + iterator_proxy proxy(&storage); + + auto equal_to = [](const DictStorage::value_type& lhs, + const iterator_proxy::value_type& rhs) { + return std::tie(lhs.first, *lhs.second) == std::tie(rhs.first, rhs.second); + }; + + EXPECT_TRUE(are_equal(storage.begin(), storage.end(), proxy.begin(), + proxy.end(), equal_to)); + + EXPECT_TRUE(are_equal(storage.rbegin(), storage.rend(), proxy.rbegin(), + proxy.rend(), equal_to)); + + EXPECT_TRUE(are_equal(storage.cbegin(), storage.cend(), proxy.cbegin(), + proxy.cend(), equal_to)); + + EXPECT_TRUE(are_equal(storage.crbegin(), storage.crend(), proxy.crbegin(), + proxy.crend(), equal_to)); +} + +} // namespace detail + +} // namespace base
diff --git a/base/values.cc b/base/values.cc index 8e2bc88..37a4e94 100644 --- a/base/values.cc +++ b/base/values.cc
@@ -244,6 +244,99 @@ return *list_; } +Value::dict_iterator Value::FindKey(const char* key) { + return FindKey(std::string(key)); +} + +Value::dict_iterator Value::FindKey(const std::string& key) { + CHECK(is_dict()); + return dict_iterator(dict_->find(key)); +} + +Value::const_dict_iterator Value::FindKey(const char* key) const { + return FindKey(std::string(key)); +} + +Value::const_dict_iterator Value::FindKey(const std::string& key) const { + CHECK(is_dict()); + return const_dict_iterator(dict_->find(key)); +} + +Value::dict_iterator Value::FindKeyOfType(const char* key, Type type) { + return FindKeyOfType(std::string(key), type); +} + +Value::dict_iterator Value::FindKeyOfType(const std::string& key, Type type) { + CHECK(is_dict()); + auto iter = dict_->find(key); + return dict_iterator((iter != dict_->end() && iter->second->IsType(type)) + ? iter + : dict_->end()); +} + +Value::const_dict_iterator Value::FindKeyOfType(const char* key, + Type type) const { + return FindKeyOfType(std::string(key), type); +} + +Value::const_dict_iterator Value::FindKeyOfType(const std::string& key, + Type type) const { + CHECK(is_dict()); + auto iter = dict_->find(key); + return const_dict_iterator( + (iter != dict_->end() && iter->second->IsType(type)) ? iter + : dict_->end()); +} + +Value::dict_iterator Value::SetKey(const char* key, Value value) { + return SetKey(std::string(key), std::move(value)); +} + +Value::dict_iterator Value::SetKey(const std::string& key, Value value) { + CHECK(is_dict()); + auto iter = dict_->find(key); + if (iter != dict_->end()) { + *iter->second = std::move(value); + return dict_iterator(iter); + } + + return dict_iterator( + dict_->emplace(key, MakeUnique<Value>(std::move(value))).first); +} + +Value::dict_iterator Value::SetKey(std::string&& key, Value value) { + CHECK(is_dict()); + auto iter = dict_->find(key); + if (iter != dict_->end()) { + *iter->second = std::move(value); + return dict_iterator(iter); + } + + return dict_iterator( + dict_->emplace(std::move(key), MakeUnique<Value>(std::move(value))) + .first); +} + +Value::dict_iterator Value::DictEnd() { + CHECK(is_dict()); + return dict_iterator(dict_->end()); +} + +Value::const_dict_iterator Value::DictEnd() const { + CHECK(is_dict()); + return const_dict_iterator(dict_->end()); +} + +Value::dict_iterator_proxy Value::DictItems() { + CHECK(is_dict()); + return dict_iterator_proxy(&*dict_); +} + +Value::const_dict_iterator_proxy Value::DictItems() const { + CHECK(is_dict()); + return const_dict_iterator_proxy(&*dict_); +} + bool Value::GetAsBoolean(bool* out_value) const { if (out_value && is_bool()) { *out_value = bool_value_;
diff --git a/base/values.h b/base/values.h index 6e75540..2dfae42 100644 --- a/base/values.h +++ b/base/values.h
@@ -34,6 +34,7 @@ #include "base/memory/manual_constructor.h" #include "base/strings/string16.h" #include "base/strings/string_piece.h" +#include "base/value_iterators.h" namespace base { @@ -137,6 +138,59 @@ ListStorage& GetList(); const ListStorage& GetList() const; + using dict_iterator = detail::dict_iterator; + using const_dict_iterator = detail::const_dict_iterator; + using dict_iterator_proxy = detail::dict_iterator_proxy; + using const_dict_iterator_proxy = detail::const_dict_iterator_proxy; + + // TODO(crbug.com/646113): Replace the following const char* and const + // std::string& overloads with a single base::StringPiece overload when + // base::less becomes available. + + // |FindKey| looks up |key| in the underlying dictionary. If found, it returns + // an iterator to the element. Otherwise the end iterator of the dictionary is + // returned. Callers are expected to compare the returned iterator against + // |DictEnd()| in order to determine whether |key| was present. + // Note: This fatally asserts if type() is not Type::DICTIONARY. + dict_iterator FindKey(const char* key); + dict_iterator FindKey(const std::string& key); + const_dict_iterator FindKey(const char* key) const; + const_dict_iterator FindKey(const std::string& key) const; + + // |FindKeyOfType| is similar to |FindKey|, but it also requires the found + // value to have type |type|. If no type is found, or the found value is of a + // different type the end iterator of the dictionary is returned. + // Callers are expected to compare the returned iterator against |DictEnd()| + // in order to determine whether |key| was present and of the correct |type|. + // Note: This fatally asserts if type() is not Type::DICTIONARY. + dict_iterator FindKeyOfType(const char* key, Type type); + dict_iterator FindKeyOfType(const std::string& key, Type type); + const_dict_iterator FindKeyOfType(const char* key, Type type) const; + const_dict_iterator FindKeyOfType(const std::string& key, Type type) const; + + // |SetKey| looks up |key| in the underlying dictionary and sets the mapped + // value to |value|. If |key| could not be found, a new element is inserted. + // An iterator to the modifed item is returned. + // Note: This fatally asserts if type() is not Type::DICTIONARY. + dict_iterator SetKey(const char* key, Value value); + dict_iterator SetKey(const std::string& key, Value value); + dict_iterator SetKey(std::string&& key, Value value); + + // |DictEnd| returns the end iterator of the underlying dictionary. It is + // intended to be used with |FindKey| in order to determine whether a given + // key is present in the dictionary. + // Note: This fatally asserts if type() is not Type::DICTIONARY. + dict_iterator DictEnd(); + const_dict_iterator DictEnd() const; + + // |DictItems| returns a proxy object that exposes iterators to the underlying + // dictionary. These are intended for iteration over all items in the + // dictionary and are compatible with for-each loops and standard library + // algorithms. + // Note: This fatally asserts if type() is not Type::DICTIONARY. + dict_iterator_proxy DictItems(); + const_dict_iterator_proxy DictItems() const; + // These methods allow the convenient retrieval of the contents of the Value. // If the current object can be converted into the given type, the value is // returned through the |out_value| parameter and true is returned;
diff --git a/base/values_unittest.cc b/base/values_unittest.cc index 38200868..3e680c9 100644 --- a/base/values_unittest.cc +++ b/base/values_unittest.cc
@@ -6,6 +6,7 @@ #include <stddef.h> +#include <functional> #include <limits> #include <memory> #include <string> @@ -13,9 +14,11 @@ #include <utility> #include <vector> +#include "base/containers/adapters.h" #include "base/memory/ptr_util.h" #include "base/strings/string16.h" #include "base/strings/utf_string_conversions.h" +#include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" namespace base { @@ -222,33 +225,16 @@ } TEST(ValuesTest, CopyDictionary) { - // TODO(crbug.com/646113): Clean this up once DictionaryValue switched to - // value semantics. - int copy; - DictionaryValue value; - value.SetInteger("Int", 123); + Value::DictStorage storage; + storage.emplace("Int", MakeUnique<Value>(123)); + Value value(std::move(storage)); - DictionaryValue copied_value(value); - copied_value.GetInteger("Int", ©); + Value copied_value(value); + EXPECT_EQ(value, copied_value); - EXPECT_EQ(value.type(), copied_value.type()); - EXPECT_EQ(123, copy); - - copied_value.Clear(); - copied_value = value; - - copied_value.GetInteger("Int", ©); - - EXPECT_EQ(value.type(), copied_value.type()); - EXPECT_EQ(123, copy); - - auto blank = MakeUnique<Value>(); - - *blank = value; - EXPECT_EQ(Value::Type::DICTIONARY, blank->type()); - - static_cast<DictionaryValue*>(blank.get())->GetInteger("Int", ©); - EXPECT_EQ(123, copy); + Value blank; + blank = value; + EXPECT_EQ(value, blank); } TEST(ValuesTest, CopyList) { @@ -338,23 +324,24 @@ EXPECT_EQ(buffer, blank.GetBlob()); } -TEST(ValuesTest, MoveDictionary) { - // TODO(crbug.com/646113): Clean this up once DictionaryValue switched to - // value semantics. - int move; - DictionaryValue value; - value.SetInteger("Int", 123); +TEST(ValuesTest, MoveConstructDictionary) { + Value::DictStorage storage; + storage.emplace("Int", MakeUnique<Value>(123)); - DictionaryValue moved_value(std::move(value)); - moved_value.GetInteger("Int", &move); - + Value value(std::move(storage)); + Value moved_value(std::move(value)); EXPECT_EQ(Value::Type::DICTIONARY, moved_value.type()); - EXPECT_EQ(123, move); + EXPECT_EQ(123, moved_value.FindKey("Int")->second.GetInt()); +} + +TEST(ValuesTest, MoveAssignDictionary) { + Value::DictStorage storage; + storage.emplace("Int", MakeUnique<Value>(123)); Value blank; - - blank = DictionaryValue(); + blank = Value(std::move(storage)); EXPECT_EQ(Value::Type::DICTIONARY, blank.type()); + EXPECT_EQ(123, blank.FindKey("Int")->second.GetInt()); } TEST(ValuesTest, MoveList) { @@ -371,6 +358,254 @@ EXPECT_EQ(123, blank.GetList().back().GetInt()); } +TEST(ValuesTest, FindKey) { + Value::DictStorage storage; + storage.emplace("foo", MakeUnique<Value>("bar")); + Value dict(std::move(storage)); + EXPECT_NE(dict.FindKey("foo"), dict.DictEnd()); + EXPECT_EQ(dict.FindKey("baz"), dict.DictEnd()); +} + +TEST(ValuesTest, FindKeyChangeValue) { + Value::DictStorage storage; + storage.emplace("foo", MakeUnique<Value>("bar")); + Value dict(std::move(storage)); + auto it = dict.FindKey("foo"); + EXPECT_NE(it, dict.DictEnd()); + const std::string& key = it->first; + EXPECT_EQ("foo", key); + EXPECT_EQ("bar", it->second.GetString()); + + it->second = Value(123); + EXPECT_EQ(123, dict.FindKey("foo")->second.GetInt()); +} + +TEST(ValuesTest, FindKeyConst) { + Value::DictStorage storage; + storage.emplace("foo", MakeUnique<Value>("bar")); + const Value dict(std::move(storage)); + EXPECT_NE(dict.FindKey("foo"), dict.DictEnd()); + EXPECT_EQ(dict.FindKey("baz"), dict.DictEnd()); +} + +TEST(ValuesTest, FindKeyOfType) { + Value::DictStorage storage; + storage.emplace("null", MakeUnique<Value>(Value::Type::NONE)); + storage.emplace("bool", MakeUnique<Value>(Value::Type::BOOLEAN)); + storage.emplace("int", MakeUnique<Value>(Value::Type::INTEGER)); + storage.emplace("double", MakeUnique<Value>(Value::Type::DOUBLE)); + storage.emplace("string", MakeUnique<Value>(Value::Type::STRING)); + storage.emplace("blob", MakeUnique<Value>(Value::Type::BINARY)); + storage.emplace("list", MakeUnique<Value>(Value::Type::LIST)); + storage.emplace("dict", MakeUnique<Value>(Value::Type::DICTIONARY)); + + Value dict(std::move(storage)); + EXPECT_NE(dict.FindKeyOfType("null", Value::Type::NONE), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("null", Value::Type::BOOLEAN), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("null", Value::Type::INTEGER), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("null", Value::Type::DOUBLE), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("null", Value::Type::STRING), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("null", Value::Type::BINARY), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("null", Value::Type::LIST), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("null", Value::Type::DICTIONARY), + dict.DictEnd()); + + EXPECT_EQ(dict.FindKeyOfType("bool", Value::Type::NONE), dict.DictEnd()); + EXPECT_NE(dict.FindKeyOfType("bool", Value::Type::BOOLEAN), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("bool", Value::Type::INTEGER), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("bool", Value::Type::DOUBLE), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("bool", Value::Type::STRING), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("bool", Value::Type::BINARY), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("bool", Value::Type::LIST), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("bool", Value::Type::DICTIONARY), + dict.DictEnd()); + + EXPECT_EQ(dict.FindKeyOfType("int", Value::Type::NONE), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("int", Value::Type::BOOLEAN), dict.DictEnd()); + EXPECT_NE(dict.FindKeyOfType("int", Value::Type::INTEGER), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("int", Value::Type::DOUBLE), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("int", Value::Type::STRING), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("int", Value::Type::BINARY), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("int", Value::Type::LIST), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("int", Value::Type::DICTIONARY), dict.DictEnd()); + + EXPECT_EQ(dict.FindKeyOfType("double", Value::Type::NONE), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("double", Value::Type::BOOLEAN), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("double", Value::Type::INTEGER), dict.DictEnd()); + EXPECT_NE(dict.FindKeyOfType("double", Value::Type::DOUBLE), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("double", Value::Type::STRING), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("double", Value::Type::BINARY), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("double", Value::Type::LIST), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("double", Value::Type::DICTIONARY), + dict.DictEnd()); + + EXPECT_EQ(dict.FindKeyOfType("string", Value::Type::NONE), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("string", Value::Type::BOOLEAN), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("string", Value::Type::INTEGER), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("string", Value::Type::DOUBLE), dict.DictEnd()); + EXPECT_NE(dict.FindKeyOfType("string", Value::Type::STRING), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("string", Value::Type::BINARY), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("string", Value::Type::LIST), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("string", Value::Type::DICTIONARY), + dict.DictEnd()); + + EXPECT_EQ(dict.FindKeyOfType("blob", Value::Type::NONE), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("blob", Value::Type::BOOLEAN), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("blob", Value::Type::INTEGER), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("blob", Value::Type::DOUBLE), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("blob", Value::Type::STRING), dict.DictEnd()); + EXPECT_NE(dict.FindKeyOfType("blob", Value::Type::BINARY), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("blob", Value::Type::LIST), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("blob", Value::Type::DICTIONARY), + dict.DictEnd()); + + EXPECT_EQ(dict.FindKeyOfType("list", Value::Type::NONE), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("list", Value::Type::BOOLEAN), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("list", Value::Type::INTEGER), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("list", Value::Type::DOUBLE), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("list", Value::Type::STRING), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("list", Value::Type::BINARY), dict.DictEnd()); + EXPECT_NE(dict.FindKeyOfType("list", Value::Type::LIST), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("list", Value::Type::DICTIONARY), + dict.DictEnd()); + + EXPECT_EQ(dict.FindKeyOfType("dict", Value::Type::NONE), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("dict", Value::Type::BOOLEAN), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("dict", Value::Type::INTEGER), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("dict", Value::Type::DOUBLE), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("dict", Value::Type::STRING), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("dict", Value::Type::BINARY), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("dict", Value::Type::LIST), dict.DictEnd()); + EXPECT_NE(dict.FindKeyOfType("dict", Value::Type::DICTIONARY), + dict.DictEnd()); +} + +TEST(ValuesTest, FindKeyOfTypeConst) { + Value::DictStorage storage; + storage.emplace("null", MakeUnique<Value>(Value::Type::NONE)); + storage.emplace("bool", MakeUnique<Value>(Value::Type::BOOLEAN)); + storage.emplace("int", MakeUnique<Value>(Value::Type::INTEGER)); + storage.emplace("double", MakeUnique<Value>(Value::Type::DOUBLE)); + storage.emplace("string", MakeUnique<Value>(Value::Type::STRING)); + storage.emplace("blob", MakeUnique<Value>(Value::Type::BINARY)); + storage.emplace("list", MakeUnique<Value>(Value::Type::LIST)); + storage.emplace("dict", MakeUnique<Value>(Value::Type::DICTIONARY)); + + const Value dict(std::move(storage)); + EXPECT_NE(dict.FindKeyOfType("null", Value::Type::NONE), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("null", Value::Type::BOOLEAN), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("null", Value::Type::INTEGER), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("null", Value::Type::DOUBLE), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("null", Value::Type::STRING), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("null", Value::Type::BINARY), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("null", Value::Type::LIST), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("null", Value::Type::DICTIONARY), + dict.DictEnd()); + + EXPECT_EQ(dict.FindKeyOfType("bool", Value::Type::NONE), dict.DictEnd()); + EXPECT_NE(dict.FindKeyOfType("bool", Value::Type::BOOLEAN), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("bool", Value::Type::INTEGER), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("bool", Value::Type::DOUBLE), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("bool", Value::Type::STRING), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("bool", Value::Type::BINARY), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("bool", Value::Type::LIST), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("bool", Value::Type::DICTIONARY), + dict.DictEnd()); + + EXPECT_EQ(dict.FindKeyOfType("int", Value::Type::NONE), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("int", Value::Type::BOOLEAN), dict.DictEnd()); + EXPECT_NE(dict.FindKeyOfType("int", Value::Type::INTEGER), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("int", Value::Type::DOUBLE), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("int", Value::Type::STRING), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("int", Value::Type::BINARY), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("int", Value::Type::LIST), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("int", Value::Type::DICTIONARY), dict.DictEnd()); + + EXPECT_EQ(dict.FindKeyOfType("double", Value::Type::NONE), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("double", Value::Type::BOOLEAN), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("double", Value::Type::INTEGER), dict.DictEnd()); + EXPECT_NE(dict.FindKeyOfType("double", Value::Type::DOUBLE), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("double", Value::Type::STRING), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("double", Value::Type::BINARY), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("double", Value::Type::LIST), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("double", Value::Type::DICTIONARY), + dict.DictEnd()); + + EXPECT_EQ(dict.FindKeyOfType("string", Value::Type::NONE), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("string", Value::Type::BOOLEAN), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("string", Value::Type::INTEGER), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("string", Value::Type::DOUBLE), dict.DictEnd()); + EXPECT_NE(dict.FindKeyOfType("string", Value::Type::STRING), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("string", Value::Type::BINARY), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("string", Value::Type::LIST), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("string", Value::Type::DICTIONARY), + dict.DictEnd()); + + EXPECT_EQ(dict.FindKeyOfType("blob", Value::Type::NONE), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("blob", Value::Type::BOOLEAN), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("blob", Value::Type::INTEGER), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("blob", Value::Type::DOUBLE), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("blob", Value::Type::STRING), dict.DictEnd()); + EXPECT_NE(dict.FindKeyOfType("blob", Value::Type::BINARY), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("blob", Value::Type::LIST), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("blob", Value::Type::DICTIONARY), + dict.DictEnd()); + + EXPECT_EQ(dict.FindKeyOfType("list", Value::Type::NONE), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("list", Value::Type::BOOLEAN), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("list", Value::Type::INTEGER), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("list", Value::Type::DOUBLE), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("list", Value::Type::STRING), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("list", Value::Type::BINARY), dict.DictEnd()); + EXPECT_NE(dict.FindKeyOfType("list", Value::Type::LIST), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("list", Value::Type::DICTIONARY), + dict.DictEnd()); + + EXPECT_EQ(dict.FindKeyOfType("dict", Value::Type::NONE), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("dict", Value::Type::BOOLEAN), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("dict", Value::Type::INTEGER), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("dict", Value::Type::DOUBLE), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("dict", Value::Type::STRING), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("dict", Value::Type::BINARY), dict.DictEnd()); + EXPECT_EQ(dict.FindKeyOfType("dict", Value::Type::LIST), dict.DictEnd()); + EXPECT_NE(dict.FindKeyOfType("dict", Value::Type::DICTIONARY), + dict.DictEnd()); +} + +TEST(ValuesTest, SetKey) { + Value::DictStorage storage; + storage.emplace("null", MakeUnique<Value>(Value::Type::NONE)); + storage.emplace("bool", MakeUnique<Value>(Value::Type::BOOLEAN)); + storage.emplace("int", MakeUnique<Value>(Value::Type::INTEGER)); + storage.emplace("double", MakeUnique<Value>(Value::Type::DOUBLE)); + storage.emplace("string", MakeUnique<Value>(Value::Type::STRING)); + storage.emplace("blob", MakeUnique<Value>(Value::Type::BINARY)); + storage.emplace("list", MakeUnique<Value>(Value::Type::LIST)); + storage.emplace("dict", MakeUnique<Value>(Value::Type::DICTIONARY)); + + Value dict(Value::Type::DICTIONARY); + dict.SetKey("null", Value(Value::Type::NONE)); + dict.SetKey("bool", Value(Value::Type::BOOLEAN)); + dict.SetKey("int", Value(Value::Type::INTEGER)); + dict.SetKey("double", Value(Value::Type::DOUBLE)); + dict.SetKey("string", Value(Value::Type::STRING)); + dict.SetKey("blob", Value(Value::Type::BINARY)); + dict.SetKey("list", Value(Value::Type::LIST)); + dict.SetKey("dict", Value(Value::Type::DICTIONARY)); + + EXPECT_EQ(Value(std::move(storage)), dict); +} + +TEST(ValuesTest, DictEnd) { + Value dict(Value::Type::DICTIONARY); + EXPECT_EQ(dict.DictItems().end(), dict.DictEnd()); +} + +TEST(ValuesTest, DictEndConst) { + const Value dict(Value::Type::DICTIONARY); + EXPECT_EQ(dict.DictItems().end(), dict.DictEnd()); +} + TEST(ValuesTest, Basic) { // Test basic dictionary getting/setting DictionaryValue settings;
diff --git a/build/config/merge_for_jumbo.py b/build/config/merge_for_jumbo.py index b3146ba..ae435d0 100755 --- a/build/config/merge_for_jumbo.py +++ b/build/config/merge_for_jumbo.py
@@ -83,7 +83,8 @@ header_files = set([x for x in all_inputs if x.endswith(".h")]) assert set(args.outputs) == written_output_set, "Did not fill all outputs" files_not_included = set(all_inputs) - written_input_set - header_files - assert not files_not_included, "Did not include files: " + files_not_included + assert not files_not_included, ("Jumbo build left out files: %s" % + files_not_included) if args.verbose: print("Generated %s (%d files) based on %s" % ( str(args.outputs), len(written_input_set), args.file_list))
diff --git a/chrome/app/chromeos_strings.grdp b/chrome/app/chromeos_strings.grdp index f0552d6..67583ff 100644 --- a/chrome/app/chromeos_strings.grdp +++ b/chrome/app/chromeos_strings.grdp
@@ -130,6 +130,9 @@ <message name="IDS_FILE_BROWSER_MEDIA_VIEW_AUDIO_ROOT_LABEL" desc="A label for the 'Audio' root of media views."> Audio </message> + <message name="IDS_FILE_BROWSER_RECENT_ROOT_LABEL" desc="A label for the 'Recent' root."> + Recent + </message> <message name="IDS_FILE_BROWSER_DOWNLOADS_DIRECTORY_WARNING" desc="Warning displayed to user when viewing downloads folder."> <ph name="BEGIN_BOLD"><strong></ph>Caution:<ph name="END_BOLD"></strong></ph> These files are temporary and may be automatically deleted to free up disk space. <ph name="BEGIN_LINK"><a href="javascript://"></ph>Learn More<ph name="END_LINK"></a><ex></a></ex></ph> </message>
diff --git a/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc b/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc index 060bf0c..bb36ce9c 100644 --- a/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc +++ b/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.cc
@@ -744,7 +744,7 @@ for (const content_settings::ContentSettingsInfo* info : *registry) { map->ClearSettingsForOneTypeWithPredicate( info->website_settings_info()->type(), delete_begin_, - base::Bind(&WebsiteSettingsFilterAdapter, filter)); + HostContentSettingsMap::PatternSourcePredicate()); } }
diff --git a/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h b/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h index 5691617..37cf3b0 100644 --- a/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h +++ b/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate.h
@@ -78,17 +78,16 @@ // "Site data" includes storage backend accessible to websites and some // additional metadata kept by the browser (e.g. site usage data). - DATA_TYPE_SITE_DATA = - content::BrowsingDataRemover::DATA_TYPE_COOKIES | - content::BrowsingDataRemover::DATA_TYPE_CHANNEL_IDS | - content::BrowsingDataRemover::DATA_TYPE_DOM_STORAGE | - DATA_TYPE_PLUGIN_DATA | + DATA_TYPE_SITE_DATA = content::BrowsingDataRemover::DATA_TYPE_COOKIES | + content::BrowsingDataRemover::DATA_TYPE_CHANNEL_IDS | + content::BrowsingDataRemover::DATA_TYPE_DOM_STORAGE | + DATA_TYPE_PLUGIN_DATA | #if defined(OS_ANDROID) - DATA_TYPE_WEB_APP_DATA | + DATA_TYPE_WEB_APP_DATA | #endif - DATA_TYPE_SITE_USAGE_DATA | - DATA_TYPE_DURABLE_PERMISSION | - DATA_TYPE_EXTERNAL_PROTOCOL_DATA, + DATA_TYPE_SITE_USAGE_DATA | + DATA_TYPE_DURABLE_PERMISSION | + DATA_TYPE_EXTERNAL_PROTOCOL_DATA, // Datatypes protected by Important Sites. IMPORTANT_SITES_DATA_TYPES = @@ -98,17 +97,16 @@ // whichever makes sense. FILTERABLE_DATA_TYPES = DATA_TYPE_SITE_DATA | content::BrowsingDataRemover::DATA_TYPE_CACHE | - content::BrowsingDataRemover::DATA_TYPE_DOWNLOADS | - DATA_TYPE_CONTENT_SETTINGS, + content::BrowsingDataRemover::DATA_TYPE_DOWNLOADS, // Includes all the available remove options. Meant to be used by clients // that wish to wipe as much data as possible from a Profile, to make it // look like a new Profile. - ALL_DATA_TYPES = DATA_TYPE_SITE_DATA | + ALL_DATA_TYPES = DATA_TYPE_SITE_DATA | // content::BrowsingDataRemover::DATA_TYPE_CACHE | content::BrowsingDataRemover::DATA_TYPE_DOWNLOADS | - DATA_TYPE_FORM_DATA | - DATA_TYPE_HISTORY | + DATA_TYPE_FORM_DATA | // + DATA_TYPE_HISTORY | // DATA_TYPE_PASSWORDS | content::BrowsingDataRemover::DATA_TYPE_MEDIA_LICENSES | DATA_TYPE_CONTENT_SETTINGS,
diff --git a/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc b/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc index 73473ec..4f9e43d 100644 --- a/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc +++ b/chrome/browser/browsing_data/chrome_browsing_data_remover_delegate_unittest.cc
@@ -1681,67 +1681,24 @@ CONTENT_SETTINGS_TYPE_GEOLOCATION, std::string(), CONTENT_SETTING_ALLOW); map->SetContentSettingDefaultScope(kOrigin2, kOrigin2, - CONTENT_SETTINGS_TYPE_GEOLOCATION, - std::string(), CONTENT_SETTING_ALLOW); - map->SetContentSettingDefaultScope(kOrigin2, kOrigin2, CONTENT_SETTINGS_TYPE_NOTIFICATIONS, std::string(), CONTENT_SETTING_ALLOW); - map->SetContentSettingDefaultScope(kOrigin3, kOrigin3, - CONTENT_SETTINGS_TYPE_NOTIFICATIONS, - std::string(), CONTENT_SETTING_ALLOW); - - // Clear all except for origin1 and origin3. - std::unique_ptr<BrowsingDataFilterBuilder> filter( - BrowsingDataFilterBuilder::Create(BrowsingDataFilterBuilder::BLACKLIST)); - filter->AddRegisterableDomain(kTestRegisterableDomain1); - filter->AddRegisterableDomain(kTestRegisterableDomain3); - BlockUntilOriginDataRemoved( + map->SetContentSettingDefaultScope(kOrigin3, GURL(), + CONTENT_SETTINGS_TYPE_COOKIES, + std::string(), CONTENT_SETTING_BLOCK); + ContentSettingsPattern pattern = + ContentSettingsPattern::FromString("[*.]example.com"); + map->SetContentSettingCustomScope(pattern, ContentSettingsPattern::Wildcard(), + CONTENT_SETTINGS_TYPE_COOKIES, + std::string(), CONTENT_SETTING_BLOCK); + BlockUntilBrowsingDataRemoved( base::Time(), base::Time::Max(), - ChromeBrowsingDataRemoverDelegate::DATA_TYPE_CONTENT_SETTINGS, - std::move(filter)); + ChromeBrowsingDataRemoverDelegate::DATA_TYPE_CONTENT_SETTINGS, false); - EXPECT_EQ(ChromeBrowsingDataRemoverDelegate::DATA_TYPE_CONTENT_SETTINGS, - GetRemovalMask()); - EXPECT_EQ(content::BrowsingDataRemover::ORIGIN_TYPE_UNPROTECTED_WEB, - GetOriginTypeMask()); - - // Verify we still have the allow setting for origin1. + // Everything except the default settings should be deleted now. ContentSettingsForOneType host_settings; map->GetSettingsForOneType(CONTENT_SETTINGS_TYPE_GEOLOCATION, std::string(), &host_settings); - ASSERT_EQ(2u, host_settings.size()); - EXPECT_EQ(ContentSettingsPattern::FromURLNoWildcard(kOrigin1), - host_settings[0].primary_pattern) - << host_settings[0].primary_pattern.ToString(); - EXPECT_EQ(CONTENT_SETTING_ALLOW, host_settings[0].GetContentSetting()); - // And the wildcard. - EXPECT_EQ(ContentSettingsPattern::Wildcard(), - host_settings[1].primary_pattern) - << host_settings[1].primary_pattern.ToString(); - EXPECT_EQ(CONTENT_SETTING_ASK, host_settings[1].GetContentSetting()); - - // There should also only be one setting for origin3 - map->GetSettingsForOneType(CONTENT_SETTINGS_TYPE_NOTIFICATIONS, std::string(), - &host_settings); - ASSERT_EQ(2u, host_settings.size()); - EXPECT_EQ(ContentSettingsPattern::FromURLNoWildcard(kOrigin3), - host_settings[0].primary_pattern) - << host_settings[0].primary_pattern.ToString(); - EXPECT_EQ(CONTENT_SETTING_ALLOW, host_settings[0].GetContentSetting()); - // And the wildcard. - EXPECT_EQ(ContentSettingsPattern::Wildcard(), - host_settings[1].primary_pattern) - << host_settings[1].primary_pattern.ToString(); - EXPECT_EQ(CONTENT_SETTING_ASK, host_settings[1].GetContentSetting()); - - BlockUntilOriginDataRemoved( - base::Time(), base::Time::Max(), - ChromeBrowsingDataRemoverDelegate::DATA_TYPE_CONTENT_SETTINGS, - BrowsingDataFilterBuilder::Create(BrowsingDataFilterBuilder::BLACKLIST)); - - // Everything except the wildcard should be deleted now. - map->GetSettingsForOneType(CONTENT_SETTINGS_TYPE_GEOLOCATION, std::string(), - &host_settings); ASSERT_EQ(1u, host_settings.size()); EXPECT_EQ(ContentSettingsPattern::Wildcard(), host_settings[0].primary_pattern) @@ -1755,6 +1712,14 @@ host_settings[0].primary_pattern) << host_settings[0].primary_pattern.ToString(); EXPECT_EQ(CONTENT_SETTING_ASK, host_settings[0].GetContentSetting()); + + map->GetSettingsForOneType(CONTENT_SETTINGS_TYPE_COOKIES, std::string(), + &host_settings); + ASSERT_EQ(1u, host_settings.size()); + EXPECT_EQ(ContentSettingsPattern::Wildcard(), + host_settings[0].primary_pattern) + << host_settings[0].primary_pattern.ToString(); + EXPECT_EQ(CONTENT_SETTING_ALLOW, host_settings[0].GetContentSetting()); } TEST_F(ChromeBrowsingDataRemoverDelegateTest, RemoveDurablePermission) {
diff --git a/chrome/browser/password_manager/chrome_password_manager_client.cc b/chrome/browser/password_manager/chrome_password_manager_client.cc index f93d6e5..5e479c2d 100644 --- a/chrome/browser/password_manager/chrome_password_manager_client.cc +++ b/chrome/browser/password_manager/chrome_password_manager_client.cc
@@ -288,10 +288,10 @@ if (update_password) { UpdatePasswordInfoBarDelegate::Create(web_contents(), std::move(form_to_save)); - return true; + } else { + SavePasswordInfoBarDelegate::Create(web_contents(), + std::move(form_to_save)); } - SavePasswordInfoBarDelegate::Create(web_contents(), - std::move(form_to_save)); #endif // !defined(OS_ANDROID) return true; }
diff --git a/chrome/browser/password_manager/save_password_infobar_delegate_android.cc b/chrome/browser/password_manager/save_password_infobar_delegate_android.cc index 9e98b2a..4c9cba8 100644 --- a/chrome/browser/password_manager/save_password_infobar_delegate_android.cc +++ b/chrome/browser/password_manager/save_password_infobar_delegate_android.cc
@@ -17,6 +17,7 @@ #include "components/infobars/core/infobar.h" #include "components/infobars/core/infobar_manager.h" #include "components/password_manager/core/browser/password_bubble_experiment.h" +#include "components/password_manager/core/browser/password_form_metrics_recorder.h" #include "content/public/browser/web_contents.h" #include "ui/base/l10n/l10n_util.h" #include "url/origin.h" @@ -41,6 +42,7 @@ SavePasswordInfoBarDelegate::~SavePasswordInfoBarDelegate() { password_manager::metrics_util::LogUIDismissalReason(infobar_response_); + form_to_save_->metrics_recorder()->RecordUIDismissalReason(infobar_response_); } SavePasswordInfoBarDelegate::SavePasswordInfoBarDelegate( @@ -62,6 +64,10 @@ &message, &message_link_range); SetMessage(message); SetMessageLinkRange(message_link_range); + + form_to_save_->metrics_recorder()->RecordPasswordBubbleShown( + form_to_save_->GetCredentialSource(), + password_manager::metrics_util::AUTOMATIC_WITH_PASSWORD_PENDING); } infobars::InfoBarDelegate::InfoBarIdentifier
diff --git a/chrome/browser/password_manager/save_password_infobar_delegate_android_unittest.cc b/chrome/browser/password_manager/save_password_infobar_delegate_android_unittest.cc index 311162fd..a00ed800 100644 --- a/chrome/browser/password_manager/save_password_infobar_delegate_android_unittest.cc +++ b/chrome/browser/password_manager/save_password_infobar_delegate_android_unittest.cc
@@ -21,10 +21,14 @@ #include "components/password_manager/core/browser/stub_password_manager_driver.h" #include "components/password_manager/core/common/password_manager_pref_names.h" #include "components/prefs/pref_service.h" +#include "components/ukm/test_ukm_recorder.h" +#include "components/ukm/ukm_source.h" #include "content/public/browser/web_contents.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" +using password_manager::PasswordFormMetricsRecorder; + namespace { class MockPasswordFormManager : public password_manager::PasswordFormManager { @@ -75,7 +79,8 @@ PrefService* prefs(); const autofill::PasswordForm& test_form() { return test_form_; } - std::unique_ptr<MockPasswordFormManager> CreateMockFormManager(); + std::unique_ptr<MockPasswordFormManager> CreateMockFormManager( + scoped_refptr<PasswordFormMetricsRecorder> metrics_recorder); protected: std::unique_ptr<ConfirmInfoBarDelegate> CreateDelegate( @@ -99,7 +104,6 @@ test_form_.origin = GURL("http://example.com"); test_form_.username_value = base::ASCIIToUTF16("username"); test_form_.password_value = base::ASCIIToUTF16("12345"); - fetcher_.Fetch(); } PrefService* SavePasswordInfoBarDelegateTest::prefs() { @@ -109,11 +113,15 @@ } std::unique_ptr<MockPasswordFormManager> -SavePasswordInfoBarDelegateTest::CreateMockFormManager() { +SavePasswordInfoBarDelegateTest::CreateMockFormManager( + scoped_refptr<PasswordFormMetricsRecorder> metrics_recorder) { auto manager = base::MakeUnique<MockPasswordFormManager>( &password_manager_, &client_, driver_.AsWeakPtr(), test_form(), &fetcher_); - manager->Init(nullptr); + manager->Init(metrics_recorder); + manager->ProvisionallySave( + test_form(), + password_manager::PasswordFormManager::ALLOW_OTHER_POSSIBLE_USERNAMES); return manager; } @@ -137,7 +145,7 @@ TEST_F(SavePasswordInfoBarDelegateTest, CancelTestCredentialSourceAPI) { std::unique_ptr<MockPasswordFormManager> password_form_manager( - CreateMockFormManager()); + CreateMockFormManager(nullptr)); EXPECT_CALL(*password_form_manager.get(), PermanentlyBlacklist()); std::unique_ptr<ConfirmInfoBarDelegate> infobar( CreateDelegate(std::move(password_form_manager))); @@ -147,9 +155,85 @@ TEST_F(SavePasswordInfoBarDelegateTest, CancelTestCredentialSourcePasswordManager) { std::unique_ptr<MockPasswordFormManager> password_form_manager( - CreateMockFormManager()); + CreateMockFormManager(nullptr)); EXPECT_CALL(*password_form_manager.get(), PermanentlyBlacklist()); std::unique_ptr<ConfirmInfoBarDelegate> infobar( CreateDelegate(std::move(password_form_manager))); EXPECT_TRUE(infobar->Cancel()); } + +class SavePasswordInfoBarDelegateTestForUKMs + : public SavePasswordInfoBarDelegateTest, + public ::testing::WithParamInterface< + PasswordFormMetricsRecorder::BubbleDismissalReason> { + public: + SavePasswordInfoBarDelegateTestForUKMs() = default; + ~SavePasswordInfoBarDelegateTestForUKMs() = default; +}; + +// Verify that URL keyed metrics are recorded for showing and interacting with +// the password save prompt. +TEST_P(SavePasswordInfoBarDelegateTestForUKMs, VerifyUKMRecording) { + using BubbleTrigger = PasswordFormMetricsRecorder::BubbleTrigger; + using BubbleDismissalReason = + PasswordFormMetricsRecorder::BubbleDismissalReason; + + BubbleDismissalReason dismissal_reason = GetParam(); + SCOPED_TRACE(::testing::Message() << "dismissal_reason = " + << static_cast<int64_t>(dismissal_reason)); + + ukm::TestUkmRecorder test_ukm_recorder; + { + // Setup metrics recorder + ukm::SourceId source_id = test_ukm_recorder.GetNewSourceID(); + static_cast<ukm::UkmRecorder*>(&test_ukm_recorder) + ->UpdateSourceURL(source_id, GURL("https://www.example.com/")); + auto recorder = base::MakeRefCounted<PasswordFormMetricsRecorder>( + true /*is_main_frame_secure*/, + PasswordFormMetricsRecorder::CreateUkmEntryBuilder(&test_ukm_recorder, + source_id)); + + // Exercise delegate. + std::unique_ptr<MockPasswordFormManager> password_form_manager( + CreateMockFormManager(recorder)); + if (dismissal_reason == BubbleDismissalReason::kDeclined) + EXPECT_CALL(*password_form_manager.get(), PermanentlyBlacklist()); + std::unique_ptr<ConfirmInfoBarDelegate> infobar( + CreateDelegate(std::move(password_form_manager))); + switch (dismissal_reason) { + case BubbleDismissalReason::kAccepted: + EXPECT_TRUE(infobar->Accept()); + break; + case BubbleDismissalReason::kDeclined: + EXPECT_TRUE(infobar->Cancel()); + break; + case BubbleDismissalReason::kIgnored: + // Do nothing. + break; + case BubbleDismissalReason::kUnknown: + NOTREACHED(); + break; + } + } + + // Verify metrics. + const ukm::UkmSource* source = + test_ukm_recorder.GetSourceForUrl("https://www.example.com/"); + ASSERT_TRUE(source); + test_ukm_recorder.ExpectMetric(*source, "PasswordForm", + password_manager::kUkmSavingPromptShown, 1); + test_ukm_recorder.ExpectMetric( + *source, "PasswordForm", password_manager::kUkmSavingPromptTrigger, + static_cast<int64_t>(BubbleTrigger::kPasswordManagerSuggestionAutomatic)); + test_ukm_recorder.ExpectMetric(*source, "PasswordForm", + password_manager::kUkmSavingPromptInteraction, + static_cast<int64_t>(dismissal_reason)); +} + +INSTANTIATE_TEST_CASE_P( + /*no extra name*/, + SavePasswordInfoBarDelegateTestForUKMs, + ::testing::Values( + PasswordFormMetricsRecorder::BubbleDismissalReason::kAccepted, + PasswordFormMetricsRecorder::BubbleDismissalReason::kDeclined, + PasswordFormMetricsRecorder::BubbleDismissalReason::kIgnored));
diff --git a/chrome/browser/password_manager/update_password_infobar_delegate_android.cc b/chrome/browser/password_manager/update_password_infobar_delegate_android.cc index ebc381ad..8e894f0 100644 --- a/chrome/browser/password_manager/update_password_infobar_delegate_android.cc +++ b/chrome/browser/password_manager/update_password_infobar_delegate_android.cc
@@ -17,6 +17,7 @@ #include "components/browser_sync/profile_sync_service.h" #include "components/infobars/core/infobar.h" #include "components/password_manager/core/browser/password_bubble_experiment.h" +#include "components/password_manager/core/browser/password_form_metrics_recorder.h" #include "components/strings/grit/components_strings.h" #include "content/public/browser/web_contents.h" #include "ui/base/l10n/l10n_util.h" @@ -36,7 +37,10 @@ is_smartlock_branding_enabled)))); } -UpdatePasswordInfoBarDelegate::~UpdatePasswordInfoBarDelegate() {} +UpdatePasswordInfoBarDelegate::~UpdatePasswordInfoBarDelegate() { + passwords_state_.form_manager()->metrics_recorder()->RecordUIDismissalReason( + infobar_response_); +} base::string16 UpdatePasswordInfoBarDelegate::GetBranding() const { return l10n_util::GetStringUTF16(is_smartlock_branding_enabled_ @@ -61,7 +65,8 @@ content::WebContents* web_contents, std::unique_ptr<password_manager::PasswordFormManager> form_to_update, bool is_smartlock_branding_enabled) - : is_smartlock_branding_enabled_(is_smartlock_branding_enabled) { + : infobar_response_(password_manager::metrics_util::NO_DIRECT_INTERACTION), + is_smartlock_branding_enabled_(is_smartlock_branding_enabled) { base::string16 message; gfx::Range message_link_range = gfx::Range(); GetSavePasswordDialogTitleTextAndLinkRange( @@ -72,6 +77,10 @@ SetMessageLinkRange(message_link_range); // TODO(melandory): Add histograms, crbug.com/577129 + form_to_update->metrics_recorder()->RecordPasswordBubbleShown( + form_to_update->GetCredentialSource(), + password_manager::metrics_util::AUTOMATIC_WITH_PASSWORD_PENDING_UPDATE); + passwords_state_.set_client( ChromePasswordManagerClient::FromWebContents(web_contents)); passwords_state_.OnUpdatePassword(std::move(form_to_update)); @@ -91,7 +100,12 @@ return l10n_util::GetStringUTF16(IDS_PASSWORD_MANAGER_UPDATE_BUTTON); } +void UpdatePasswordInfoBarDelegate::InfoBarDismissed() { + infobar_response_ = password_manager::metrics_util::CLICKED_CANCEL; +} + bool UpdatePasswordInfoBarDelegate::Accept() { + infobar_response_ = password_manager::metrics_util::CLICKED_SAVE; UpdatePasswordInfoBar* update_password_infobar = static_cast<UpdatePasswordInfoBar*>(infobar()); password_manager::PasswordFormManager* form_manager = @@ -106,3 +120,8 @@ } return true; } + +bool UpdatePasswordInfoBarDelegate::Cancel() { + infobar_response_ = password_manager::metrics_util::CLICKED_CANCEL; + return true; +}
diff --git a/chrome/browser/password_manager/update_password_infobar_delegate_android.h b/chrome/browser/password_manager/update_password_infobar_delegate_android.h index 7bdf489..7e8b345 100644 --- a/chrome/browser/password_manager/update_password_infobar_delegate_android.h +++ b/chrome/browser/password_manager/update_password_infobar_delegate_android.h
@@ -12,6 +12,7 @@ #include "chrome/browser/password_manager/password_manager_infobar_delegate_android.h" #include "chrome/browser/ui/passwords/manage_passwords_state.h" #include "components/password_manager/core/browser/password_form_manager.h" +#include "components/password_manager/core/browser/password_manager_metrics_util.h" namespace content { class WebContents; @@ -57,11 +58,16 @@ std::unique_ptr<password_manager::PasswordFormManager> form_to_update, bool is_smartlock_branding_enabled); + // Used to track the results we get from the info bar. + password_manager::metrics_util::UIDismissalReason infobar_response_; + // ConfirmInfoBarDelegate: infobars::InfoBarDelegate::InfoBarIdentifier GetIdentifier() const override; int GetButtons() const override; base::string16 GetButtonLabel(InfoBarButton button) const override; bool Accept() override; + void InfoBarDismissed() override; + bool Cancel() override; ManagePasswordsState passwords_state_; base::string16 branding_;
diff --git a/chrome/browser/resources/chromeos/login/active_directory_password_change.html b/chrome/browser/resources/chromeos/login/active_directory_password_change.html index 3f21f19..194c1a5 100644 --- a/chrome/browser/resources/chromeos/login/active_directory_password_change.html +++ b/chrome/browser/resources/chromeos/login/active_directory_password_change.html
@@ -2,7 +2,6 @@ Use of this source code is governed by a BSD-style license that can be found in the LICENSE file. --> -<link rel="import" href="chrome://resources/polymer/v1_0/iron-flex-layout/classes/iron-flex-layout.html"> <link rel="import" href="chrome://resources/polymer/v1_0/neon-animation/animations/fade-in-animation.html"> <link rel="import" href="chrome://resources/polymer/v1_0/neon-animation/animations/fade-out-animation.html"> <link rel="import" href="chrome://resources/polymer/v1_0/neon-animation/neon-animatable.html"> @@ -27,6 +26,7 @@ --> <dom-module id="active-directory-password-change"> <link rel="stylesheet" href="gaia_password_changed.css"> + <link rel="stylesheet" href="oobe_flex_layout.css"> <template> <neon-animated-pages id="animatedPages" class="fit" entry-animation="fade-in-animation" exit-animation="fade-out-animation"
diff --git a/chrome/browser/resources/chromeos/login/arc_terms_of_service.html b/chrome/browser/resources/chromeos/login/arc_terms_of_service.html index e859848d..13c31aa 100644 --- a/chrome/browser/resources/chromeos/login/arc_terms_of_service.html +++ b/chrome/browser/resources/chromeos/login/arc_terms_of_service.html
@@ -2,7 +2,6 @@ Use of this source code is governed by a BSD-style license that can be found in the LICENSE file. --> -<link rel="import" href="chrome://resources/polymer/v1_0/iron-flex-layout/classes/iron-flex-layout.html"> <link rel="import" href="chrome://resources/polymer/v1_0/iron-icon/iron-icon.html"> <link rel="import" href="chrome://resources/polymer/v1_0/paper-checkbox/paper-checkbox.html"> <link rel="import" href="chrome://resources/polymer/v1_0/paper-styles/color.html"> @@ -12,6 +11,7 @@ <template> <link rel="stylesheet" href="arc_terms_of_service.css"> <link rel="stylesheet" href="oobe_dialog_parameters.css"> + <link rel="stylesheet" href="oobe_flex_layout.css"> <oobe-dialog id="arc-tos-dialog-md" class="arc-tos-loading" has-buttons> <iron-icon src="chrome://oobe/playstore.svg" class="oobe-icon"> </iron-icon>
diff --git a/chrome/browser/resources/chromeos/login/controller-pairing-screen.html b/chrome/browser/resources/chromeos/login/controller-pairing-screen.html index 7613bfd6..4976681 100644 --- a/chrome/browser/resources/chromeos/login/controller-pairing-screen.html +++ b/chrome/browser/resources/chromeos/login/controller-pairing-screen.html
@@ -2,7 +2,6 @@ Use of this source code is governed by a BSD-style license that can be found in the LICENSE file. --> -<link rel="import" href="chrome://resources/polymer/v1_0/iron-flex-layout/classes/iron-flex-layout.html"> <link rel="import" href="chrome://resources/polymer/v1_0/iron-icon/iron-icon.html"> <link rel="import" href="chrome://resources/polymer/v1_0/iron-iconset-svg/iron-iconset-svg.html"> <link rel="import" href="chrome://resources/polymer/v1_0/iron-selector/iron-selector.html"> @@ -28,6 +27,7 @@ user actions and a spinner is shown near selected device. --> <dom-module id="pairing-device-list"> + <link rel="stylesheet" href="oobe_flex_layout.css"> <link rel="stylesheet" href="pairing_device_list.css"> <iron-iconset-svg name="pairing-device-list-icons">
diff --git a/chrome/browser/resources/chromeos/login/encryption_migration.html b/chrome/browser/resources/chromeos/login/encryption_migration.html index 14afc2a..540e238 100644 --- a/chrome/browser/resources/chromeos/login/encryption_migration.html +++ b/chrome/browser/resources/chromeos/login/encryption_migration.html
@@ -4,7 +4,6 @@ <link rel="import" href="chrome://resources/cr_elements/icons.html"> <link rel="import" href="chrome://resources/html/i18n_behavior.html"> -<link rel="import" href="chrome://resources/polymer/v1_0/iron-flex-layout/classes/iron-flex-layout.html"> <link rel="import" href="chrome://resources/polymer/v1_0/paper-progress/paper-progress.html"> <link rel="import" href="chrome://resources/polymer/v1_0/paper-styles/color.html"> <link rel="import" href="chrome://oobe/custom_elements.html"> @@ -13,6 +12,7 @@ <template> <link rel="stylesheet" href="encryption_migration.css"> <link rel="stylesheet" href="oobe_dialog_parameters.css"> + <link rel="stylesheet" href="oobe_flex_layout.css"> <template is="dom-if" if="[[isInitial_(uiState)]]"> <oobe-dialog tabindex="0" has-buttons></oobe-dialog> </template>
diff --git a/chrome/browser/resources/chromeos/login/gaia_card.html b/chrome/browser/resources/chromeos/login/gaia_card.html index 6315118..f1c8494 100644 --- a/chrome/browser/resources/chromeos/login/gaia_card.html +++ b/chrome/browser/resources/chromeos/login/gaia_card.html
@@ -2,7 +2,6 @@ Use of this source code is governed by a BSD-style license that can be found in the LICENSE file. --> -<link rel="import" href="chrome://resources/polymer/v1_0/iron-flex-layout/classes/iron-flex-layout.html"> <link rel="import" href="chrome://resources/polymer/v1_0/paper-progress/paper-progress.html"> <link rel="import" href="chrome://resources/polymer/v1_0/paper-styles/color.html"> <link rel="import" href="chrome://resources/polymer/v1_0/polymer/polymer.html"> @@ -25,6 +24,7 @@ --> <dom-module id="gaia-card"> <link rel="stylesheet" href="gaia_card.css"> + <link rel="stylesheet" href="oobe_flex_layout.css"> <template> <div class="gaia-header vertical layout relative">
diff --git a/chrome/browser/resources/chromeos/login/gaia_header.html b/chrome/browser/resources/chromeos/login/gaia_header.html index 6318e22..2fd9beb 100644 --- a/chrome/browser/resources/chromeos/login/gaia_header.html +++ b/chrome/browser/resources/chromeos/login/gaia_header.html
@@ -2,7 +2,6 @@ Use of this source code is governed by a BSD-style license that can be found in the LICENSE file. --> -<link rel="import" href="chrome://resources/polymer/v1_0/iron-flex-layout/classes/iron-flex-layout.html"> <link rel="import" href="chrome://resources/polymer/v1_0/polymer/polymer.html"> <!-- @@ -17,6 +16,7 @@ --> <dom-module id="gaia-header"> <link rel="stylesheet" href="gaia_header.css"> + <link rel="stylesheet" href="oobe_flex_layout.css"> <template> <img src="chrome://theme/IDR_LOGO_AVATAR_CIRCLE_BLUE_COLOR" alt
diff --git a/chrome/browser/resources/chromeos/login/gaia_input.html b/chrome/browser/resources/chromeos/login/gaia_input.html index 67cac86..4f1e66d6 100644 --- a/chrome/browser/resources/chromeos/login/gaia_input.html +++ b/chrome/browser/resources/chromeos/login/gaia_input.html
@@ -2,7 +2,6 @@ Use of this source code is governed by a BSD-style license that can be found in the LICENSE file. --> -<link rel="import" href="chrome://resources/polymer/v1_0/iron-flex-layout/classes/iron-flex-layout.html"> <link rel="import" href="chrome://resources/polymer/v1_0/iron-input/iron-input.html"> <link rel="import" href="chrome://resources/polymer/v1_0/paper-input/paper-input-container.html"> <link rel="import" href="chrome://resources/polymer/v1_0/paper-input/paper-input-error.html"> @@ -36,6 +35,7 @@ --> <dom-module id="gaia-input"> <link rel="stylesheet" href="gaia_input.css"> + <link rel="stylesheet" href="oobe_flex_layout.css"> <template> <paper-input-container id="decorator" on-tap="onTap"
diff --git a/chrome/browser/resources/chromeos/login/gaia_input_form.html b/chrome/browser/resources/chromeos/login/gaia_input_form.html index cad99e230..8378295 100644 --- a/chrome/browser/resources/chromeos/login/gaia_input_form.html +++ b/chrome/browser/resources/chromeos/login/gaia_input_form.html
@@ -2,7 +2,6 @@ Use of this source code is governed by a BSD-style license that can be found in the LICENSE file. --> -<link rel="import" href="chrome://resources/polymer/v1_0/iron-flex-layout/classes/iron-flex-layout.html"> <link rel="import" href="chrome://resources/polymer/v1_0/polymer/polymer.html"> <!-- @@ -25,6 +24,7 @@ --> <dom-module id="gaia-input-form"> <link rel="stylesheet" href="gaia_input_form.css"> + <link rel="stylesheet" href="oobe_flex_layout.css"> <template> <div on-keydown="onKeyDown_">
diff --git a/chrome/browser/resources/chromeos/login/gaia_password_changed.html b/chrome/browser/resources/chromeos/login/gaia_password_changed.html index bcb6d33..5457b44 100644 --- a/chrome/browser/resources/chromeos/login/gaia_password_changed.html +++ b/chrome/browser/resources/chromeos/login/gaia_password_changed.html
@@ -2,7 +2,6 @@ Use of this source code is governed by a BSD-style license that can be found in the LICENSE file. --> -<link rel="import" href="chrome://resources/polymer/v1_0/iron-flex-layout/classes/iron-flex-layout.html"> <link rel="import" href="chrome://resources/polymer/v1_0/iron-icons/iron-icons.html"> <link rel="import" href="chrome://resources/polymer/v1_0/neon-animation/animations/fade-in-animation.html"> <link rel="import" href="chrome://resources/polymer/v1_0/neon-animation/animations/fade-out-animation.html"> @@ -41,6 +40,7 @@ --> <dom-module id="gaia-password-changed"> <link rel="stylesheet" href="gaia_password_changed.css"> + <link rel="stylesheet" href="oobe_flex_layout.css"> <template> <neon-animated-pages id="animatedPages" class="fit"
diff --git a/chrome/browser/resources/chromeos/login/header_bar.html b/chrome/browser/resources/chromeos/login/header_bar.html index c58d052..b5c4b23 100644 --- a/chrome/browser/resources/chromeos/login/header_bar.html +++ b/chrome/browser/resources/chromeos/login/header_bar.html
@@ -1,5 +1,4 @@ -<link rel="import" href="chrome://resources/polymer/v1_0/iron-flex-layout/classes/iron-flex-layout.html"> - +<link rel="stylesheet" href="oobe_flex_layout.css"> <div id="login-header-bar" hidden class="login-header-bar-hidden layout horizontal"> <div id="shutdown-header-bar-item" class="header-bar-item">
diff --git a/chrome/browser/resources/chromeos/login/host-pairing-screen.html b/chrome/browser/resources/chromeos/login/host-pairing-screen.html index 4f973744..48cee7d 100644 --- a/chrome/browser/resources/chromeos/login/host-pairing-screen.html +++ b/chrome/browser/resources/chromeos/login/host-pairing-screen.html
@@ -2,7 +2,6 @@ Use of this source code is governed by a BSD-style license that can be found in the LICENSE file. --> -<link rel="import" href="chrome://resources/polymer/v1_0/iron-flex-layout/classes/iron-flex-layout.html"> <link rel="import" href="chrome://resources/polymer/v1_0/iron-icon/iron-icon.html"> <link rel="import" href="chrome://resources/polymer/v1_0/iron-icons/device-icons.html"> <link rel="import" href="chrome://resources/polymer/v1_0/neon-animation/animations/fade-in-animation.html"> @@ -28,7 +27,6 @@ </dom-module> <dom-module id="host-pairing-screen"> - <link rel="stylesheet" href="oobe_screen_host_pairing.css"> <template>
diff --git a/chrome/browser/resources/chromeos/login/md_header_bar.html b/chrome/browser/resources/chromeos/login/md_header_bar.html index 04df3a9..9b25573 100644 --- a/chrome/browser/resources/chromeos/login/md_header_bar.html +++ b/chrome/browser/resources/chromeos/login/md_header_bar.html
@@ -1,5 +1,4 @@ -<link rel="import" href="chrome://resources/polymer/v1_0/iron-flex-layout/classes/iron-flex-layout.html"> - +<link rel="stylesheet" href="oobe_flex_layout.css"> <div id="login-header-bar" hidden class="login-header-bar-hidden layout horizontal"> <div id="shutdown-header-bar-item" class="header-bar-item">
diff --git a/chrome/browser/resources/chromeos/login/navigation_bar.html b/chrome/browser/resources/chromeos/login/navigation_bar.html index 60a3ab3c..cdf416e 100644 --- a/chrome/browser/resources/chromeos/login/navigation_bar.html +++ b/chrome/browser/resources/chromeos/login/navigation_bar.html
@@ -2,9 +2,6 @@ Use of this source code is governed by a BSD-style license that can be found in the LICENSE file. --> -<link rel="import" href="chrome://resources/polymer/v1_0/iron-flex-layout/classes/iron-flex-layout.html"> - - <!-- A navigation panel with back, refresh, and/or close buttons. ┌─────────────────────────────────┐ @@ -25,6 +22,7 @@ --> <dom-module id="navigation-bar"> <link rel="stylesheet" href="navigation_bar.css"> + <link rel="stylesheet" href="oobe_flex_layout.css"> <template> <div class="fit layout horizontal center">
diff --git a/chrome/browser/resources/chromeos/login/notification_card.html b/chrome/browser/resources/chromeos/login/notification_card.html index a3dee9f..dbb77b7ba 100644 --- a/chrome/browser/resources/chromeos/login/notification_card.html +++ b/chrome/browser/resources/chromeos/login/notification_card.html
@@ -2,7 +2,6 @@ Use of this source code is governed by a BSD-style license that can be found in the LICENSE file. --> -<link rel="import" href="chrome://resources/polymer/v1_0/iron-flex-layout/classes/iron-flex-layout.html"> <link rel="import" href="chrome://resources/polymer/v1_0/iron-icons/iron-icons.html"> <link rel="import" href="chrome://resources/polymer/v1_0/paper-styles/color.html"> <link rel="import" href="chrome://resources/polymer/v1_0/polymer/polymer.html"> @@ -28,6 +27,7 @@ --> <dom-module id="notification-card"> <link rel="stylesheet" href="notification_card.css"> + <link rel="stylesheet" href="oobe_flex_layout.css"> <template> <div id="container" class="vertical layout center fit">
diff --git a/chrome/browser/resources/chromeos/login/offline_ad_login.html b/chrome/browser/resources/chromeos/login/offline_ad_login.html index 4c26792..2a51d97 100644 --- a/chrome/browser/resources/chromeos/login/offline_ad_login.html +++ b/chrome/browser/resources/chromeos/login/offline_ad_login.html
@@ -2,7 +2,6 @@ Use of this source code is governed by a BSD-style license that can be found in the LICENSE file. --> -<link rel="import" href="chrome://resources/polymer/v1_0/iron-flex-layout/classes/iron-flex-layout.html"> <link rel="import" href="chrome://resources/polymer/v1_0/polymer/polymer.html"> <!-- @@ -34,6 +33,7 @@ --> <dom-module id="offline-ad-login"> <link rel="stylesheet" href="offline_gaia.css"> + <link rel="stylesheet" href="oobe_flex_layout.css"> <template> <gaia-card id="gaiaCard" class="fit"> <div class="header flex vertical layout end-justified start">
diff --git a/chrome/browser/resources/chromeos/login/offline_gaia.html b/chrome/browser/resources/chromeos/login/offline_gaia.html index fb30b000..7d95d1f 100644 --- a/chrome/browser/resources/chromeos/login/offline_gaia.html +++ b/chrome/browser/resources/chromeos/login/offline_gaia.html
@@ -4,7 +4,6 @@ <link rel="import" href="chrome://resources/cr_elements/shared_style_css.html"> <link rel="import" href="chrome://resources/cr_elements/cr_dialog/cr_dialog.html"> -<link rel="import" href="chrome://resources/polymer/v1_0/iron-flex-layout/classes/iron-flex-layout.html"> <link rel="import" href="chrome://resources/polymer/v1_0/neon-animation/animations/slide-from-left-animation.html"> <link rel="import" href="chrome://resources/polymer/v1_0/neon-animation/animations/slide-from-right-animation.html"> <link rel="import" href="chrome://resources/polymer/v1_0/neon-animation/animations/slide-left-animation.html"> @@ -49,6 +48,7 @@ <template> <style include="cr-shared-style"></style> + <link rel="stylesheet" href="oobe_flex_layout.css"> <link rel="stylesheet" href="oobe_dialog_host.css"> <link rel="stylesheet" href="oobe_dialog_parameters.css"> <template is="dom-if" if="[[glifMode]]" restamp>
diff --git a/chrome/browser/resources/chromeos/login/oobe_a11y_option.html b/chrome/browser/resources/chromeos/login/oobe_a11y_option.html index 81f5e01..925b5760 100644 --- a/chrome/browser/resources/chromeos/login/oobe_a11y_option.html +++ b/chrome/browser/resources/chromeos/login/oobe_a11y_option.html
@@ -1,5 +1,4 @@ <link rel="import" href="chrome://resources/html/polymer.html"> -<link rel="import" href="chrome://resources/polymer/v1_0/iron-flex-layout/classes/iron-flex-layout.html"> <link rel="import" href="chrome://resources/polymer/v1_0/paper-toggle-button/paper-toggle-button.html"> <dom-module id="oobe-a11y-option"> @@ -31,6 +30,7 @@ var(--oobe-toggle-button-size); } </style> + <link rel="stylesheet" href="oobe_flex_layout.css"> <link rel="stylesheet" href="oobe_a11y_option.css"> <div id="elementBox" class="layout horizontal"> <div class="flex layout vertical center-justified">
diff --git a/chrome/browser/resources/chromeos/login/oobe_buttons.html b/chrome/browser/resources/chromeos/login/oobe_buttons.html index 36bbe31f..9a7d998 100644 --- a/chrome/browser/resources/chromeos/login/oobe_buttons.html +++ b/chrome/browser/resources/chromeos/login/oobe_buttons.html
@@ -40,6 +40,7 @@ 'border' - adds border to the button. --> <dom-module id="oobe-text-button"> + <link rel="stylesheet" href="oobe_flex_layout.css"> <link rel="stylesheet" href="oobe_text_button.css"> <template> <style> @@ -76,6 +77,7 @@ 'label-for-aria' - accessibility label. --> <dom-module id="oobe-back-button"> + <link rel="stylesheet" href="oobe_flex_layout.css"> <link rel="stylesheet" href="oobe_nav_button.css"> <template> <paper-button id="button" on-tap="onClick_" disabled="[[disabled]]" @@ -89,6 +91,7 @@ </dom-module> <dom-module id="oobe-next-button"> + <link rel="stylesheet" href="oobe_flex_layout.css"> <link rel="stylesheet" href="oobe_nav_button.css"> <template> <paper-button id="button" on-tap="onClick_" disabled="[[disabled]]"> @@ -115,6 +118,7 @@ 'label-for-aria' - accessibility label. --> <dom-module id="oobe-welcome-secondary-button"> + <link rel="stylesheet" href="oobe_flex_layout.css"> <link rel="stylesheet" href="oobe_welcome_secondary_button.css"> <template> <paper-button id="button" aria-label$="[[labelForAria]]">
diff --git a/chrome/browser/resources/chromeos/login/oobe_dialog.html b/chrome/browser/resources/chromeos/login/oobe_dialog.html index 4b1d8e5b..74855bcd 100644 --- a/chrome/browser/resources/chromeos/login/oobe_dialog.html +++ b/chrome/browser/resources/chromeos/login/oobe_dialog.html
@@ -2,7 +2,6 @@ Use of this source code is governed by a BSD-style license that can be found in the LICENSE file. --> -<link rel="import" href="chrome://resources/polymer/v1_0/iron-flex-layout/classes/iron-flex-layout.html"> <link rel="import" href="chrome://resources/polymer/v1_0/paper-styles/color.html"> <!-- @@ -57,6 +56,7 @@ <template> <link rel="stylesheet" href="oobe_dialog_host.css"> <link rel="stylesheet" href="oobe_dialog.css"> + <link rel="stylesheet" href="oobe_flex_layout.css"> <div id="header-container" hidden="[[noHeader]]" android$="[[android]]"> <div class="oobe-icon-div"> <content select=".oobe-icon"></content>
diff --git a/chrome/browser/resources/chromeos/login/oobe_eula.html b/chrome/browser/resources/chromeos/login/oobe_eula.html index 908926c..1a446fb 100644 --- a/chrome/browser/resources/chromeos/login/oobe_eula.html +++ b/chrome/browser/resources/chromeos/login/oobe_eula.html
@@ -2,7 +2,6 @@ Use of this source code is governed by a BSD-style license that can be found in the LICENSE file. --> -<link rel="import" href="chrome://resources/polymer/v1_0/iron-flex-layout/classes/iron-flex-layout.html"> <link rel="import" href="chrome://resources/polymer/v1_0/iron-icon/iron-icon.html"> <link rel="import" href="chrome://resources/polymer/v1_0/iron-iconset-svg/iron-iconset-svg.html"> <link rel="import" href="chrome://resources/polymer/v1_0/paper-checkbox/paper-checkbox.html"> @@ -42,6 +41,7 @@ <template> <link rel="stylesheet" href="oobe_eula.css"> <link rel="stylesheet" href="oobe_dialog_parameters.css"> + <link rel="stylesheet" href="oobe_flex_layout.css"> <oobe-dialog id="eulaLoadingDialog" hidden="[[!eulaLoadingScreenShown]]" role="dialog" i18n-values="aria-label:termsOfServiceLoading" has-buttons>
diff --git a/chrome/browser/resources/chromeos/login/oobe_flex_layout.css b/chrome/browser/resources/chromeos/login/oobe_flex_layout.css new file mode 100644 index 0000000..e6cd4ca2 --- /dev/null +++ b/chrome/browser/resources/chromeos/login/oobe_flex_layout.css
@@ -0,0 +1,112 @@ +/* Copyright 2017 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. */ + +/****************************************************** + OOBE Flex Layout (based on polymer iron-flex-layout) +*******************************************************/ + +.layout.horizontal, +.layout.horizontal-reverse, +.layout.vertical, +.layout.vertical-reverse { + display: flex; +} + +.layout.inline { + display: inline-flex; +} + +.layout.horizontal { + flex-direction: row; +} + +.layout.horizontal-reverse { + flex-direction: row-reverse; +} + +.layout.vertical { + flex-direction: column; +} + +.layout.vertical-reverse { + flex-direction: column-reverse; +} + +.flex { + flex: 1; +} + +/* alignment in cross axis */ + +.layout.start { + align-items: flex-start; +} + +.layout.center { + align-items: center; +} + +.layout.end { + align-items: flex-end; +} + +/* alignment in main axis */ + +.layout.start-justified { + justify-content: flex-start; +} + +.layout.center-justified { + justify-content: center; +} + +.layout.end-justified { + justify-content: flex-end; +} + +.layout.around-justified { + justify-content: space-around; +} + +.layout.justified { + justify-content: space-between; +} + +/* self alignment */ + +.self-start { + align-self: flex-start; +} + +.self-center { + align-self: center; +} + +.self-end { + align-self: flex-end; +} + +.self-stretch { + align-self: stretch; +} + +/******************************* + Other Layout +*******************************/ + +[hidden] { + display: none !important; +} + +.relative { + position: relative; +} + +.fit { + bottom: 0; + left: 0; + position: absolute; + right: 0; + top: 0; +}
diff --git a/chrome/browser/resources/chromeos/login/oobe_hid_detection.html b/chrome/browser/resources/chromeos/login/oobe_hid_detection.html index acb628c..3f744ee4 100644 --- a/chrome/browser/resources/chromeos/login/oobe_hid_detection.html +++ b/chrome/browser/resources/chromeos/login/oobe_hid_detection.html
@@ -1,4 +1,3 @@ -<link rel="import" href="chrome://resources/polymer/v1_0/iron-flex-layout/classes/iron-flex-layout.html"> <link rel="import" href="chrome://resources/polymer/v1_0/iron-icon/iron-icon.html"> <link rel="import" href="chrome://resources/polymer/v1_0/iron-iconset-svg/iron-iconset-svg.html"> <link rel="import" href="chrome://resources/polymer/v1_0/paper-checkbox/paper-checkbox.html"> @@ -17,6 +16,7 @@ <link rel="stylesheet" href="oobe_hid_detection.css"> <link rel="stylesheet" href="oobe_dialog_parameters.css"> <link rel="stylesheet" href="../../options/chromeos/bluetooth.css"> + <link rel="stylesheet" href="oobe_flex_layout.css"> <oobe-dialog has-buttons> <iron-icon icon="oobe-hid-detection:bluetooth" class="oobe-icon"> </iron-icon>
diff --git a/chrome/browser/resources/chromeos/login/oobe_reset.html b/chrome/browser/resources/chromeos/login/oobe_reset.html index 3afc401..b15e1988 100644 --- a/chrome/browser/resources/chromeos/login/oobe_reset.html +++ b/chrome/browser/resources/chromeos/login/oobe_reset.html
@@ -2,7 +2,6 @@ Use of this source code is governed by a BSD-style license that can be found in the LICENSE file. --> -<link rel="import" href="chrome://resources/polymer/v1_0/iron-flex-layout/classes/iron-flex-layout.html"> <link rel="import" href="chrome://resources/polymer/v1_0/iron-icon/iron-icon.html"> <link rel="import" href="chrome://resources/polymer/v1_0/iron-iconset-svg/iron-iconset-svg.html"> @@ -28,9 +27,10 @@ <dom-module id="oobe-reset-md"> <template> - <link rel="stylesheet" href="oobe_reset.css"> - <link rel="stylesheet" href="oobe_dialog_parameters.css"> <link rel="stylesheet" href="chrome://resources/css/throbber.css"> + <link rel="stylesheet" href="oobe_dialog_parameters.css"> + <link rel="stylesheet" href="oobe_flex_layout.css"> + <link rel="stylesheet" href="oobe_reset.css"> <oobe-dialog id="resetDialog" role="dialog" i18n-values="aria-label:resetScreenAccessibleTitle" has-buttons> <hd-iron-icon class="oobe-icon"
diff --git a/chrome/browser/resources/chromeos/login/oobe_reset_confirmation_overlay.html b/chrome/browser/resources/chromeos/login/oobe_reset_confirmation_overlay.html index a1b2e35..520885e0 100644 --- a/chrome/browser/resources/chromeos/login/oobe_reset_confirmation_overlay.html +++ b/chrome/browser/resources/chromeos/login/oobe_reset_confirmation_overlay.html
@@ -4,6 +4,7 @@ <dom-module id="reset-confirm-overlay-md"> <template> + <link rel="stylesheet" href="oobe_flex_layout.css"> <link rel="stylesheet" href="oobe_reset_confirmation_overlay.css"> <div class="reset-popup not-resizable"> <h1 i18n-content="confirmPowerwashTitle" hidden="[[!isPowerwashView_]]">
diff --git a/chrome/browser/resources/chromeos/login/oobe_screen_oauth_enrollment.html b/chrome/browser/resources/chromeos/login/oobe_screen_oauth_enrollment.html index 231f18c..0f71a3fa 100644 --- a/chrome/browser/resources/chromeos/login/oobe_screen_oauth_enrollment.html +++ b/chrome/browser/resources/chromeos/login/oobe_screen_oauth_enrollment.html
@@ -6,6 +6,7 @@ <div id="oauth-enrollment" class="step no-logo hidden" hidden> <div id="oauth-enroll-step-contents"> + <link rel="stylesheet" href="oobe_flex_layout.css"> <div id="oauth-enroll-step-signin"> <webview id="oauth-enroll-auth-view" name="oauth-enroll-auth-view"> </webview>
diff --git a/chrome/browser/resources/chromeos/login/oobe_screen_user_image.html b/chrome/browser/resources/chromeos/login/oobe_screen_user_image.html index 5bf5e2c..893d47f 100644 --- a/chrome/browser/resources/chromeos/login/oobe_screen_user_image.html +++ b/chrome/browser/resources/chromeos/login/oobe_screen_user_image.html
@@ -35,6 +35,7 @@ </div> <div id="user-image-controls" class="step-controls"></div> <div id="user-images-loading" class="step-loading"> + <link rel="stylesheet" href="oobe_flex_layout.css"> <throbber-notice i18n-values="text:syncingPreferences" class="fit"> </throbber-notice> </div>
diff --git a/chrome/browser/resources/chromeos/login/oobe_update.html b/chrome/browser/resources/chromeos/login/oobe_update.html index 2f01183d..9ebec60d 100644 --- a/chrome/browser/resources/chromeos/login/oobe_update.html +++ b/chrome/browser/resources/chromeos/login/oobe_update.html
@@ -2,7 +2,6 @@ Use of this source code is governed by a BSD-style license that can be found in the LICENSE file. --> -<link rel="import" href="chrome://resources/polymer/v1_0/iron-flex-layout/classes/iron-flex-layout.html"> <link rel="import" href="chrome://resources/polymer/v1_0/iron-icon/iron-icon.html"> <link rel="import" href="chrome://resources/polymer/v1_0/iron-iconset-svg/iron-iconset-svg.html"> <link rel="import" href="chrome://resources/polymer/v1_0/paper-progress/paper-progress.html"> @@ -25,8 +24,9 @@ <dom-module id="oobe-update-md"> <template> - <link rel="stylesheet" href="oobe_update.css"> <link rel="stylesheet" href="oobe_dialog_parameters.css"> + <link rel="stylesheet" href="oobe_flex_layout.css"> + <link rel="stylesheet" href="oobe_update.css"> <oobe-dialog hidden="[[!checkingForUpdate]]" tabindex="0" aria-live="polite"> <iron-icon icon="oobe-update:googleg" class="oobe-icon"></iron-icon>
diff --git a/chrome/browser/resources/chromeos/login/oobe_voice_interaction_value_prop.html b/chrome/browser/resources/chromeos/login/oobe_voice_interaction_value_prop.html index 06630f0..0f638d6 100644 --- a/chrome/browser/resources/chromeos/login/oobe_voice_interaction_value_prop.html +++ b/chrome/browser/resources/chromeos/login/oobe_voice_interaction_value_prop.html
@@ -2,12 +2,11 @@ Use of this source code is governed by a BSD-style license that can be found in the LICENSE file. --> -<link rel="import" href="chrome://resources/polymer/v1_0/iron-flex-layout/classes/iron-flex-layout.html"> - <dom-module id="voice-interaction-value-prop-md"> <template> - <link rel="stylesheet" href="oobe_voice_interaction_value_prop.css"> <link rel="stylesheet" href="oobe_dialog_parameters.css"> + <link rel="stylesheet" href="oobe_flex_layout.css"> + <link rel="stylesheet" href="oobe_voice_interaction_value_prop.css"> <oobe-dialog id="voice-dialog" class="value-prop-loading" role="dialog" hide-shadow has-buttons no-footer-padding android> <div class = "header">
diff --git a/chrome/browser/resources/chromeos/login/oobe_wait_for_container_ready.html b/chrome/browser/resources/chromeos/login/oobe_wait_for_container_ready.html index 4ecc2048..f4828c2 100644 --- a/chrome/browser/resources/chromeos/login/oobe_wait_for_container_ready.html +++ b/chrome/browser/resources/chromeos/login/oobe_wait_for_container_ready.html
@@ -2,12 +2,11 @@ Use of this source code is governed by a BSD-style license that can be found in the LICENSE file. --> -<link rel="import" href="chrome://resources/polymer/v1_0/iron-flex-layout/classes/iron-flex-layout.html"> - <dom-module id="wait-for-container-ready-md"> <template> - <link rel="stylesheet" href="oobe_wait_for_container_ready.css"> <link rel="stylesheet" href="oobe_dialog_parameters.css"> + <link rel="stylesheet" href="oobe_flex_layout.css"> + <link rel="stylesheet" href="oobe_wait_for_container_ready.css"> <oobe-dialog id="waitForContainerReadyDialog" role="dialog" hide-shadow no-footer android> <iron-icon src="https://www.gstatic.com/opa-chromeos/oobe/images/assistant_logo.png" class="oobe-icon">
diff --git a/chrome/browser/resources/chromeos/login/oobe_welcome.html b/chrome/browser/resources/chromeos/login/oobe_welcome.html index b101431..40b2ffc 100644 --- a/chrome/browser/resources/chromeos/login/oobe_welcome.html +++ b/chrome/browser/resources/chromeos/login/oobe_welcome.html
@@ -6,7 +6,6 @@ <link rel="import" href="chrome://resources/cr_elements/chromeos/network/cr_network_select.html"> <link rel="import" href="chrome://resources/cr_elements/chromeos/network/cr_onc_types.html"> <link rel="import" href="chrome://resources/html/i18n_behavior.html"> -<link rel="import" href="chrome://resources/polymer/v1_0/iron-flex-layout/classes/iron-flex-layout.html"> <link rel="import" href="chrome://resources/polymer/v1_0/iron-icon/iron-icon.html"> <link rel="import" href="chrome://resources/polymer/v1_0/iron-iconset-svg/iron-iconset-svg.html"> @@ -83,6 +82,7 @@ <link rel="stylesheet" href="oobe_dialog_host.css"> <link rel="stylesheet" href="oobe_welcome.css"> <link rel="stylesheet" href="oobe_dialog_parameters.css"> + <link rel="stylesheet" href="oobe_flex_layout.css"> <oobe-welcome-dialog id="welcomeScreen" role="dialog" i18n-values="aria-label:networkScreenGreeting" current-language="[[currentLanguage]]"
diff --git a/chrome/browser/resources/chromeos/login/oobe_welcome_dialog.html b/chrome/browser/resources/chromeos/login/oobe_welcome_dialog.html index 125a476b..980ee6ec 100644 --- a/chrome/browser/resources/chromeos/login/oobe_welcome_dialog.html +++ b/chrome/browser/resources/chromeos/login/oobe_welcome_dialog.html
@@ -2,7 +2,6 @@ Use of this source code is governed by a BSD-style license that can be found in the LICENSE file. --> -<link rel="import" href="chrome://resources/polymer/v1_0/iron-flex-layout/classes/iron-flex-layout.html"> <link rel="import" href="chrome://resources/polymer/v1_0/paper-styles/color.html"> <!-- @@ -11,6 +10,7 @@ <dom-module id="oobe-welcome-dialog"> <template> <link rel="stylesheet" href="oobe_dialog_host.css"> + <link rel="stylesheet" href="oobe_flex_layout.css"> <link rel="stylesheet" href="oobe_welcome_dialog.css"> <div class="layout horizontal flex"> <div class="vertical-margin"></div>
diff --git a/chrome/browser/resources/chromeos/login/saml_confirm_password.html b/chrome/browser/resources/chromeos/login/saml_confirm_password.html index 50d94aa..f03a62b 100644 --- a/chrome/browser/resources/chromeos/login/saml_confirm_password.html +++ b/chrome/browser/resources/chromeos/login/saml_confirm_password.html
@@ -4,7 +4,6 @@ <link rel="import" href="chrome://resources/cr_elements/shared_style_css.html"> <link rel="import" href="chrome://resources/cr_elements/cr_dialog/cr_dialog.html"> -<link rel="import" href="chrome://resources/polymer/v1_0/iron-flex-layout/classes/iron-flex-layout.html"> <link rel="import" href="chrome://resources/polymer/v1_0/neon-animation/animations/fade-in-animation.html"> <link rel="import" href="chrome://resources/polymer/v1_0/neon-animation/animations/fade-out-animation.html"> <link rel="import" href="chrome://resources/polymer/v1_0/neon-animation/neon-animatable.html"> @@ -39,6 +38,7 @@ 'focus' - If the current card is the first one, focuses password input. --> <dom-module id="saml-confirm-password"> + <link rel="stylesheet" href="oobe_flex_layout.css"> <link rel="stylesheet" href="saml_confirm_password.css"> <template>
diff --git a/chrome/browser/resources/chromeos/login/unrecoverable_cryptohome_error_card.html b/chrome/browser/resources/chromeos/login/unrecoverable_cryptohome_error_card.html index 0c98c25..54bffd26 100644 --- a/chrome/browser/resources/chromeos/login/unrecoverable_cryptohome_error_card.html +++ b/chrome/browser/resources/chromeos/login/unrecoverable_cryptohome_error_card.html
@@ -2,7 +2,6 @@ Use of this source code is governed by a BSD-style license that can be found in the LICENSE file. --> -<link rel="import" href="chrome://resources/polymer/v1_0/iron-flex-layout/classes/iron-flex-layout.html"> <link rel="import" href="chrome://resources/polymer/v1_0/polymer/polymer.html"> <!-- @@ -15,6 +14,7 @@ 'done' - fired when user clicks on continue button. --> <dom-module id="unrecoverable-cryptohome-error-card"> + <link rel="stylesheet" href="oobe_flex_layout.css"> <link rel="stylesheet" href="unrecoverable_cryptohome_error_card.css"> <template>
diff --git a/chrome/browser/ui/cocoa/passwords/base_passwords_controller_test.mm b/chrome/browser/ui/cocoa/passwords/base_passwords_controller_test.mm index 33a4334..08a9ab7 100644 --- a/chrome/browser/ui/cocoa/passwords/base_passwords_controller_test.mm +++ b/chrome/browser/ui/cocoa/passwords/base_passwords_controller_test.mm
@@ -35,6 +35,8 @@ ui_controller_.reset(new testing::NiceMock<PasswordsModelDelegateMock>); ON_CALL(*ui_controller_, GetWebContents()) .WillByDefault(Return(test_web_contents_.get())); + ON_CALL(*ui_controller_, GetPasswordFormMetricsRecorder()) + .WillByDefault(Return(nullptr)); PasswordStoreFactory::GetInstance()->SetTestingFactoryAndUse( profile(), password_manager::BuildPasswordStore< content::BrowserContext,
diff --git a/chrome/browser/ui/passwords/manage_passwords_bubble_model.cc b/chrome/browser/ui/passwords/manage_passwords_bubble_model.cc index eab9341..63df6d8 100644 --- a/chrome/browser/ui/passwords/manage_passwords_bubble_model.cc +++ b/chrome/browser/ui/passwords/manage_passwords_bubble_model.cc
@@ -22,6 +22,7 @@ #include "chrome/grit/generated_resources.h" #include "components/browser_sync/profile_sync_service.h" #include "components/password_manager/core/browser/password_bubble_experiment.h" +#include "components/password_manager/core/browser/password_form_metrics_recorder.h" #include "components/password_manager/core/browser/password_manager_constants.h" #include "components/password_manager/core/browser/password_store.h" #include "components/password_manager/core/common/password_manager_pref_names.h" @@ -110,11 +111,16 @@ } private: + static password_manager::metrics_util::UIDismissalReason + ToNearestUIDismissalReason( + password_manager::metrics_util::UpdatePasswordSubmissionEvent + update_password_submission_event); + // The way the bubble appeared. const password_manager::metrics_util::UIDisplayDisposition display_disposition_; - // Dismissal reason for a bubble. + // Dismissal reason for a save bubble. Not filled for update bubbles. password_manager::metrics_util::UIDismissalReason dismissal_reason_; // Dismissal reason for the update bubble. @@ -216,6 +222,51 @@ if (update_password_submission_event_ != metrics_util::NO_UPDATE_SUBMISSION) LogUpdatePasswordSubmissionEvent(update_password_submission_event_); } + + // Record UKM statistics on dismissal reason. + if (model->delegate_ && model->delegate_->GetPasswordFormMetricsRecorder()) { + if (model->state() != password_manager::ui::PENDING_PASSWORD_UPDATE_STATE) { + model->delegate_->GetPasswordFormMetricsRecorder() + ->RecordUIDismissalReason(dismissal_reason_); + } else { + model->delegate_->GetPasswordFormMetricsRecorder() + ->RecordUIDismissalReason( + ToNearestUIDismissalReason(update_password_submission_event_)); + } + } +} + +// static +password_manager::metrics_util::UIDismissalReason +ManagePasswordsBubbleModel::InteractionKeeper::ToNearestUIDismissalReason( + password_manager::metrics_util::UpdatePasswordSubmissionEvent + update_password_submission_event) { + switch (update_password_submission_event) { + case password_manager::metrics_util::NO_ACCOUNTS_CLICKED_UPDATE: + case password_manager::metrics_util::ONE_ACCOUNT_CLICKED_UPDATE: + case password_manager::metrics_util::MULTIPLE_ACCOUNTS_CLICKED_UPDATE: + case password_manager::metrics_util::PASSWORD_OVERRIDDEN_CLICKED_UPDATE: + return password_manager::metrics_util::CLICKED_SAVE; + + case password_manager::metrics_util::NO_ACCOUNTS_CLICKED_NOPE: + case password_manager::metrics_util::ONE_ACCOUNT_CLICKED_NOPE: + case password_manager::metrics_util::MULTIPLE_ACCOUNTS_CLICKED_NOPE: + case password_manager::metrics_util::PASSWORD_OVERRIDDEN_CLICKED_NOPE: + return password_manager::metrics_util::CLICKED_CANCEL; + + case password_manager::metrics_util::NO_ACCOUNTS_NO_INTERACTION: + case password_manager::metrics_util::ONE_ACCOUNT_NO_INTERACTION: + case password_manager::metrics_util::MULTIPLE_ACCOUNTS_NO_INTERACTION: + case password_manager::metrics_util::PASSWORD_OVERRIDDEN_NO_INTERACTION: + case password_manager::metrics_util::NO_UPDATE_SUBMISSION: + return password_manager::metrics_util::NO_DIRECT_INTERACTION; + + case password_manager::metrics_util::UPDATE_PASSWORD_EVENT_COUNT: + // Not reached. + break; + } + NOTREACHED(); + return password_manager::metrics_util::NO_DIRECT_INTERACTION; } ManagePasswordsBubbleModel::ManagePasswordsBubbleModel( @@ -318,6 +369,11 @@ break; } } + + if (delegate_ && delegate_->GetPasswordFormMetricsRecorder()) { + delegate_->GetPasswordFormMetricsRecorder()->RecordPasswordBubbleShown( + delegate_->GetCredentialSource(), display_disposition); + } metrics_util::LogUIDisplayDisposition(display_disposition); interaction_keeper_.reset(new InteractionKeeper(std::move(interaction_stats), display_disposition));
diff --git a/chrome/browser/ui/passwords/manage_passwords_bubble_model_unittest.cc b/chrome/browser/ui/passwords/manage_passwords_bubble_model_unittest.cc index a93866b0..b3e03e2 100644 --- a/chrome/browser/ui/passwords/manage_passwords_bubble_model_unittest.cc +++ b/chrome/browser/ui/passwords/manage_passwords_bubble_model_unittest.cc
@@ -9,6 +9,7 @@ #include "base/memory/ptr_util.h" #include "base/metrics/histogram_samples.h" +#include "base/run_loop.h" #include "base/strings/utf_string_conversions.h" #include "base/test/histogram_tester.h" #include "base/test/simple_test_clock.h" @@ -19,6 +20,7 @@ #include "chrome/test/base/testing_profile.h" #include "components/browser_sync/profile_sync_service_mock.h" #include "components/password_manager/core/browser/mock_password_store.h" +#include "components/password_manager/core/browser/password_form_metrics_recorder.h" #include "components/password_manager/core/browser/password_manager_metrics_util.h" #include "components/password_manager/core/browser/password_manager_test_utils.h" #include "components/password_manager/core/browser/statistics_table.h" @@ -26,9 +28,12 @@ #include "components/password_manager/core/common/password_manager_pref_names.h" #include "components/password_manager/core/common/password_manager_ui.h" #include "components/prefs/pref_service.h" +#include "components/ukm/test_ukm_recorder.h" +#include "components/ukm/ukm_source.h" #include "content/public/browser/web_contents.h" #include "content/public/test/test_browser_thread_bundle.h" #include "content/public/test/web_contents_tester.h" +#include "services/metrics/public/cpp/ukm_recorder.h" #include "testing/gmock/include/gmock/gmock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -104,7 +109,9 @@ void SetUp() override { test_web_contents_.reset( content::WebContentsTester::CreateTestWebContents(&profile_, nullptr)); - mock_delegate_.reset(new testing::StrictMock<PasswordsModelDelegateMock>); + mock_delegate_.reset(new PasswordsModelDelegateMock); + ON_CALL(*mock_delegate_, GetPasswordFormMetricsRecorder()) + .WillByDefault(Return(nullptr)); PasswordStoreFactory::GetInstance()->SetTestingFactoryAndUse( profile(), password_manager::BuildPasswordStore< @@ -279,7 +286,7 @@ EXPECT_CALL(*controller(), SavePassword()).Times(0); EXPECT_CALL(*controller(), NeverSavePassword()); model()->OnNeverForThisSiteClicked(); - EXPECT_EQ(password_manager::ui::PENDING_PASSWORD_STATE, model()->state()); + EXPECT_EQ(password_manager::ui::PENDING_PASSWORD_STATE, model()->state()); DestroyModelExpectReason(password_manager::metrics_util::CLICKED_NEVER); } @@ -473,3 +480,117 @@ ManagePasswordsBubbleModelManageLinkTest, ::testing::Values(TestSyncService::SyncedTypes::ALL, TestSyncService::SyncedTypes::NONE)); + +// Verify that URL keyed metrics are properly recorded. +TEST_F(ManagePasswordsBubbleModelTest, RecordUKMs) { + using BubbleDismissalReason = + password_manager::PasswordFormMetricsRecorder::BubbleDismissalReason; + using BubbleTrigger = + password_manager::PasswordFormMetricsRecorder::BubbleTrigger; + using password_manager::metrics_util::CredentialSourceType; + + // |credential_management_api| defines whether credentials originate from the + // credential management API. + for (const bool credential_management_api : {false, true}) { + // |update| defines whether this is an update or a save bubble. + for (const bool update : {false, true}) { + for (const auto interaction : + {BubbleDismissalReason::kAccepted, BubbleDismissalReason::kDeclined, + BubbleDismissalReason::kIgnored}) { + SCOPED_TRACE(testing::Message() + << "update = " << update + << ", interaction = " << static_cast<int64_t>(interaction) + << ", credential management api =" + << credential_management_api); + ukm::TestUkmRecorder test_ukm_recorder; + { + // Setup metrics recorder + ukm::SourceId source_id = test_ukm_recorder.GetNewSourceID(); + static_cast<ukm::UkmRecorder*>(&test_ukm_recorder) + ->UpdateSourceURL(source_id, GURL("https://www.example.com/")); + auto recorder = base::MakeRefCounted< + password_manager::PasswordFormMetricsRecorder>( + true /*is_main_frame_secure*/, + password_manager::PasswordFormMetricsRecorder:: + CreateUkmEntryBuilder(&test_ukm_recorder, source_id)); + + // Exercise bubble. + ON_CALL(*controller(), GetPasswordFormMetricsRecorder()) + .WillByDefault(Return(recorder.get())); + ON_CALL(*controller(), GetCredentialSource()) + .WillByDefault( + Return(credential_management_api + ? CredentialSourceType::kCredentialManagementAPI + : CredentialSourceType::kPasswordManager)); + + if (update) + PretendUpdatePasswordWaiting(); + else + PretendPasswordWaiting(); + + if (interaction == BubbleDismissalReason::kAccepted && update) { + autofill::PasswordForm form; + EXPECT_CALL(*controller(), UpdatePassword(form)); + model()->OnUpdateClicked(form); + } else if (interaction == BubbleDismissalReason::kAccepted && + !update) { + EXPECT_CALL(*GetStore(), + RemoveSiteStatsImpl(GURL(kSiteOrigin).GetOrigin())); + EXPECT_CALL(*controller(), SavePassword()); + model()->OnSaveClicked(); + } else if (interaction == BubbleDismissalReason::kDeclined && + update) { + EXPECT_CALL(*controller(), SavePassword()).Times(0); + model()->OnNopeUpdateClicked(); + } else if (interaction == BubbleDismissalReason::kDeclined && + !update) { + EXPECT_CALL(*GetStore(), + RemoveSiteStatsImpl(GURL(kSiteOrigin).GetOrigin())); + EXPECT_CALL(*controller(), SavePassword()).Times(0); + EXPECT_CALL(*controller(), NeverSavePassword()); + model()->OnNeverForThisSiteClicked(); + } else if (interaction == BubbleDismissalReason::kIgnored && update) { + EXPECT_CALL(*controller(), SavePassword()).Times(0); + EXPECT_CALL(*controller(), NeverSavePassword()).Times(0); + } else if (interaction == BubbleDismissalReason::kIgnored && + !update) { + EXPECT_CALL(*GetStore(), AddSiteStatsImpl(testing::_)); + EXPECT_CALL(*controller(), OnNoInteraction()); + EXPECT_CALL(*controller(), SavePassword()).Times(0); + EXPECT_CALL(*controller(), NeverSavePassword()).Times(0); + } else { + NOTREACHED(); + } + DestroyModel(); + } + // Flush async calls on password store. + base::RunLoop().RunUntilIdle(); + ASSERT_TRUE(testing::Mock::VerifyAndClearExpectations(controller())); + ASSERT_TRUE(testing::Mock::VerifyAndClearExpectations(GetStore())); + + // Verify metrics. + const ukm::UkmSource* source = + test_ukm_recorder.GetSourceForUrl("https://www.example.com/"); + ASSERT_TRUE(source); + test_ukm_recorder.ExpectMetric( + *source, "PasswordForm", + update ? password_manager::kUkmUpdatingPromptShown + : password_manager::kUkmSavingPromptShown, + 1); + test_ukm_recorder.ExpectMetric( + *source, "PasswordForm", + update ? password_manager::kUkmUpdatingPromptTrigger + : password_manager::kUkmSavingPromptTrigger, + static_cast<int64_t>( + credential_management_api + ? BubbleTrigger::kCredentialManagementAPIAutomatic + : BubbleTrigger::kPasswordManagerSuggestionAutomatic)); + test_ukm_recorder.ExpectMetric( + *source, "PasswordForm", + update ? password_manager::kUkmUpdatingPromptInteraction + : password_manager::kUkmSavingPromptInteraction, + static_cast<int64_t>(interaction)); + } + } + } +}
diff --git a/chrome/browser/ui/passwords/manage_passwords_ui_controller.cc b/chrome/browser/ui/passwords/manage_passwords_ui_controller.cc index 25c5cf770..bc4c761 100644 --- a/chrome/browser/ui/passwords/manage_passwords_ui_controller.cc +++ b/chrome/browser/ui/passwords/manage_passwords_ui_controller.cc
@@ -208,6 +208,15 @@ return passwords_data_.origin(); } +password_manager::PasswordFormMetricsRecorder* +ManagePasswordsUIController::GetPasswordFormMetricsRecorder() { + // The form manager may be null for example for auto sign-in toasts of the + // credential manager API. + password_manager::PasswordFormManager* form_manager = + passwords_data_.form_manager(); + return form_manager ? form_manager->metrics_recorder() : nullptr; +} + password_manager::ui::State ManagePasswordsUIController::GetState() const { return passwords_data_.state(); } @@ -226,6 +235,15 @@ return form_manager->pending_credentials(); } +password_manager::metrics_util::CredentialSourceType +ManagePasswordsUIController::GetCredentialSource() const { + password_manager::PasswordFormManager* form_manager = + passwords_data_.form_manager(); + return form_manager + ? form_manager->GetCredentialSource() + : password_manager::metrics_util::CredentialSourceType::kUnknown; +} + bool ManagePasswordsUIController::IsPasswordOverridden() const { const password_manager::PasswordFormManager* form_manager = passwords_data_.form_manager();
diff --git a/chrome/browser/ui/passwords/manage_passwords_ui_controller.h b/chrome/browser/ui/passwords/manage_passwords_ui_controller.h index dfcfa146..f325b3f 100644 --- a/chrome/browser/ui/passwords/manage_passwords_ui_controller.h +++ b/chrome/browser/ui/passwords/manage_passwords_ui_controller.h
@@ -85,8 +85,12 @@ // PasswordsModelDelegate: content::WebContents* GetWebContents() const override; const GURL& GetOrigin() const override; + password_manager::PasswordFormMetricsRecorder* + GetPasswordFormMetricsRecorder() override; password_manager::ui::State GetState() const override; const autofill::PasswordForm& GetPendingPassword() const override; + password_manager::metrics_util::CredentialSourceType GetCredentialSource() + const override; bool IsPasswordOverridden() const override; const std::vector<std::unique_ptr<autofill::PasswordForm>>& GetCurrentForms() const override;
diff --git a/chrome/browser/ui/passwords/passwords_model_delegate.h b/chrome/browser/ui/passwords/passwords_model_delegate.h index c904b2b..c8339c93 100644 --- a/chrome/browser/ui/passwords/passwords_model_delegate.h +++ b/chrome/browser/ui/passwords/passwords_model_delegate.h
@@ -19,7 +19,11 @@ } namespace password_manager { struct InteractionsStats; -} +class PasswordFormMetricsRecorder; +namespace metrics_util { +enum class CredentialSourceType; +} // namespace metrics_util +} // namespace password_manager class GURL; // An interface for ManagePasswordsBubbleModel implemented by @@ -30,6 +34,12 @@ // Returns WebContents* the model is attached to. virtual content::WebContents* GetWebContents() const = 0; + // Returns the password_manager::PasswordFormMetricsRecorder that is + // associated with the PasswordFormManager that governs the password being + // submitted. + virtual password_manager::PasswordFormMetricsRecorder* + GetPasswordFormMetricsRecorder() = 0; + // Returns the URL of the site the current forms are retrieved for. virtual const GURL& GetOrigin() const = 0; @@ -41,6 +51,10 @@ // the returned credential in AUTO_SIGNIN_STATE. virtual const autofill::PasswordForm& GetPendingPassword() const = 0; + // Returns the source of the credential to be saved. + virtual password_manager::metrics_util::CredentialSourceType + GetCredentialSource() const = 0; + // True if the password for previously stored account was overridden, i.e. in // newly submitted form the password is different from stored one. virtual bool IsPasswordOverridden() const = 0;
diff --git a/chrome/browser/ui/passwords/passwords_model_delegate_mock.h b/chrome/browser/ui/passwords/passwords_model_delegate_mock.h index 87bd2a54..6f999293 100644 --- a/chrome/browser/ui/passwords/passwords_model_delegate_mock.h +++ b/chrome/browser/ui/passwords/passwords_model_delegate_mock.h
@@ -18,9 +18,13 @@ ~PasswordsModelDelegateMock() override; MOCK_CONST_METHOD0(GetWebContents, content::WebContents*()); + MOCK_METHOD0(GetPasswordFormMetricsRecorder, + password_manager::PasswordFormMetricsRecorder*()); MOCK_CONST_METHOD0(GetOrigin, const GURL&()); MOCK_CONST_METHOD0(GetState, password_manager::ui::State()); MOCK_CONST_METHOD0(GetPendingPassword, const autofill::PasswordForm&()); + MOCK_CONST_METHOD0(GetCredentialSource, + password_manager::metrics_util::CredentialSourceType()); MOCK_CONST_METHOD0(IsPasswordOverridden, bool()); MOCK_CONST_METHOD0( GetCurrentForms,
diff --git a/components/exo/BUILD.gn b/components/exo/BUILD.gn index 4a4fc560..a496d19d 100644 --- a/components/exo/BUILD.gn +++ b/components/exo/BUILD.gn
@@ -17,6 +17,7 @@ "keyboard.h", "keyboard_delegate.h", "keyboard_device_configuration_delegate.h", + "keyboard_observer.h", "layer_tree_frame_sink_holder.cc", "layer_tree_frame_sink_holder.h", "notification_surface.cc",
diff --git a/components/exo/keyboard.cc b/components/exo/keyboard.cc index fd261e1..f45174b4 100644 --- a/components/exo/keyboard.cc +++ b/components/exo/keyboard.cc
@@ -102,9 +102,8 @@ } Keyboard::~Keyboard() { - delegate_->OnKeyboardDestroying(this); - if (device_configuration_delegate_) - device_configuration_delegate_->OnKeyboardDestroying(this); + for (KeyboardObserver& observer : observer_list_) + observer.OnKeyboardDestroying(this); if (focus_) focus_->RemoveSurfaceObserver(this); auto* helper = WMHelper::GetInstance(); @@ -124,6 +123,30 @@ OnKeyboardDeviceConfigurationChanged(); } +void Keyboard::AddObserver(KeyboardObserver* observer) { + observer_list_.AddObserver(observer); +} + +bool Keyboard::HasObserver(KeyboardObserver* observer) const { + return observer_list_.HasObserver(observer); +} + +void Keyboard::RemoveObserver(KeyboardObserver* observer) { + observer_list_.HasObserver(observer); +} + +void Keyboard::SetNeedKeyboardKeyAcks(bool need_acks) { + are_keyboard_key_acks_needed = need_acks; +} + +bool Keyboard::AreKeyboardKeyAcksNeeded() const { + return are_keyboard_key_acks_needed; +} + +void Keyboard::AckKeyboardKey(uint32_t serial, bool handled) { + // TODO(yhanada): Implement this method. See http://b/28104183. +} + //////////////////////////////////////////////////////////////////////////////// // ui::EventHandler overrides:
diff --git a/components/exo/keyboard.h b/components/exo/keyboard.h index 98c4a4a..86e9ff0 100644 --- a/components/exo/keyboard.h +++ b/components/exo/keyboard.h
@@ -8,6 +8,7 @@ #include <vector> #include "base/macros.h" +#include "components/exo/keyboard_observer.h" #include "components/exo/surface_observer.h" #include "components/exo/wm_helper.h" #include "ui/events/event_handler.h" @@ -37,6 +38,16 @@ void SetDeviceConfigurationDelegate( KeyboardDeviceConfigurationDelegate* delegate); + // Management of the observer list. + void AddObserver(KeyboardObserver* observer); + bool HasObserver(KeyboardObserver* observer) const; + void RemoveObserver(KeyboardObserver* observer); + + void SetNeedKeyboardKeyAcks(bool need_acks); + bool AreKeyboardKeyAcksNeeded() const; + + void AckKeyboardKey(uint32_t serial, bool handled); + // Overridden from ui::EventHandler: void OnKeyEvent(ui::KeyEvent* event) override; @@ -67,6 +78,8 @@ // to. KeyboardDeviceConfigurationDelegate* device_configuration_delegate_ = nullptr; + bool are_keyboard_key_acks_needed = false; + // The current focus surface for the keyboard. Surface* focus_ = nullptr; @@ -76,6 +89,8 @@ // Current set of modifier flags. int modifier_flags_ = 0; + base::ObserverList<KeyboardObserver> observer_list_; + DISALLOW_COPY_AND_ASSIGN(Keyboard); };
diff --git a/components/exo/keyboard_delegate.h b/components/exo/keyboard_delegate.h index 619f56a8..9e73cae 100644 --- a/components/exo/keyboard_delegate.h +++ b/components/exo/keyboard_delegate.h
@@ -14,16 +14,11 @@ } namespace exo { -class Keyboard; class Surface; // Handles events on keyboards in context-specific ways. class KeyboardDelegate { public: - // Called at the top of the keyboard's destructor, to give observers a - // chance to remove themselves. - virtual void OnKeyboardDestroying(Keyboard* keyboard) = 0; - // This should return true if |surface| is a valid target for this keyboard. // E.g. the surface is owned by the same client as the keyboard. virtual bool CanAcceptKeyboardEventsForSurface(Surface* surface) const = 0;
diff --git a/components/exo/keyboard_device_configuration_delegate.h b/components/exo/keyboard_device_configuration_delegate.h index 263be3d6..aa80e4c 100644 --- a/components/exo/keyboard_device_configuration_delegate.h +++ b/components/exo/keyboard_device_configuration_delegate.h
@@ -6,15 +6,10 @@ #define COMPONENTS_EXO_KEYBOARD_DEVICE_CONFIGURATION_H_ namespace exo { -class Keyboard; // Used as an extension to the KeyboardDelegate. class KeyboardDeviceConfigurationDelegate { public: - // Called at the top of the keyboard's destructor, to give observers a chance - // to remove themselves. - virtual void OnKeyboardDestroying(Keyboard* keyboard) = 0; - // Called when used keyboard type changed. virtual void OnKeyboardTypeChanged(bool is_physical) = 0;
diff --git a/components/exo/keyboard_observer.h b/components/exo/keyboard_observer.h new file mode 100644 index 0000000..1264132 --- /dev/null +++ b/components/exo/keyboard_observer.h
@@ -0,0 +1,23 @@ +// Copyright 2017 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. + +#ifndef COMPONENTS_EXO_KEYBOARD_OBSERVER_H_ +#define COMPONENTS_EXO_KEYBOARD_OBSERVER_H_ + +namespace exo { +class Keyboard; + +// Observers to the Keyboard are notified when the Keyboard destructs. +class KeyboardObserver { + public: + virtual ~KeyboardObserver() {} + + // Called at the top of the keyboard's destructor, to give observers a change + // to remove themselves. + virtual void OnKeyboardDestroying(Keyboard* keyboard) = 0; +}; + +} // namespace exo + +#endif // COMPONENTS_EXO_KEYBOARD_OBSERVER_H_
diff --git a/components/exo/keyboard_unittest.cc b/components/exo/keyboard_unittest.cc index 74fed6ea..cc00b4e2 100644 --- a/components/exo/keyboard_unittest.cc +++ b/components/exo/keyboard_unittest.cc
@@ -10,6 +10,7 @@ #include "components/exo/buffer.h" #include "components/exo/keyboard_delegate.h" #include "components/exo/keyboard_device_configuration_delegate.h" +#include "components/exo/keyboard_observer.h" #include "components/exo/shell_surface.h" #include "components/exo/surface.h" #include "components/exo/test/exo_test_base.h" @@ -49,6 +50,14 @@ MOCK_METHOD1(OnKeyboardTypeChanged, void(bool)); }; +class MockKeyboardObserver : public KeyboardObserver { + public: + MockKeyboardObserver() {} + + // Overridden from KeyboardObserver: + MOCK_METHOD1(OnKeyboardDestroying, void(Keyboard*)); +}; + TEST_F(KeyboardTest, OnKeyboardEnter) { std::unique_ptr<Surface> surface(new Surface); std::unique_ptr<ShellSurface> shell_surface(new ShellSurface(surface.get())); @@ -86,7 +95,6 @@ // Surface should maintain keyboard focus when moved to top-level window. focus_client->FocusWindow(surface->window()->GetToplevelWindow()); - EXPECT_CALL(delegate, OnKeyboardDestroying(keyboard.get())); keyboard.reset(); } @@ -116,7 +124,6 @@ EXPECT_CALL(delegate, OnKeyboardLeave(surface.get())); focus_client->FocusWindow(nullptr); - EXPECT_CALL(delegate, OnKeyboardDestroying(keyboard.get())); keyboard.reset(); } @@ -155,7 +162,6 @@ generator.ReleaseKey(ui::VKEY_A, 0); generator.ReleaseKey(ui::VKEY_A, 0); - EXPECT_CALL(delegate, OnKeyboardDestroying(keyboard.get())); keyboard.reset(); } @@ -199,7 +205,6 @@ EXPECT_CALL(delegate, OnKeyboardModifiers(0)); generator.ReleaseKey(ui::VKEY_B, 0); - EXPECT_CALL(delegate, OnKeyboardDestroying(keyboard.get())); keyboard.reset(); } @@ -245,12 +250,57 @@ static_cast<ui::DeviceHotplugEventObserver*>(device_data_manager) ->OnKeyboardDevicesUpdated(keyboards); - EXPECT_CALL(delegate, OnKeyboardDestroying(keyboard.get())); - EXPECT_CALL(configuration_delegate, OnKeyboardDestroying(keyboard.get())); keyboard.reset(); maximize_mode_controller->EnableMaximizeModeWindowManager(false); } +TEST_F(KeyboardTest, KeyboardObserver) { + std::unique_ptr<Surface> surface(new Surface); + std::unique_ptr<ShellSurface> shell_surface(new ShellSurface(surface.get())); + gfx::Size buffer_size(10, 10); + std::unique_ptr<Buffer> buffer( + new Buffer(exo_test_helper()->CreateGpuMemoryBuffer(buffer_size))); + surface->Attach(buffer.get()); + surface->Commit(); + + aura::client::FocusClient* focus_client = + aura::client::GetFocusClient(ash::Shell::GetPrimaryRootWindow()); + focus_client->FocusWindow(nullptr); + + MockKeyboardDelegate delegate; + auto keyboard = base::MakeUnique<Keyboard>(&delegate); + MockKeyboardObserver observer; + keyboard->AddObserver(&observer); + + EXPECT_CALL(observer, OnKeyboardDestroying(keyboard.get())); + keyboard.reset(); +} + +TEST_F(KeyboardTest, NeedKeyboardKeyAcks) { + std::unique_ptr<Surface> surface(new Surface); + std::unique_ptr<ShellSurface> shell_surface(new ShellSurface(surface.get())); + gfx::Size buffer_size(10, 10); + std::unique_ptr<Buffer> buffer( + new Buffer(exo_test_helper()->CreateGpuMemoryBuffer(buffer_size))); + surface->Attach(buffer.get()); + surface->Commit(); + + aura::client::FocusClient* focus_client = + aura::client::GetFocusClient(ash::Shell::GetPrimaryRootWindow()); + focus_client->FocusWindow(nullptr); + + MockKeyboardDelegate delegate; + auto keyboard = base::MakeUnique<Keyboard>(&delegate); + + EXPECT_FALSE(keyboard->AreKeyboardKeyAcksNeeded()); + keyboard->SetNeedKeyboardKeyAcks(true); + EXPECT_TRUE(keyboard->AreKeyboardKeyAcksNeeded()); + keyboard->SetNeedKeyboardKeyAcks(false); + EXPECT_FALSE(keyboard->AreKeyboardKeyAcksNeeded()); + + keyboard.reset(); +} + } // namespace } // namespace exo
diff --git a/components/exo/wayland/BUILD.gn b/components/exo/wayland/BUILD.gn index 55aae92..dc01f94 100644 --- a/components/exo/wayland/BUILD.gn +++ b/components/exo/wayland/BUILD.gn
@@ -40,6 +40,7 @@ "//third_party/wayland-protocols:alpha_compositing_protocol", "//third_party/wayland-protocols:gaming_input_protocol", "//third_party/wayland-protocols:keyboard_configuration_protocol", + "//third_party/wayland-protocols:keyboard_extension_protocol", "//third_party/wayland-protocols:presentation_time_protocol", "//third_party/wayland-protocols:remote_shell_protocol", "//third_party/wayland-protocols:secure_output_protocol",
diff --git a/components/exo/wayland/server.cc b/components/exo/wayland/server.cc index d477e07c..20880dc 100644 --- a/components/exo/wayland/server.cc +++ b/components/exo/wayland/server.cc
@@ -9,6 +9,7 @@ #include <gaming-input-unstable-v2-server-protocol.h> #include <grp.h> #include <keyboard-configuration-unstable-v1-server-protocol.h> +#include <keyboard-extension-unstable-v1-server-protocol.h> #include <linux/input.h> #include <presentation-time-server-protocol.h> #include <remote-shell-unstable-v1-server-protocol.h> @@ -54,6 +55,7 @@ #include "components/exo/keyboard.h" #include "components/exo/keyboard_delegate.h" #include "components/exo/keyboard_device_configuration_delegate.h" +#include "components/exo/keyboard_observer.h" #include "components/exo/notification_surface.h" #include "components/exo/notification_surface_manager.h" #include "components/exo/pointer.h" @@ -2843,11 +2845,13 @@ // Keyboard delegate class that accepts events for surfaces owned by the same // client as a keyboard resource. class WaylandKeyboardDelegate - : public KeyboardDelegate + : public KeyboardDelegate, + public KeyboardObserver #if defined(USE_OZONE) && defined(OS_CHROMEOS) - , public chromeos::input_method::ImeKeyboard::Observer + , + public chromeos::input_method::ImeKeyboard::Observer #endif - { +{ public: explicit WaylandKeyboardDelegate(wl_resource* keyboard_resource) : keyboard_resource_(keyboard_resource), @@ -3133,9 +3137,12 @@ wl_resource* keyboard_resource = wl_resource_create(client, &wl_keyboard_interface, version, id); + WaylandKeyboardDelegate* delegate = + new WaylandKeyboardDelegate(keyboard_resource); + std::unique_ptr<Keyboard> keyboard = base::MakeUnique<Keyboard>(delegate); + keyboard->AddObserver(delegate); SetImplementation(keyboard_resource, &keyboard_implementation, - base::MakeUnique<Keyboard>( - new WaylandKeyboardDelegate(keyboard_resource))); + std::move(keyboard)); // TODO(reveman): Keep repeat info synchronized with chromium and the host OS. if (version >= WL_KEYBOARD_REPEAT_INFO_SINCE_VERSION) @@ -3921,16 +3928,20 @@ // keyboard_device_configuration interface: class WaylandKeyboardDeviceConfigurationDelegate - : public KeyboardDeviceConfigurationDelegate { + : public KeyboardDeviceConfigurationDelegate, + public KeyboardObserver { public: WaylandKeyboardDeviceConfigurationDelegate(wl_resource* resource, Keyboard* keyboard) : resource_(resource), keyboard_(keyboard) { keyboard_->SetDeviceConfigurationDelegate(this); + keyboard_->AddObserver(this); } ~WaylandKeyboardDeviceConfigurationDelegate() override { - if (keyboard_) + if (keyboard_) { keyboard_->SetDeviceConfigurationDelegate(nullptr); + keyboard_->RemoveObserver(this); + } } void OnKeyboardDestroying(Keyboard* keyboard) override { @@ -4083,6 +4094,93 @@ nullptr); } +//////////////////////////////////////////////////////////////////////////////// +// extended_keyboard interface: + +class WaylandExtendedKeyboardImpl : public KeyboardObserver { + public: + WaylandExtendedKeyboardImpl(Keyboard* keyboard) : keyboard_(keyboard) { + keyboard_->AddObserver(this); + keyboard_->SetNeedKeyboardKeyAcks(true); + } + ~WaylandExtendedKeyboardImpl() override { + if (keyboard_) { + keyboard_->RemoveObserver(this); + keyboard_->SetNeedKeyboardKeyAcks(false); + } + } + + // Overridden from KeyboardObserver: + void OnKeyboardDestroying(Keyboard* keyboard) override { + DCHECK(keyboard_ == keyboard); + keyboard_ = nullptr; + } + + void AckKeyboardKey(uint32_t serial, bool handled) { + if (keyboard_) + keyboard_->AckKeyboardKey(serial, handled); + } + + private: + Keyboard* keyboard_; + + DISALLOW_COPY_AND_ASSIGN(WaylandExtendedKeyboardImpl); +}; + +void extended_keyboard_destroy(wl_client* client, wl_resource* resource) { + wl_resource_destroy(resource); +} + +void extended_keyboard_ack_key(wl_client* client, + wl_resource* resource, + uint32_t serial, + uint32_t handled_state) { + GetUserDataAs<WaylandExtendedKeyboardImpl>(resource)->AckKeyboardKey( + serial, handled_state == ZCR_EXTENDED_KEYBOARD_V1_HANDLED_STATE_HANDLED); +} + +const struct zcr_extended_keyboard_v1_interface + extended_keyboard_implementation = {extended_keyboard_destroy, + extended_keyboard_ack_key}; + +//////////////////////////////////////////////////////////////////////////////// +// keyboard_extension interface: + +void keyboard_extension_get_extended_keyboard(wl_client* client, + wl_resource* resource, + uint32_t id, + wl_resource* keyboard_resource) { + Keyboard* keyboard = GetUserDataAs<Keyboard>(keyboard_resource); + if (keyboard->AreKeyboardKeyAcksNeeded()) { + wl_resource_post_error( + resource, ZCR_KEYBOARD_EXTENSION_V1_ERROR_EXTENDED_KEYBOARD_EXISTS, + "keyboard has already been associated with a extended_keyboard object"); + return; + } + + wl_resource* extended_keyboard_resource = + wl_resource_create(client, &zcr_extended_keyboard_v1_interface, 1, id); + + SetImplementation(extended_keyboard_resource, + &extended_keyboard_implementation, + base::MakeUnique<WaylandExtendedKeyboardImpl>(keyboard)); +} + +const struct zcr_keyboard_extension_v1_interface + keyboard_extension_implementation = { + keyboard_extension_get_extended_keyboard}; + +void bind_keyboard_extension(wl_client* client, + void* data, + uint32_t version, + uint32_t id) { + wl_resource* resource = wl_resource_create( + client, &zcr_keyboard_extension_v1_interface, version, id); + + wl_resource_set_implementation(resource, &keyboard_extension_implementation, + data, nullptr); +} + } // namespace //////////////////////////////////////////////////////////////////////////////// @@ -4137,6 +4235,8 @@ 2, display_, bind_keyboard_configuration); wl_global_create(wl_display_.get(), &zcr_stylus_tools_v1_interface, 1, display_, bind_stylus_tools); + wl_global_create(wl_display_.get(), &zcr_keyboard_extension_v1_interface, 1, + display_, bind_keyboard_extension); } Server::~Server() {}
diff --git a/components/password_manager/core/browser/credential_manager_password_form_manager.cc b/components/password_manager/core/browser/credential_manager_password_form_manager.cc index 4406d19..ee3dd044 100644 --- a/components/password_manager/core/browser/credential_manager_password_form_manager.cc +++ b/components/password_manager/core/browser/credential_manager_password_form_manager.cc
@@ -68,6 +68,11 @@ weak_factory_.GetWeakPtr())); } +metrics_util::CredentialSourceType +CredentialManagerPasswordFormManager::GetCredentialSource() { + return metrics_util::CredentialSourceType::kCredentialManagementAPI; +} + void CredentialManagerPasswordFormManager::NotifyDelegate() { delegate_->OnProvisionalSaveComplete(); }
diff --git a/components/password_manager/core/browser/credential_manager_password_form_manager.h b/components/password_manager/core/browser/credential_manager_password_form_manager.h index 4816e17..6f052bb 100644 --- a/components/password_manager/core/browser/credential_manager_password_form_manager.h +++ b/components/password_manager/core/browser/credential_manager_password_form_manager.h
@@ -52,6 +52,8 @@ const std::vector<const autofill::PasswordForm*>& non_federated, size_t filtered_count) override; + metrics_util::CredentialSourceType GetCredentialSource() override; + private: // Calls OnProvisionalSaveComplete on |delegate_|. void NotifyDelegate();
diff --git a/components/password_manager/core/browser/credential_manager_password_form_manager_unittest.cc b/components/password_manager/core/browser/credential_manager_password_form_manager_unittest.cc index 403f8ae..585d88e0 100644 --- a/components/password_manager/core/browser/credential_manager_password_form_manager_unittest.cc +++ b/components/password_manager/core/browser/credential_manager_password_form_manager_unittest.cc
@@ -75,4 +75,18 @@ EXPECT_FALSE(form_manager); } +// Ensure that GetCredentialSource is actually overriden and returns the proper +// value. +TEST_F(CredentialManagerPasswordFormManagerTest, GetCredentialSource) { + PasswordForm observed_form; + MockDelegate delegate; + auto form_manager = base::MakeUnique<CredentialManagerPasswordFormManager>( + &client_, driver_.AsWeakPtr(), observed_form, + base::MakeUnique<PasswordForm>(observed_form), &delegate, + base::MakeUnique<StubFormSaver>(), base::MakeUnique<FakeFormFetcher>()); + form_manager->Init(nullptr); + ASSERT_EQ(metrics_util::CredentialSourceType::kCredentialManagementAPI, + form_manager->GetCredentialSource()); +} + } // namespace password_manager
diff --git a/components/password_manager/core/browser/password_form_manager.cc b/components/password_manager/core/browser/password_form_manager.cc index 3f4c74a..e6677737 100644 --- a/components/password_manager/core/browser/password_form_manager.cc +++ b/components/password_manager/core/browser/password_form_manager.cc
@@ -1361,6 +1361,10 @@ return result; } +metrics_util::CredentialSourceType PasswordFormManager::GetCredentialSource() { + return metrics_util::CredentialSourceType::kPasswordManager; +} + void PasswordFormManager::SendVotesOnSave() { if (observed_form_.IsPossibleChangePasswordFormWithoutUsername()) return;
diff --git a/components/password_manager/core/browser/password_form_manager.h b/components/password_manager/core/browser/password_form_manager.h index 32c6a5ef..2cf48e7 100644 --- a/components/password_manager/core/browser/password_form_manager.h +++ b/components/password_manager/core/browser/password_form_manager.h
@@ -24,6 +24,7 @@ #include "components/password_manager/core/browser/password_form_metrics_recorder.h" #include "components/password_manager/core/browser/password_form_user_action.h" #include "components/password_manager/core/browser/password_manager_driver.h" +#include "components/password_manager/core/browser/password_manager_metrics_util.h" #include "components/password_manager/core/browser/password_store.h" using autofill::FormData; @@ -275,6 +276,10 @@ // adds itself as a consumer of the new one. void GrabFetcher(std::unique_ptr<FormFetcher> fetcher); + PasswordFormMetricsRecorder* metrics_recorder() { + return metrics_recorder_.get(); + } + // Create a copy of |*this| which can be passed to the code handling // save-password related UI. This omits some parts of the internal data, so // the result is not identical to the original. @@ -282,6 +287,11 @@ // another one. std::unique_ptr<PasswordFormManager> Clone(); + // Returns who created this PasswordFormManager. The Credential Management API + // uses a derived class of the PasswordFormManager that can indicate its + // origin. + virtual metrics_util::CredentialSourceType GetCredentialSource(); + protected: // FormFetcher::Consumer: void ProcessMatches(
diff --git a/components/password_manager/core/browser/password_form_manager_unittest.cc b/components/password_manager/core/browser/password_form_manager_unittest.cc index b474e5f..de82490 100644 --- a/components/password_manager/core/browser/password_form_manager_unittest.cc +++ b/components/password_manager/core/browser/password_form_manager_unittest.cc
@@ -3759,8 +3759,7 @@ const auto* source = test_ukm_recorder.GetSourceForUrl(form_to_fill.origin.spec().c_str()); test_ukm_recorder.ExpectMetric(*source, "PasswordForm", - internal::kUkmManagerFillEvent, - test.expected_event); + kUkmManagerFillEvent, test.expected_event); } }
diff --git a/components/password_manager/core/browser/password_form_metrics_recorder.cc b/components/password_manager/core/browser/password_form_metrics_recorder.cc index 60bc42b..3f0ff46f 100644 --- a/components/password_manager/core/browser/password_form_metrics_recorder.cc +++ b/components/password_manager/core/browser/password_form_metrics_recorder.cc
@@ -43,7 +43,7 @@ metrics_util::LogPasswordGenerationAvailableSubmissionEvent( metrics_util::PASSWORD_NOT_SUBMITTED); } - RecordUkmMetric(internal::kUkmSubmissionObserved, 0 /*false*/); + RecordUkmMetric(kUkmSubmissionObserved, 0 /*false*/); } if (submitted_form_type_ != kSubmittedFormTypeUnspecified) { @@ -54,8 +54,11 @@ submitted_form_type_, kSubmittedFormTypeMax); } - RecordUkmMetric(internal::kUkmSubmissionFormType, submitted_form_type_); + RecordUkmMetric(kUkmSubmissionFormType, submitted_form_type_); } + + RecordUkmMetric(kUkmUpdatingPromptShown, update_prompt_shown_); + RecordUkmMetric(kUkmSavingPromptShown, save_prompt_shown_); } // static @@ -112,8 +115,8 @@ } } base::RecordAction(base::UserMetricsAction("PasswordManager_LoginPassed")); - RecordUkmMetric(internal::kUkmSubmissionObserved, 1 /*true*/); - RecordUkmMetric(internal::kUkmSubmissionResult, kSubmitResultPassed); + RecordUkmMetric(kUkmSubmissionObserved, 1 /*true*/); + RecordUkmMetric(kUkmSubmissionResult, kSubmitResultPassed); submit_result_ = kSubmitResultPassed; } @@ -126,8 +129,8 @@ metrics_util::PASSWORD_SUBMISSION_FAILED); } base::RecordAction(base::UserMetricsAction("PasswordManager_LoginFailed")); - RecordUkmMetric(internal::kUkmSubmissionObserved, 1 /*true*/); - RecordUkmMetric(internal::kUkmSubmissionResult, kSubmitResultFailed); + RecordUkmMetric(kUkmSubmissionObserved, 1 /*true*/); + RecordUkmMetric(kUkmSubmissionResult, kSubmitResultFailed); submit_result_ = kSubmitResultFailed; } @@ -223,8 +226,111 @@ RecordUkmMetric("SuppressedAccount.Manual.SameOrganizationName", best_match); } +void PasswordFormMetricsRecorder::RecordPasswordBubbleShown( + metrics_util::CredentialSourceType credential_source_type, + metrics_util::UIDisplayDisposition display_disposition) { + if (credential_source_type == metrics_util::CredentialSourceType::kUnknown) + return; + BubbleTrigger automatic_trigger_type = + credential_source_type == + metrics_util::CredentialSourceType::kPasswordManager + ? BubbleTrigger::kPasswordManagerSuggestionAutomatic + : BubbleTrigger::kCredentialManagementAPIAutomatic; + BubbleTrigger manual_trigger_type = + credential_source_type == + metrics_util::CredentialSourceType::kPasswordManager + ? BubbleTrigger::kPasswordManagerSuggestionManual + : BubbleTrigger::kCredentialManagementAPIManual; + + switch (display_disposition) { + // New credential cases: + case metrics_util::AUTOMATIC_WITH_PASSWORD_PENDING: + save_prompt_shown_ = true; + RecordUkmMetric(kUkmSavingPromptTrigger, + static_cast<int64_t>(automatic_trigger_type)); + break; + case metrics_util::MANUAL_WITH_PASSWORD_PENDING: + save_prompt_shown_ = true; + RecordUkmMetric(kUkmSavingPromptTrigger, + static_cast<int64_t>(manual_trigger_type)); + break; + + // Update cases: + case metrics_util::AUTOMATIC_WITH_PASSWORD_PENDING_UPDATE: + update_prompt_shown_ = true; + RecordUkmMetric(kUkmUpdatingPromptTrigger, + static_cast<int64_t>(automatic_trigger_type)); + break; + case metrics_util::MANUAL_WITH_PASSWORD_PENDING_UPDATE: + update_prompt_shown_ = true; + RecordUkmMetric(kUkmUpdatingPromptTrigger, + static_cast<int64_t>(manual_trigger_type)); + break; + + // Other reasons to show a bubble: + case metrics_util::MANUAL_MANAGE_PASSWORDS: + case metrics_util::AUTOMATIC_GENERATED_PASSWORD_CONFIRMATION: + case metrics_util::AUTOMATIC_SIGNIN_TOAST: + // Do nothing. + return; + + // Obsolte display dispositions: + case metrics_util::MANUAL_BLACKLISTED_OBSOLETE: + case metrics_util::AUTOMATIC_CREDENTIAL_REQUEST_OBSOLETE: + case metrics_util::NUM_DISPLAY_DISPOSITIONS: + NOTREACHED(); + return; + } +} + +void PasswordFormMetricsRecorder::RecordUIDismissalReason( + metrics_util::UIDismissalReason ui_dismissal_reason) { + DCHECK(!(update_prompt_shown_ && save_prompt_shown_)); + if (!(update_prompt_shown_ || save_prompt_shown_)) + return; + const char* metric = update_prompt_shown_ ? kUkmUpdatingPromptInteraction + : kUkmSavingPromptInteraction; + switch (ui_dismissal_reason) { + // Accepted by user. + case metrics_util::CLICKED_SAVE: + RecordUkmMetric(metric, + static_cast<int64_t>(BubbleDismissalReason::kAccepted)); + break; + + // Declined by user. + case metrics_util::CLICKED_CANCEL: + case metrics_util::CLICKED_NEVER: + RecordUkmMetric(metric, + static_cast<int64_t>(BubbleDismissalReason::kDeclined)); + break; + + // Ignored by user. + case metrics_util::NO_DIRECT_INTERACTION: + RecordUkmMetric(metric, + static_cast<int64_t>(BubbleDismissalReason::kIgnored)); + break; + + // Ignore these for metrics collection: + case metrics_util::CLICKED_MANAGE: + case metrics_util::CLICKED_DONE: + case metrics_util::CLICKED_OK: + case metrics_util::CLICKED_BRAND_NAME: + case metrics_util::CLICKED_PASSWORDS_DASHBOARD: + case metrics_util::AUTO_SIGNIN_TOAST_TIMEOUT: + break; + + // These should not reach here: + case metrics_util::CLICKED_UNBLACKLIST_OBSOLETE: + case metrics_util::CLICKED_CREDENTIAL_OBSOLETE: + case metrics_util::AUTO_SIGNIN_TOAST_CLICKED_OBSOLETE: + case metrics_util::NUM_UI_RESPONSES: + NOTREACHED(); + break; + } +} + void PasswordFormMetricsRecorder::RecordFillEvent(ManagerAutofillEvent event) { - RecordUkmMetric(internal::kUkmManagerFillEvent, event); + RecordUkmMetric(kUkmManagerFillEvent, event); } PasswordFormMetricsRecorder::SuppressedAccountExistence
diff --git a/components/password_manager/core/browser/password_form_metrics_recorder.h b/components/password_manager/core/browser/password_form_metrics_recorder.h index f1c5ceb..63896bc 100644 --- a/components/password_manager/core/browser/password_form_metrics_recorder.h +++ b/components/password_manager/core/browser/password_form_metrics_recorder.h
@@ -14,13 +14,12 @@ #include "base/memory/ref_counted.h" #include "components/autofill/core/common/password_form.h" #include "components/password_manager/core/browser/password_form_user_action.h" +#include "components/password_manager/core/browser/password_manager_metrics_util.h" #include "services/metrics/public/cpp/ukm_recorder.h" namespace password_manager { -// Internal namespace is intended for component wide access only. -namespace internal { -// UKM Metric names. Exposed in internal namespace for unittesting. +// URL Keyed Metrics. // This metric records whether a submission of a password form has been // observed. The values 0 and 1 correspond to false and true respectively. @@ -36,10 +35,36 @@ // Note that no metric is recorded for kSubmittedFormTypeUnspecified. constexpr char kUkmSubmissionFormType[] = "Submission.SubmittedFormType"; +// This metric records the boolean value indicating whether a password update +// prompt was shown, which asked the user for permission to update a password. +constexpr char kUkmUpdatingPromptShown[] = "Updating.Prompt.Shown"; + +// This metric records the reason why a password update prompt was shown to ask +// the user for permission to update a password. The values correspond to +// PasswordFormMetricsRecorder::BubbleTrigger. +constexpr char kUkmUpdatingPromptTrigger[] = "Updating.Prompt.Trigger"; + +// This metric records how a user interacted with an updating prompt. The values +// correspond to PasswordFormMetricsRecorder::BubbleDismissalReason. +constexpr char kUkmUpdatingPromptInteraction[] = "Updating.Prompt.Interaction"; + +// This metric records the boolean value indicating whether a password save +// prompt was shown, which asked the user for permission to save a new +// credential. +constexpr char kUkmSavingPromptShown[] = "Saving.Prompt.Shown"; + +// This metric records the reason why a password save prompt was shown to ask +// the user for permission to save a new credential. The values correspond to +// PasswordFormMetricsRecorder::BubbleTrigger. +constexpr char kUkmSavingPromptTrigger[] = "Saving.Prompt.Trigger"; + +// This metric records how a user interacted with a saving prompt. The values +// correspond to PasswordFormMetricsRecorder::BubbleDismissalReason. +constexpr char kUkmSavingPromptInteraction[] = "Saving.Prompt.Interaction"; + // This metric records attempts to fill a password form. Values correspond to // PasswordFormMetricsRecorder::ManagerFillEvent. constexpr char kUkmManagerFillEvent[] = "ManagerFill.Action"; -} // namespace internal class FormFetcher; @@ -147,6 +172,28 @@ kSubmittedFormTypeMax }; + // The reason why a password bubble was shown on the screen. + enum class BubbleTrigger { + kUnknown = 0, + // The password manager suggests the user to save a password and asks for + // confirmation. + kPasswordManagerSuggestionAutomatic, + kPasswordManagerSuggestionManual, + // The site asked the user to save a password via the credential management + // API. + kCredentialManagementAPIAutomatic, + kCredentialManagementAPIManual, + kMax, + }; + + // The reason why a password bubble was dismissed. + enum class BubbleDismissalReason { + kUnknown = 0, + kAccepted = 1, + kDeclined = 2, + kIgnored = 3 + }; + // The maximum number of combinations of the ManagerAction, UserAction and // SubmitResult enums. // This is used when recording the actions taken by the form in UMA. @@ -194,6 +241,15 @@ const FormFetcher& form_fetcher, const autofill::PasswordForm& pending_credentials); + // Records the event that a password bubble was shown. + void RecordPasswordBubbleShown( + metrics_util::CredentialSourceType credential_source_type, + metrics_util::UIDisplayDisposition display_disposition); + + // Records the dismissal of a password bubble. + void RecordUIDismissalReason( + metrics_util::UIDismissalReason ui_dismissal_reason); + // Records that the password manager managed or failed to fill a form. void RecordFillEvent(ManagerAutofillEvent event); @@ -249,6 +305,12 @@ // Whether this form has an auto generated password. bool has_generated_password_ = false; + // Whether the user was shown a prompt to update a password. + bool update_prompt_shown_ = false; + + // Whether the user was shown a prompt to save a new credential. + bool save_prompt_shown_ = false; + // These three fields record the "ActionsTaken" by the browser and // the user with this form, and the result. They are combined and // recorded in UMA when the PasswordFormMetricsRecorder is destroyed.
diff --git a/components/password_manager/core/browser/password_form_metrics_recorder_unittest.cc b/components/password_manager/core/browser/password_form_metrics_recorder_unittest.cc index b93956e8..e97174c 100644 --- a/components/password_manager/core/browser/password_form_metrics_recorder_unittest.cc +++ b/components/password_manager/core/browser/password_form_metrics_recorder_unittest.cc
@@ -31,6 +31,7 @@ source_id); } +// TODO(crbug.com/738921) Replace this with generalized infrastructure. // Verifies that the metric |metric_name| was recorded with value |value| in the // single entry of |test_ukm_recorder_| exactly |expected_count| times. void ExpectUkmValueCount(ukm::TestUkmRecorder* test_ukm_recorder, @@ -108,7 +109,7 @@ } ExpectUkmValueCount( - &test_ukm_recorder, internal::kUkmSubmissionObserved, + &test_ukm_recorder, kUkmSubmissionObserved, test.submission != PasswordFormMetricsRecorder::kSubmitResultNotSubmitted ? 1 @@ -120,7 +121,7 @@ : 0; EXPECT_EQ(expected_login_failed, user_action_tester.GetActionCount("PasswordManager_LoginFailed")); - ExpectUkmValueCount(&test_ukm_recorder, internal::kUkmSubmissionResult, + ExpectUkmValueCount(&test_ukm_recorder, kUkmSubmissionResult, PasswordFormMetricsRecorder::kSubmitResultFailed, expected_login_failed); @@ -129,7 +130,7 @@ : 0; EXPECT_EQ(expected_login_passed, user_action_tester.GetActionCount("PasswordManager_LoginPassed")); - ExpectUkmValueCount(&test_ukm_recorder, internal::kUkmSubmissionResult, + ExpectUkmValueCount(&test_ukm_recorder, kUkmSubmissionResult, PasswordFormMetricsRecorder::kSubmitResultPassed, expected_login_passed); @@ -359,7 +360,7 @@ if (test.form_type != PasswordFormMetricsRecorder::kSubmittedFormTypeUnspecified) { - ExpectUkmValueCount(&test_ukm_recorder, internal::kUkmSubmissionFormType, + ExpectUkmValueCount(&test_ukm_recorder, kUkmSubmissionFormType, test.form_type, 1); } @@ -382,4 +383,136 @@ } } +TEST(PasswordFormMetricsRecorder, RecordPasswordBubbleShown) { + using Trigger = PasswordFormMetricsRecorder::BubbleTrigger; + static constexpr struct { + // Stimuli: + metrics_util::CredentialSourceType credential_source_type; + metrics_util::UIDisplayDisposition display_disposition; + // Expectations: + const char* expected_trigger_metric; + Trigger expected_trigger_value; + bool expected_save_prompt_shown; + bool expected_update_prompt_shown; + } kTests[] = { + // Source = PasswordManager, Saving. + {metrics_util::CredentialSourceType::kPasswordManager, + metrics_util::AUTOMATIC_WITH_PASSWORD_PENDING, kUkmSavingPromptTrigger, + Trigger::kPasswordManagerSuggestionAutomatic, true, false}, + {metrics_util::CredentialSourceType::kPasswordManager, + metrics_util::MANUAL_WITH_PASSWORD_PENDING, kUkmSavingPromptTrigger, + Trigger::kPasswordManagerSuggestionManual, true, false}, + // Source = PasswordManager, Updating. + {metrics_util::CredentialSourceType::kPasswordManager, + metrics_util::AUTOMATIC_WITH_PASSWORD_PENDING_UPDATE, + kUkmUpdatingPromptTrigger, Trigger::kPasswordManagerSuggestionAutomatic, + false, true}, + {metrics_util::CredentialSourceType::kPasswordManager, + metrics_util::MANUAL_WITH_PASSWORD_PENDING_UPDATE, + kUkmUpdatingPromptTrigger, Trigger::kPasswordManagerSuggestionManual, + false, true}, + // Source = Credential Management API, Saving. + {metrics_util::CredentialSourceType::kCredentialManagementAPI, + metrics_util::AUTOMATIC_WITH_PASSWORD_PENDING, kUkmSavingPromptTrigger, + Trigger::kCredentialManagementAPIAutomatic, true, false}, + {metrics_util::CredentialSourceType::kCredentialManagementAPI, + metrics_util::MANUAL_WITH_PASSWORD_PENDING, kUkmSavingPromptTrigger, + Trigger::kCredentialManagementAPIManual, true, false}, + // Source = Credential Management API, Updating. + {metrics_util::CredentialSourceType::kCredentialManagementAPI, + metrics_util::AUTOMATIC_WITH_PASSWORD_PENDING_UPDATE, + kUkmUpdatingPromptTrigger, Trigger::kCredentialManagementAPIAutomatic, + false, true}, + {metrics_util::CredentialSourceType::kCredentialManagementAPI, + metrics_util::MANUAL_WITH_PASSWORD_PENDING_UPDATE, + kUkmUpdatingPromptTrigger, Trigger::kCredentialManagementAPIManual, + false, true}, + // Source = Unknown, Saving. + {metrics_util::CredentialSourceType::kUnknown, + metrics_util::AUTOMATIC_WITH_PASSWORD_PENDING, kUkmSavingPromptTrigger, + Trigger::kPasswordManagerSuggestionAutomatic, false, false}, + }; + + for (const auto& test : kTests) { + SCOPED_TRACE(testing::Message() + << "credential_source_type = " + << static_cast<int64_t>(test.credential_source_type) + << ", display_disposition = " << test.display_disposition); + ukm::TestUkmRecorder test_ukm_recorder; + { + auto recorder = base::MakeRefCounted<PasswordFormMetricsRecorder>( + true /*is_main_frame_secure*/, + CreateUkmEntryBuilder(&test_ukm_recorder)); + recorder->RecordPasswordBubbleShown(test.credential_source_type, + test.display_disposition); + } + // Verify data + const ukm::UkmSource* source = test_ukm_recorder.GetSourceForUrl(kTestUrl); + ASSERT_TRUE(source); + if (test.credential_source_type != + metrics_util::CredentialSourceType::kUnknown) { + test_ukm_recorder.ExpectMetric( + *source, "PasswordForm", test.expected_trigger_metric, + static_cast<int64_t>(test.expected_trigger_value)); + } else { + EXPECT_FALSE(test_ukm_recorder.HasMetric(*source, "PasswordForm", + kUkmSavingPromptTrigger)); + EXPECT_FALSE(test_ukm_recorder.HasMetric(*source, "PasswordForm", + kUkmUpdatingPromptTrigger)); + } + test_ukm_recorder.ExpectMetric(*source, "PasswordForm", + kUkmSavingPromptShown, + test.expected_save_prompt_shown); + test_ukm_recorder.ExpectMetric(*source, "PasswordForm", + kUkmUpdatingPromptShown, + test.expected_update_prompt_shown); + } +} + +TEST(PasswordFormMetricsRecorder, RecordUIDismissalReason) { + static constexpr struct { + // Stimuli: + metrics_util::UIDisplayDisposition display_disposition; + metrics_util::UIDismissalReason dismissal_reason; + // Expectations: + const char* expected_trigger_metric; + PasswordFormMetricsRecorder::BubbleDismissalReason expected_metric_value; + } kTests[] = { + {metrics_util::AUTOMATIC_WITH_PASSWORD_PENDING, + metrics_util::CLICKED_SAVE, kUkmSavingPromptInteraction, + PasswordFormMetricsRecorder::BubbleDismissalReason::kAccepted}, + {metrics_util::MANUAL_WITH_PASSWORD_PENDING, metrics_util::CLICKED_CANCEL, + kUkmSavingPromptInteraction, + PasswordFormMetricsRecorder::BubbleDismissalReason::kDeclined}, + {metrics_util::AUTOMATIC_WITH_PASSWORD_PENDING_UPDATE, + metrics_util::CLICKED_NEVER, kUkmUpdatingPromptInteraction, + PasswordFormMetricsRecorder::BubbleDismissalReason::kDeclined}, + {metrics_util::MANUAL_WITH_PASSWORD_PENDING_UPDATE, + metrics_util::NO_DIRECT_INTERACTION, kUkmUpdatingPromptInteraction, + PasswordFormMetricsRecorder::BubbleDismissalReason::kIgnored}, + }; + + for (const auto& test : kTests) { + SCOPED_TRACE(testing::Message() + << "display_disposition = " << test.display_disposition + << ", dismissal_reason = " << test.dismissal_reason); + ukm::TestUkmRecorder test_ukm_recorder; + { + auto recorder = base::MakeRefCounted<PasswordFormMetricsRecorder>( + true /*is_main_frame_secure*/, + CreateUkmEntryBuilder(&test_ukm_recorder)); + recorder->RecordPasswordBubbleShown( + metrics_util::CredentialSourceType::kPasswordManager, + test.display_disposition); + recorder->RecordUIDismissalReason(test.dismissal_reason); + } + // Verify data + const ukm::UkmSource* source = test_ukm_recorder.GetSourceForUrl(kTestUrl); + ASSERT_TRUE(source); + test_ukm_recorder.ExpectMetric( + *source, "PasswordForm", test.expected_trigger_metric, + static_cast<int64_t>(test.expected_metric_value)); + } +} + } // namespace password_manager
diff --git a/components/password_manager/core/browser/password_manager_metrics_recorder.h b/components/password_manager/core/browser/password_manager_metrics_recorder.h index ea9b816..4d2a7f70 100644 --- a/components/password_manager/core/browser/password_manager_metrics_recorder.h +++ b/components/password_manager/core/browser/password_manager_metrics_recorder.h
@@ -16,13 +16,13 @@ namespace password_manager { -// Internal namespace is intended for component wide access only. -namespace internal { +// URL Keyed Metrics. +// Records a 1 for every page on which a user has modified the content of a +// password field - regardless of how many password fields a page contains or +// the user modifies. constexpr char kUkmUserModifiedPasswordField[] = "UserModifiedPasswordField"; -} // namespace internal - class BrowserSavePasswordProgressLogger; // The pupose of this class is to record various types of metrics about the
diff --git a/components/password_manager/core/browser/password_manager_metrics_recorder_unittest.cc b/components/password_manager/core/browser/password_manager_metrics_recorder_unittest.cc index 89874c40..9cb4e2f 100644 --- a/components/password_manager/core/browser/password_manager_metrics_recorder_unittest.cc +++ b/components/password_manager/core/browser/password_manager_metrics_recorder_unittest.cc
@@ -27,6 +27,7 @@ test_ukm_recorder, source_id); } +// TODO(crbug.com/738921) Replace this with generalized infrastructure. // Verifies that the metric |metric_name| was recorded with value |value| in the // single entry of |test_ukm_recorder| exactly |expected_count| times. void ExpectUkmValueCount(ukm::TestUkmRecorder* test_ukm_recorder, @@ -57,8 +58,7 @@ CreateUkmEntryBuilder(&test_ukm_recorder)); recorder.RecordUserModifiedPasswordField(); } - ExpectUkmValueCount(&test_ukm_recorder, - internal::kUkmUserModifiedPasswordField, 1, 1); + ExpectUkmValueCount(&test_ukm_recorder, kUkmUserModifiedPasswordField, 1, 1); } TEST(PasswordManagerMetricsRecorder, UserModifiedPasswordFieldMultipleTimes) { @@ -70,8 +70,7 @@ recorder.RecordUserModifiedPasswordField(); recorder.RecordUserModifiedPasswordField(); } - ExpectUkmValueCount(&test_ukm_recorder, - internal::kUkmUserModifiedPasswordField, 1, 1); + ExpectUkmValueCount(&test_ukm_recorder, kUkmUserModifiedPasswordField, 1, 1); } TEST(PasswordManagerMetricsRecorder, UserModifiedPasswordFieldNotCalled) { @@ -80,8 +79,7 @@ PasswordManagerMetricsRecorder recorder( CreateUkmEntryBuilder(&test_ukm_recorder)); } - ExpectUkmValueCount(&test_ukm_recorder, - internal::kUkmUserModifiedPasswordField, 1, 0); + ExpectUkmValueCount(&test_ukm_recorder, kUkmUserModifiedPasswordField, 1, 0); } } // namespace password_manager
diff --git a/components/password_manager/core/browser/password_manager_metrics_util.h b/components/password_manager/core/browser/password_manager_metrics_util.h index 909a914..2f2ff18 100644 --- a/components/password_manager/core/browser/password_manager_metrics_util.h +++ b/components/password_manager/core/browser/password_manager_metrics_util.h
@@ -208,6 +208,17 @@ REAUTH_COUNT }; +// Specifies the type of PasswordFormManagers and derived classes to distinguish +// the context in which a PasswordFormManager is being created and used. +enum class CredentialSourceType { + kUnknown, + // This is used for form based credential management (PasswordFormManager). + kPasswordManager, + // This is used for credential management API based credential management + // (CredentialManagerPasswordFormManager). + kCredentialManagementAPI +}; + #if defined(OS_WIN) || (defined(OS_MACOSX) && !defined(OS_IOS)) || \ (defined(OS_LINUX) && !defined(OS_CHROMEOS)) enum class SyncPasswordHashChange {
diff --git a/headless/lib/browser/devtools_api/devtools_connection.js b/headless/lib/browser/devtools_api/devtools_connection.js index 3061c196..07b9741a 100644 --- a/headless/lib/browser/devtools_api/devtools_connection.js +++ b/headless/lib/browser/devtools_api/devtools_connection.js
@@ -56,6 +56,14 @@ */ this.eventListenerIdToEventName_ = new Map(); + /** + * An object containing listeners for all DevTools protocol events keyed + * by listener id. + * + * @private {!Map<number, !chromium.DevTools.Connection.AllEventsFunction>} + */ + this.allEventsListeners_ = new Map(); + this.transport_.onmessage = this.onJsonMessage_.bind(this); } @@ -97,6 +105,31 @@ return this.eventListeners_.get(eventName).delete(id); } + /** + * Listens for all DevTools protocol events and issues the + * callback upon reception. + * + * @param {!chromium.DevTools.Connection.AllEventsFunction} listener The + * callback issued when we receive a DevTools protocol event. + * @return {number} The id of this event listener. + */ + addAllEventsListener(listener) { + let id = this.nextListenerId_++; + this.allEventsListeners_.set(id, listener); + return id; + } + + /** + * Removes an event listener previously added by + * <code>addAllEventsListener</code>. + * + * @param {number} id The id of the event listener to remove. + * @return {boolean} Whether the event listener was actually removed. + */ + removeAllEventsListener(id) { + if (!this.allEventsListeners_.has(id)) return false; + return this.allEventsListeners_.delete(id); + } /** * Issues a DevTools protocol command and returns a promise for the results. @@ -124,15 +157,12 @@ } /** - * If a subclass needs to customize message handling it should override this - * method. - * - * @param {*} message A parsed DevTools protocol message. * @param {string} jsonMessage A string containing a JSON DevTools protocol * message. - * @protected + * @private */ - onMessage_(message, jsonMessage) { + onJsonMessage_(jsonMessage) { + let message = JSON.parse(jsonMessage); if (message.hasOwnProperty('id')) { if (!this.pendingCommands_.has(message.id)) throw new Error('Unrecognized id:' + jsonMessage); @@ -148,6 +178,9 @@ } const method = message['method']; const params = message['params']; + this.allEventsListeners_.forEach(function(listener) { + listener({method, params}); + }); if (this.eventListeners_.has(method)) { this.eventListeners_.get(method).forEach(function(listener) { listener(params); @@ -155,16 +188,6 @@ } } } - - - /** - * @param {string} jsonMessage A string containing a JSON DevTools protocol - * message. - * @private - */ - onJsonMessage_(jsonMessage) { - this.onMessage_(JSON.parse(jsonMessage), jsonMessage); - } } /** @@ -173,6 +196,11 @@ chromium.DevTools.Connection.EventFunction; /** + * @typedef {function(Object): undefined} + */ +chromium.DevTools.Connection.AllEventsFunction; + +/** * @typedef {{ * resolve: function(!Object), * reject: function(!Object)
diff --git a/ios/chrome/browser/passwords/ios_chrome_password_manager_infobar_delegate.h b/ios/chrome/browser/passwords/ios_chrome_password_manager_infobar_delegate.h index afc444a..7a0035ea 100644 --- a/ios/chrome/browser/passwords/ios_chrome_password_manager_infobar_delegate.h +++ b/ios/chrome/browser/passwords/ios_chrome_password_manager_infobar_delegate.h
@@ -41,6 +41,10 @@ infobar_response_ = response; } + password_manager::metrics_util::UIDismissalReason infobar_response() const { + return infobar_response_; + } + private: // ConfirmInfoBarDelegate implementation. Type GetInfoBarType() const override;
diff --git a/ios/chrome/browser/passwords/ios_chrome_save_password_infobar_delegate.mm b/ios/chrome/browser/passwords/ios_chrome_save_password_infobar_delegate.mm index c0e55e5..e9838a9 100644 --- a/ios/chrome/browser/passwords/ios_chrome_save_password_infobar_delegate.mm +++ b/ios/chrome/browser/passwords/ios_chrome_save_password_infobar_delegate.mm
@@ -34,13 +34,20 @@ infobar_manager->CreateConfirmInfoBar(std::move(delegate))); } -IOSChromeSavePasswordInfoBarDelegate::~IOSChromeSavePasswordInfoBarDelegate() {} +IOSChromeSavePasswordInfoBarDelegate::~IOSChromeSavePasswordInfoBarDelegate() { + form_to_save()->metrics_recorder()->RecordUIDismissalReason( + infobar_response()); +} IOSChromeSavePasswordInfoBarDelegate::IOSChromeSavePasswordInfoBarDelegate( bool is_smart_lock_branding_enabled, - std::unique_ptr<PasswordFormManager> form_to_save) + std::unique_ptr<PasswordFormManager> form_manager) : IOSChromePasswordManagerInfoBarDelegate(is_smart_lock_branding_enabled, - std::move(form_to_save)) {} + std::move(form_manager)) { + form_to_save()->metrics_recorder()->RecordPasswordBubbleShown( + form_to_save()->GetCredentialSource(), + password_manager::metrics_util::AUTOMATIC_WITH_PASSWORD_PENDING); +} infobars::InfoBarDelegate::InfoBarIdentifier IOSChromeSavePasswordInfoBarDelegate::GetIdentifier() const {
diff --git a/ios/chrome/browser/passwords/ios_chrome_update_password_infobar_delegate.h b/ios/chrome/browser/passwords/ios_chrome_update_password_infobar_delegate.h index d720ba2..bf5e790 100644 --- a/ios/chrome/browser/passwords/ios_chrome_update_password_infobar_delegate.h +++ b/ios/chrome/browser/passwords/ios_chrome_update_password_infobar_delegate.h
@@ -61,6 +61,7 @@ int GetButtons() const override; base::string16 GetButtonLabel(InfoBarButton button) const override; bool Accept() override; + bool Cancel() override; // The credential that should be displayed in the infobar, and for which the // password will be updated.
diff --git a/ios/chrome/browser/passwords/ios_chrome_update_password_infobar_delegate.mm b/ios/chrome/browser/passwords/ios_chrome_update_password_infobar_delegate.mm index 44a786bafd..e015593 100644 --- a/ios/chrome/browser/passwords/ios_chrome_update_password_infobar_delegate.mm +++ b/ios/chrome/browser/passwords/ios_chrome_update_password_infobar_delegate.mm
@@ -42,7 +42,10 @@ } IOSChromeUpdatePasswordInfoBarDelegate:: - ~IOSChromeUpdatePasswordInfoBarDelegate() {} + ~IOSChromeUpdatePasswordInfoBarDelegate() { + form_to_save()->metrics_recorder()->RecordUIDismissalReason( + infobar_response()); +} IOSChromeUpdatePasswordInfoBarDelegate::IOSChromeUpdatePasswordInfoBarDelegate( bool is_smart_lock_branding_enabled, @@ -50,6 +53,9 @@ : IOSChromePasswordManagerInfoBarDelegate(is_smart_lock_branding_enabled, std::move(form_manager)) { selected_account_ = form_to_save()->preferred_match()->username_value; + form_to_save()->metrics_recorder()->RecordPasswordBubbleShown( + form_to_save()->GetCredentialSource(), + password_manager::metrics_util::AUTOMATIC_WITH_PASSWORD_PENDING_UPDATE); } bool IOSChromeUpdatePasswordInfoBarDelegate::ShowMultipleAccounts() const { @@ -106,5 +112,12 @@ } else { form_to_save()->Update(form_to_save()->pending_credentials()); } + set_infobar_response(password_manager::metrics_util::CLICKED_SAVE); + return true; +} + +bool IOSChromeUpdatePasswordInfoBarDelegate::Cancel() { + DCHECK(form_to_save()); + set_infobar_response(password_manager::metrics_util::CLICKED_CANCEL); return true; }
diff --git a/third_party/WebKit/LayoutTests/TestExpectations b/third_party/WebKit/LayoutTests/TestExpectations index 636b2a9..e235ff5 100644 --- a/third_party/WebKit/LayoutTests/TestExpectations +++ b/third_party/WebKit/LayoutTests/TestExpectations
@@ -2478,11 +2478,6 @@ crbug.com/595993 virtual/off-main-thread-fetch/external/wpt/service-workers/service-worker/request-end-to-end.https.html [ Failure ] crbug.com/595993 virtual/service-worker-script-streaming/external/wpt/service-workers/service-worker/request-end-to-end.https.html [ Failure ] -crbug.com/735883 external/wpt/service-workers/service-worker/fetch-canvas-tainting.https.html [ Pass Failure ] -crbug.com/735883 virtual/off-main-thread-fetch/external/wpt/service-workers/service-worker/fetch-canvas-tainting.https.html [ Pass Failure ] -crbug.com/735883 external/wpt/service-workers/service-worker/fetch-canvas-tainting-cache.https.html [ Pass Failure ] -crbug.com/735883 virtual/off-main-thread-fetch/external/wpt/service-workers/service-worker/fetch-canvas-tainting-cache.https.html [ Pass Failure ] - crbug.com/619427 [ Mac Linux ] fast/overflow/overflow-height-float-not-removed-crash3.html [ Pass Failure ] crbug.com/667371 inspector/elements/styles-1/color-aware-property-value-edit.html [ Pass Failure ]
diff --git a/third_party/WebKit/LayoutTests/editing/undo/undo-combined-delete-expected.txt b/third_party/WebKit/LayoutTests/editing/undo/undo-combined-delete-expected.txt deleted file mode 100644 index 750acc7..0000000 --- a/third_party/WebKit/LayoutTests/editing/undo/undo-combined-delete-expected.txt +++ /dev/null
@@ -1,28 +0,0 @@ -mac -PASS selection.anchorNode.data is "This word should be selected only on mac." -PASS selection.anchorOffset is 5 -PASS selection.focusNode.data is "This word should be selected only on mac." -PASS selection.focusOffset is 9 -PASS selection.toString() is "word" -win -PASS selection.anchorNode.data is "This word should be selected only on mac." -PASS selection.anchorOffset is 7 -PASS selection.focusNode.data is "This word should be selected only on mac." -PASS selection.focusOffset is 7 -PASS selection.toString() is "" -unix -PASS selection.anchorNode.data is "This word should be selected only on mac." -PASS selection.anchorOffset is 7 -PASS selection.focusNode.data is "This word should be selected only on mac." -PASS selection.focusOffset is 7 -PASS selection.toString() is "" -android -PASS selection.anchorNode.data is "This word should be selected only on mac." -PASS selection.anchorOffset is 7 -PASS selection.focusNode.data is "This word should be selected only on mac." -PASS selection.focusOffset is 7 -PASS selection.toString() is "" -PASS successfullyParsed is true - -TEST COMPLETE -
diff --git a/third_party/WebKit/LayoutTests/editing/undo/undo-combined-delete.html b/third_party/WebKit/LayoutTests/editing/undo/undo-combined-delete.html deleted file mode 100644 index 0a6af3c..0000000 --- a/third_party/WebKit/LayoutTests/editing/undo/undo-combined-delete.html +++ /dev/null
@@ -1,47 +0,0 @@ -<!DOCTYPE html> -<html> -<body> -<div id="container"> -<p id="description"></p> -<p>To test manually, place the cursor after 'o' in 'word' and delete it completely character by character. Do ctrl+z. On Mac, 'word' should be selected. On other platforms 'word' should not be selected and the cursor should be placed after 'o' in 'word'.</p> -<div id="test" style="border: 2px solid red;" contenteditable>This word should be selected only on mac.</div> -</div> -<div id="console"></div> -<script src="../../resources/js-test.js"></script> -<script> -description('Verifies the selection behavior on undoing a text deletion.'); -var selectionNode = document.getElementById('test').firstChild; -var selectionOffset = selectionNode.data.indexOf('o') + 1; -var startOffsetMac = selectionNode.data.indexOf(' ') + 1; -var endOffsetMac = selectionNode.data.indexOf('d') + 1; -var selection = window.getSelection(); - -function undoTest(platform, expectedStartNode, expectedStartOffset, expectedEndNode, expectedEndOffset, selectedText) { - debug(platform); - internals.settings.setEditingBehavior(platform); - - selection.collapse(selectionNode, selectionOffset); - for (var i = 0; i < 2; i++) - document.execCommand('delete'); - for (var i = 0; i < 2; i++) - document.execCommand('forwarddelete'); - document.execCommand('undo'); - - shouldBeEqualToString('selection.anchorNode.data', expectedStartNode.data); - shouldBe('selection.anchorOffset', expectedStartOffset + ''); - shouldBeEqualToString('selection.focusNode.data', expectedEndNode.data); - shouldBe('selection.focusOffset', expectedEndOffset + ''); - shouldBeEqualToString('selection.toString()', selectedText); -} - -if (window.internals) { - undoTest('mac', selectionNode, startOffsetMac, selectionNode, endOffsetMac, 'word'); - undoTest('win', selectionNode, selectionOffset, selectionNode, selectionOffset, ''); - undoTest('unix', selectionNode, selectionOffset, selectionNode, selectionOffset, ''); - undoTest('android', selectionNode, selectionOffset, selectionNode, selectionOffset, ''); -} -if (window.testRunner) - document.getElementById('container').outerHTML = ''; -</script> -</body> -</html>
diff --git a/third_party/WebKit/LayoutTests/editing/undo/undo-delete-expected.txt b/third_party/WebKit/LayoutTests/editing/undo/undo-delete-expected.txt deleted file mode 100644 index ea7359b..0000000 --- a/third_party/WebKit/LayoutTests/editing/undo/undo-delete-expected.txt +++ /dev/null
@@ -1,28 +0,0 @@ -mac -PASS selection.anchorNode.data is "This word should be selected only on mac." -PASS selection.anchorOffset is 9 -PASS selection.focusNode.data is "This word should be selected only on mac." -PASS selection.focusOffset is 5 -PASS selection.toString() is "word" -win -PASS selection.anchorNode.data is "This word should be selected only on mac." -PASS selection.anchorOffset is 9 -PASS selection.focusNode.data is "This word should be selected only on mac." -PASS selection.focusOffset is 9 -PASS selection.toString() is "" -unix -PASS selection.anchorNode.data is "This word should be selected only on mac." -PASS selection.anchorOffset is 9 -PASS selection.focusNode.data is "This word should be selected only on mac." -PASS selection.focusOffset is 9 -PASS selection.toString() is "" -android -PASS selection.anchorNode.data is "This word should be selected only on mac." -PASS selection.anchorOffset is 9 -PASS selection.focusNode.data is "This word should be selected only on mac." -PASS selection.focusOffset is 9 -PASS selection.toString() is "" -PASS successfullyParsed is true - -TEST COMPLETE -
diff --git a/third_party/WebKit/LayoutTests/editing/undo/undo-delete.html b/third_party/WebKit/LayoutTests/editing/undo/undo-delete.html index c5ccf4d..a56a382 100644 --- a/third_party/WebKit/LayoutTests/editing/undo/undo-delete.html +++ b/third_party/WebKit/LayoutTests/editing/undo/undo-delete.html
@@ -1,44 +1,73 @@ -<!DOCTYPE html> -<html> -<body> -<div id="container"> -<p id="description"></p> -<p>To test manually, place the cursor at the end of 'word' and delete it completely character by character. Do ctrl+z. On Mac, 'word' should be selected. On other platforms 'word' should not be selected and the cursor should be placed at the end of 'word'.</p> -<div id="test" style="border: 2px solid red;" contenteditable>This word should be selected only on mac.</div> -</div> -<div id="console"></div> -<script src="../../resources/js-test.js"></script> +<!doctype html> +<script src="../../resources/testharness.js"></script> +<script src="../../resources/testharnessreport.js"></script> +<script src="../assert_selection.js"></script> <script> -description('Verifies the selection behavior on undoing a text deletion.'); -var selectionNode = document.getElementById('test').firstChild; -var selectionOffset = selectionNode.data.indexOf('d') + 1; -var endOffsetMac = selectionNode.data.indexOf(' ') + 1; -var selection = window.getSelection(); +function undoTest(behavior) { + if (window.internals) + internals.settings.setEditingBehavior(behavior); + test(() => assert_selection( + [ + '<div contenteditable>', + 'This word| should be selected only on mac.', + '</div>' + ].join(''), + selection => { + for (let i = 0; i < 4; ++i) + selection.document.execCommand('delete'); + selection.document.execCommand('undo'); + }, + [ + '<div contenteditable>', + behavior === 'mac' + ? 'This |word^ should be selected only on mac.' + : 'This word| should be selected only on mac.', + '</div>', + ].join('')), + `${behavior}: backward delete`); -function undoTest(platform, expectedStartNode, expectedStartOffset, expectedEndNode, expectedEndOffset, selectedText) { - debug(platform); - internals.settings.setEditingBehavior(platform); + test(() => assert_selection( + [ + '<div contenteditable>', + 'This |word should be selected only on mac.', + '</div>' + ].join(''), + selection => { + for (let i = 0; i < 4; ++i) + selection.document.execCommand('forwardDelete'); + selection.document.execCommand('undo'); + }, + [ + '<div contenteditable>', + behavior === 'mac' + ? 'This ^word| should be selected only on mac.' + : 'This |word should be selected only on mac.', + '</div>', + ].join('')), + `${behavior}: forward delete`); - selection.collapse(selectionNode, selectionOffset); - for (var i = 0; i < 4; i++) - document.execCommand('delete'); - document.execCommand('undo'); - - shouldBeEqualToString('selection.anchorNode.data', expectedStartNode.data); - shouldBe('selection.anchorOffset', expectedStartOffset + ''); - shouldBeEqualToString('selection.focusNode.data', expectedEndNode.data); - shouldBe('selection.focusOffset', expectedEndOffset + ''); - shouldBeEqualToString('selection.toString()', selectedText); + test(() => assert_selection( + [ + '<div contenteditable>', + 'This wo|rd should be selected only on mac.', + '</div>' + ].join(''), + selection => { + for (let i = 0; i < 2; ++i) + selection.document.execCommand('delete'); + for (let i = 0; i < 2; ++i) + selection.document.execCommand('forwardDelete'); + selection.document.execCommand('undo'); + }, + [ + '<div contenteditable>', + behavior === 'mac' + ? 'This ^word| should be selected only on mac.' + : 'This wo|rd should be selected only on mac.', + '</div>', + ].join('')), + `${behavior}: backward and forward delete`); } - -if (window.internals) { - undoTest('mac', selectionNode, selectionOffset, selectionNode, endOffsetMac, 'word'); - undoTest('win', selectionNode, selectionOffset, selectionNode, selectionOffset, ''); - undoTest('unix', selectionNode, selectionOffset, selectionNode, selectionOffset, ''); - undoTest('android', selectionNode, selectionOffset, selectionNode, selectionOffset, ''); -} -if (window.testRunner) - document.getElementById('container').outerHTML = ''; +for (let behavior of ['android', 'mac', 'unix', 'win']) + undoTest(behavior); </script> -</body> -</html>
diff --git a/third_party/WebKit/LayoutTests/editing/undo/undo-forward-delete-expected.txt b/third_party/WebKit/LayoutTests/editing/undo/undo-forward-delete-expected.txt deleted file mode 100644 index a695a17..0000000 --- a/third_party/WebKit/LayoutTests/editing/undo/undo-forward-delete-expected.txt +++ /dev/null
@@ -1,28 +0,0 @@ -mac -PASS selection.anchorNode.data is "This word should be selected only on mac." -PASS selection.anchorOffset is 5 -PASS selection.focusNode.data is "This word should be selected only on mac." -PASS selection.focusOffset is 9 -PASS selection.toString() is "word" -win -PASS selection.anchorNode.data is "This word should be selected only on mac." -PASS selection.anchorOffset is 5 -PASS selection.focusNode.data is "This word should be selected only on mac." -PASS selection.focusOffset is 5 -PASS selection.toString() is "" -unix -PASS selection.anchorNode.data is "This word should be selected only on mac." -PASS selection.anchorOffset is 5 -PASS selection.focusNode.data is "This word should be selected only on mac." -PASS selection.focusOffset is 5 -PASS selection.toString() is "" -android -PASS selection.anchorNode.data is "This word should be selected only on mac." -PASS selection.anchorOffset is 5 -PASS selection.focusNode.data is "This word should be selected only on mac." -PASS selection.focusOffset is 5 -PASS selection.toString() is "" -PASS successfullyParsed is true - -TEST COMPLETE -
diff --git a/third_party/WebKit/LayoutTests/editing/undo/undo-forward-delete.html b/third_party/WebKit/LayoutTests/editing/undo/undo-forward-delete.html deleted file mode 100644 index fcd2e4f1..0000000 --- a/third_party/WebKit/LayoutTests/editing/undo/undo-forward-delete.html +++ /dev/null
@@ -1,44 +0,0 @@ -<!DOCTYPE html> -<html> -<body> -<div id="container"> -<p id="description"></p> -<p>To test manually, place the cursor before 'w' of 'word' and delete it completely character by character. Do ctrl+z. On Mac, 'word' should be selected. On other platforms 'word' should not be selected and the cursor should be placed before 'w' of 'word'.</p> -<div id="test" style="border: 2px solid red;" contenteditable>This word should be selected only on mac.</div> -</div> -<div id="console"></div> -<script src="../../resources/js-test.js"></script> -<script> -description('Verifies the selection behavior on undoing a text deletion.'); -var selectionNode = document.getElementById('test').firstChild; -var selectionOffset = selectionNode.data.indexOf(' ') + 1; -var endOffsetMac = selectionNode.data.indexOf('d') + 1; -var selection = window.getSelection(); - -function undoTest(platform, expectedStartNode, expectedStartOffset, expectedEndNode, expectedEndOffset, selectedText) { - debug(platform); - internals.settings.setEditingBehavior(platform); - - selection.collapse(selectionNode, selectionOffset); - for (var i = 0; i < 4; i++) - document.execCommand('forwarddelete'); - document.execCommand('undo'); - - shouldBeEqualToString('selection.anchorNode.data', expectedStartNode.data); - shouldBe('selection.anchorOffset', expectedStartOffset + ''); - shouldBeEqualToString('selection.focusNode.data', expectedEndNode.data); - shouldBe('selection.focusOffset', expectedEndOffset + ''); - shouldBeEqualToString('selection.toString()', selectedText); -} - -if (window.internals) { - undoTest('mac', selectionNode, selectionOffset, selectionNode, endOffsetMac, 'word'); - undoTest('win', selectionNode, selectionOffset, selectionNode, selectionOffset, ''); - undoTest('unix', selectionNode, selectionOffset, selectionNode, selectionOffset, ''); - undoTest('android', selectionNode, selectionOffset, selectionNode, selectionOffset, ''); -} -if (window.testRunner) - document.getElementById('container').outerHTML = ''; -</script> -</body> -</html>
diff --git a/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-canvas-tainting-cache.https-expected.txt b/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-canvas-tainting-cache.https-expected.txt deleted file mode 100644 index d6af7056..0000000 --- a/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-canvas-tainting-cache.https-expected.txt +++ /dev/null
@@ -1,4 +0,0 @@ -This is a testharness.js-based test. -FAIL Verify canvas tainting of fetched image in a Service Worker assert_equals: expected "finish" but got "failure:Result of url:https://web-platform.test:8444/service-workers/service-worker/resources/fetch-access-control.py?PNGIMAGE&mode=cors&url=https%3A%2F%2Fwww1.web-platform.test%3A8444%2Fservice-workers%2Fservice-worker%2Fresources%2Ffetch-access-control.py%3FPNGIMAGE%26ACAOrigin%3Dhttps%3A%2F%2Fweb-platform.test%3A8444&cache cross_origin: use-credentials must be LOAD_ERROR but NOT_TAINTED" -Harness: the test ran to completion. -
diff --git a/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-canvas-tainting.https-expected.txt b/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-canvas-tainting.https-expected.txt deleted file mode 100644 index d639d286..0000000 --- a/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/fetch-canvas-tainting.https-expected.txt +++ /dev/null
@@ -1,4 +0,0 @@ -This is a testharness.js-based test. -FAIL Verify canvas tainting of fetched image in a Service Worker assert_equals: expected "finish" but got "failure:Result of url:https://web-platform.test:8444/service-workers/service-worker/resources/fetch-access-control.py?PNGIMAGE&mode=cors&url=https%3A%2F%2Fwww1.web-platform.test%3A8444%2Fservice-workers%2Fservice-worker%2Fresources%2Ffetch-access-control.py%3FPNGIMAGE%26ACAOrigin%3Dhttps%3A%2F%2Fweb-platform.test%3A8444 cross_origin: use-credentials must be LOAD_ERROR but NOT_TAINTED" -Harness: the test ran to completion. -
diff --git a/third_party/WebKit/LayoutTests/platform/mac-mac10.11/virtual/off-main-thread-fetch/external/wpt/service-workers/service-worker/fetch-canvas-tainting.https-expected.txt b/third_party/WebKit/LayoutTests/platform/mac-mac10.11/virtual/off-main-thread-fetch/external/wpt/service-workers/service-worker/fetch-canvas-tainting.https-expected.txt deleted file mode 100644 index d639d286..0000000 --- a/third_party/WebKit/LayoutTests/platform/mac-mac10.11/virtual/off-main-thread-fetch/external/wpt/service-workers/service-worker/fetch-canvas-tainting.https-expected.txt +++ /dev/null
@@ -1,4 +0,0 @@ -This is a testharness.js-based test. -FAIL Verify canvas tainting of fetched image in a Service Worker assert_equals: expected "finish" but got "failure:Result of url:https://web-platform.test:8444/service-workers/service-worker/resources/fetch-access-control.py?PNGIMAGE&mode=cors&url=https%3A%2F%2Fwww1.web-platform.test%3A8444%2Fservice-workers%2Fservice-worker%2Fresources%2Ffetch-access-control.py%3FPNGIMAGE%26ACAOrigin%3Dhttps%3A%2F%2Fweb-platform.test%3A8444 cross_origin: use-credentials must be LOAD_ERROR but NOT_TAINTED" -Harness: the test ran to completion. -
diff --git a/third_party/WebKit/LayoutTests/platform/mac-mac10.9/virtual/off-main-thread-fetch/external/wpt/service-workers/service-worker/fetch-canvas-tainting-cache.https-expected.txt b/third_party/WebKit/LayoutTests/platform/mac-mac10.9/virtual/off-main-thread-fetch/external/wpt/service-workers/service-worker/fetch-canvas-tainting-cache.https-expected.txt deleted file mode 100644 index cf2caad..0000000 --- a/third_party/WebKit/LayoutTests/platform/mac-mac10.9/virtual/off-main-thread-fetch/external/wpt/service-workers/service-worker/fetch-canvas-tainting-cache.https-expected.txt +++ /dev/null
@@ -1,4 +0,0 @@ -This is a testharness.js-based test. -FAIL Verify canvas tainting of fetched image in a Service Worker assert_equals: expected "finish" but got "failure:Result of url:https://www1.web-platform.test:8444/service-workers/service-worker/resources/fetch-access-control.py?PNGIMAGE&mode=cors&credentials=same-origin&url=https%3A%2F%2Fwww1.web-platform.test%3A8444%2Fservice-workers%2Fservice-worker%2Fresources%2Ffetch-access-control.py%3FPNGIMAGE%26ACAOrigin%3Dhttps%3A%2F%2Fweb-platform.test%3A8444&cache cross_origin: must be TAINTED but NOT_TAINTED" -Harness: the test ran to completion. -
diff --git a/third_party/WebKit/LayoutTests/platform/mac-retina/virtual/off-main-thread-fetch/external/wpt/service-workers/service-worker/fetch-canvas-tainting.https-expected.txt b/third_party/WebKit/LayoutTests/platform/mac-retina/virtual/off-main-thread-fetch/external/wpt/service-workers/service-worker/fetch-canvas-tainting.https-expected.txt deleted file mode 100644 index d639d286..0000000 --- a/third_party/WebKit/LayoutTests/platform/mac-retina/virtual/off-main-thread-fetch/external/wpt/service-workers/service-worker/fetch-canvas-tainting.https-expected.txt +++ /dev/null
@@ -1,4 +0,0 @@ -This is a testharness.js-based test. -FAIL Verify canvas tainting of fetched image in a Service Worker assert_equals: expected "finish" but got "failure:Result of url:https://web-platform.test:8444/service-workers/service-worker/resources/fetch-access-control.py?PNGIMAGE&mode=cors&url=https%3A%2F%2Fwww1.web-platform.test%3A8444%2Fservice-workers%2Fservice-worker%2Fresources%2Ffetch-access-control.py%3FPNGIMAGE%26ACAOrigin%3Dhttps%3A%2F%2Fweb-platform.test%3A8444 cross_origin: use-credentials must be LOAD_ERROR but NOT_TAINTED" -Harness: the test ran to completion. -
diff --git a/third_party/WebKit/LayoutTests/platform/mac/virtual/off-main-thread-fetch/external/wpt/service-workers/service-worker/fetch-canvas-tainting.https-expected.txt b/third_party/WebKit/LayoutTests/platform/mac/virtual/off-main-thread-fetch/external/wpt/service-workers/service-worker/fetch-canvas-tainting.https-expected.txt deleted file mode 100644 index a4b6320..0000000 --- a/third_party/WebKit/LayoutTests/platform/mac/virtual/off-main-thread-fetch/external/wpt/service-workers/service-worker/fetch-canvas-tainting.https-expected.txt +++ /dev/null
@@ -1,4 +0,0 @@ -This is a testharness.js-based test. -FAIL Verify canvas tainting of fetched image in a Service Worker assert_equals: expected "finish" but got "failure:Result of url:https://www1.web-platform.test:8444/service-workers/service-worker/resources/fetch-access-control.py?PNGIMAGE&mode=cors&credentials=same-origin&url=https%3A%2F%2Fwww1.web-platform.test%3A8444%2Fservice-workers%2Fservice-worker%2Fresources%2Ffetch-access-control.py%3FPNGIMAGE%26ACAOrigin%3Dhttps%3A%2F%2Fweb-platform.test%3A8444 cross_origin: must be TAINTED but NOT_TAINTED" -Harness: the test ran to completion. -
diff --git a/third_party/WebKit/Source/core/BUILD.gn b/third_party/WebKit/Source/core/BUILD.gn index 68a39a16..2087d79 100644 --- a/third_party/WebKit/Source/core/BUILD.gn +++ b/third_party/WebKit/Source/core/BUILD.gn
@@ -471,6 +471,7 @@ "$blink_core_output_dir/css/properties/CSSPropertyAPIAnimationName.h", "$blink_core_output_dir/css/properties/CSSPropertyAPIAnimationPlayState.h", "$blink_core_output_dir/css/properties/CSSPropertyAPIBaselineShift.h", + "$blink_core_output_dir/css/properties/CSSPropertyAPIBorderColor.h", "$blink_core_output_dir/css/properties/CSSPropertyAPIBorderImageOutset.h", "$blink_core_output_dir/css/properties/CSSPropertyAPIBorderImageRepeat.h", "$blink_core_output_dir/css/properties/CSSPropertyAPIBorderImageSlice.h", @@ -603,6 +604,9 @@ "$blink_core_output_dir/css/properties/CSSShorthandPropertyAPIOverflow.h", "$blink_core_output_dir/css/properties/CSSShorthandPropertyAPIOffset.h", "$blink_core_output_dir/css/properties/CSSShorthandPropertyAPIScrollPaddingBlock.h", + "$blink_core_output_dir/css/properties/CSSShorthandPropertyAPIScrollPaddingInline.h", + "$blink_core_output_dir/css/properties/CSSShorthandPropertyAPIScrollSnapMarginBlock.h", + "$blink_core_output_dir/css/properties/CSSShorthandPropertyAPIScrollSnapMarginInline.h", "$blink_core_output_dir/css/properties/CSSShorthandPropertyAPITextDecoration.h", "$blink_core_output_dir/css/properties/CSSShorthandPropertyAPIWebkitBorderAfter.h", "$blink_core_output_dir/css/properties/CSSShorthandPropertyAPIWebkitBorderBefore.h",
diff --git a/third_party/WebKit/Source/core/css/BUILD.gn b/third_party/WebKit/Source/core/css/BUILD.gn index 309458f..ec78751 100644 --- a/third_party/WebKit/Source/core/css/BUILD.gn +++ b/third_party/WebKit/Source/core/css/BUILD.gn
@@ -381,6 +381,7 @@ "properties/CSSPropertyAPIAnimationName.cpp", "properties/CSSPropertyAPIAnimationPlayState.cpp", "properties/CSSPropertyAPIBaselineShift.cpp", + "properties/CSSPropertyAPIBorderColor.cpp", "properties/CSSPropertyAPIBorderImageOutset.cpp", "properties/CSSPropertyAPIBorderImageRepeat.cpp", "properties/CSSPropertyAPIBorderImageSlice.cpp", @@ -546,6 +547,9 @@ "properties/CSSShorthandPropertyAPIOutline.cpp", "properties/CSSShorthandPropertyAPIOverflow.cpp", "properties/CSSShorthandPropertyAPIScrollPaddingBlock.cpp", + "properties/CSSShorthandPropertyAPIScrollPaddingInline.cpp", + "properties/CSSShorthandPropertyAPIScrollSnapMarginBlock.cpp", + "properties/CSSShorthandPropertyAPIScrollSnapMarginInline.cpp", "properties/CSSShorthandPropertyAPITextDecoration.cpp", "properties/CSSShorthandPropertyAPIWebkitBorderAfter.cpp", "properties/CSSShorthandPropertyAPIWebkitBorderBefore.cpp",
diff --git a/third_party/WebKit/Source/core/css/CSSProperties.json5 b/third_party/WebKit/Source/core/css/CSSProperties.json5 index 968d9a4..9e6bfdac 100644 --- a/third_party/WebKit/Source/core/css/CSSProperties.json5 +++ b/third_party/WebKit/Source/core/css/CSSProperties.json5
@@ -748,6 +748,8 @@ }, { name: "border-bottom-color", + api_class: "CSSPropertyAPIBorderColor", + api_methods: ["parseSingleValue"], custom_all: true, interpolable: true, type_name: "Color", @@ -792,6 +794,7 @@ { name: "border-bottom-width", api_class: "CSSPropertyAPIBorderWidth", + api_methods: ["parseSingleValue"], converter: "ConvertBorderWidth", interpolable: true, keywords: ["thin", "medium", "thick"], @@ -844,6 +847,8 @@ }, { name: "border-left-color", + api_class: "CSSPropertyAPIBorderColor", + api_methods: ["parseSingleValue"], custom_all: true, interpolable: true, field_template: "storage_only", @@ -864,6 +869,7 @@ { name: "border-left-width", api_class: "CSSPropertyAPIBorderWidth", + api_methods: ["parseSingleValue"], converter: "ConvertBorderWidth", interpolable: true, keywords: ["thin", "medium", "thick"], @@ -876,6 +882,8 @@ }, { name: "border-right-color", + api_class: "CSSPropertyAPIBorderColor", + api_methods: ["parseSingleValue"], custom_all: true, interpolable: true, field_template: "storage_only", @@ -896,6 +904,7 @@ { name: "border-right-width", api_class: "CSSPropertyAPIBorderWidth", + api_methods: ["parseSingleValue"], converter: "ConvertBorderWidth", interpolable: true, keywords: ["thin", "medium", "thick"], @@ -908,6 +917,8 @@ }, { name: "border-top-color", + api_class: "CSSPropertyAPIBorderColor", + api_methods: ["parseSingleValue"], custom_all: true, interpolable: true, field_template: "storage_only", @@ -952,6 +963,7 @@ { name: "border-top-width", api_class: "CSSPropertyAPIBorderWidth", + api_methods: ["parseSingleValue"], converter: "ConvertBorderWidth", interpolable: true, keywords: ["thin", "medium", "thick"], @@ -3847,6 +3859,8 @@ { name: "scroll-padding-inline", longhands: ["scroll-padding-inline-start", "scroll-padding-inline-end"], + api_class: true, + api_methods: ["parseShorthand"], runtime_flag: "CSSScrollSnapPoints", }, { @@ -3857,11 +3871,15 @@ { name: "scroll-snap-margin-block", longhands: ["scroll-snap-margin-block-start", "scroll-snap-margin-block-end"], + api_class: true, + api_methods: ["parseShorthand"], runtime_flag: "CSSScrollSnapPoints", }, { name: "scroll-snap-margin-inline", longhands: ["scroll-snap-margin-inline-start", "scroll-snap-margin-inline-end"], + api_class: true, + api_methods: ["parseShorthand"], runtime_flag: "CSSScrollSnapPoints", }, {
diff --git a/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp b/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp index efd3f16..9ee80dc4 100644 --- a/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp +++ b/third_party/WebKit/Source/core/css/parser/CSSPropertyParser.cpp
@@ -47,7 +47,6 @@ #include "core/css/properties/CSSPropertyPositionUtils.h" #include "core/css/properties/CSSPropertyTextDecorationLineUtils.h" #include "core/css/properties/CSSPropertyTransitionPropertyUtils.h" -#include "core/css/properties/CSSPropertyWebkitBorderWidthUtils.h" #include "core/frame/UseCounter.h" #include "core/layout/LayoutTheme.h" #include "platform/wtf/text/StringBuilder.h" @@ -1222,27 +1221,6 @@ case CSSPropertyGridRowGap: return ConsumeLengthOrPercent(range_, context_->Mode(), kValueRangeNonNegative); - case CSSPropertyBorderBottomColor: - case CSSPropertyBorderLeftColor: - case CSSPropertyBorderRightColor: - case CSSPropertyBorderTopColor: { - bool allow_quirky_colors = - InQuirksMode() && (current_shorthand == CSSPropertyInvalid || - current_shorthand == CSSPropertyBorderColor); - return ConsumeColor(range_, context_->Mode(), allow_quirky_colors); - } - case CSSPropertyBorderBottomWidth: - case CSSPropertyBorderLeftWidth: - case CSSPropertyBorderRightWidth: - case CSSPropertyBorderTopWidth: { - bool allow_quirky_lengths = - InQuirksMode() && (current_shorthand == CSSPropertyInvalid || - current_shorthand == CSSPropertyBorderWidth); - UnitlessQuirk unitless = - allow_quirky_lengths ? UnitlessQuirk::kAllow : UnitlessQuirk::kForbid; - return CSSPropertyWebkitBorderWidthUtils::ConsumeBorderWidth( - range_, context_->Mode(), unitless); - } case CSSPropertyFilter: case CSSPropertyBackdropFilter: return ConsumeFilter(range_, context_); @@ -2470,14 +2448,8 @@ return ConsumePlaceSelfShorthand(important); case CSSPropertyScrollPadding: return Consume4Values(scrollPaddingShorthand(), important); - case CSSPropertyScrollPaddingInline: - return Consume2Values(scrollPaddingInlineShorthand(), important); case CSSPropertyScrollSnapMargin: return Consume4Values(scrollSnapMarginShorthand(), important); - case CSSPropertyScrollSnapMarginBlock: - return Consume2Values(scrollSnapMarginBlockShorthand(), important); - case CSSPropertyScrollSnapMarginInline: - return Consume2Values(scrollSnapMarginInlineShorthand(), important); default: return false; }
diff --git a/third_party/WebKit/Source/core/css/properties/CSSPropertyAPIBorderColor.cpp b/third_party/WebKit/Source/core/css/properties/CSSPropertyAPIBorderColor.cpp new file mode 100644 index 0000000..9154894 --- /dev/null +++ b/third_party/WebKit/Source/core/css/properties/CSSPropertyAPIBorderColor.cpp
@@ -0,0 +1,27 @@ +// Copyright 2017 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 "core/css/properties/CSSPropertyAPIBorderColor.h" + +#include "core/CSSPropertyNames.h" +#include "core/css/parser/CSSParserContext.h" +#include "core/css/parser/CSSParserLocalContext.h" +#include "core/css/parser/CSSParserMode.h" +#include "core/css/parser/CSSPropertyParserHelpers.h" + +namespace blink { + +const CSSValue* CSSPropertyAPIBorderColor::parseSingleValue( + CSSParserTokenRange& range, + const CSSParserContext& context, + const CSSParserLocalContext& local_context) { + CSSPropertyID shorthand = local_context.CurrentShorthand(); + bool allow_quirky_colors = + IsQuirksModeBehavior(context.Mode()) && + (shorthand == CSSPropertyInvalid || shorthand == CSSPropertyBorderColor); + return CSSPropertyParserHelpers::ConsumeColor(range, context.Mode(), + allow_quirky_colors); +} + +} // namespace blink
diff --git a/third_party/WebKit/Source/core/css/properties/CSSPropertyAPIBorderWidth.cpp b/third_party/WebKit/Source/core/css/properties/CSSPropertyAPIBorderWidth.cpp index de961cb..cf1f4898 100644 --- a/third_party/WebKit/Source/core/css/properties/CSSPropertyAPIBorderWidth.cpp +++ b/third_party/WebKit/Source/core/css/properties/CSSPropertyAPIBorderWidth.cpp
@@ -4,4 +4,27 @@ #include "core/css/properties/CSSPropertyAPIBorderWidth.h" -namespace blink {} // namespace blink +#include "core/css/parser/CSSParserContext.h" +#include "core/css/parser/CSSParserLocalContext.h" +#include "core/css/parser/CSSParserMode.h" +#include "core/css/parser/CSSPropertyParserHelpers.h" +#include "core/css/properties/CSSPropertyWebkitBorderWidthUtils.h" + +namespace blink { + +const CSSValue* CSSPropertyAPIBorderWidth::parseSingleValue( + CSSParserTokenRange& range, + const CSSParserContext& context, + const CSSParserLocalContext& local_context) { + CSSPropertyID shorthand = local_context.CurrentShorthand(); + bool allow_quirky_lengths = + IsQuirksModeBehavior(context.Mode()) && + (shorthand == CSSPropertyInvalid || shorthand == CSSPropertyBorderWidth); + CSSPropertyParserHelpers::UnitlessQuirk unitless = + allow_quirky_lengths ? CSSPropertyParserHelpers::UnitlessQuirk::kAllow + : CSSPropertyParserHelpers::UnitlessQuirk::kForbid; + return CSSPropertyWebkitBorderWidthUtils::ConsumeBorderWidth( + range, context.Mode(), unitless); +} + +} // namespace blink
diff --git a/third_party/WebKit/Source/core/css/properties/CSSShorthandPropertyAPIScrollPaddingInline.cpp b/third_party/WebKit/Source/core/css/properties/CSSShorthandPropertyAPIScrollPaddingInline.cpp new file mode 100644 index 0000000..80591a0 --- /dev/null +++ b/third_party/WebKit/Source/core/css/properties/CSSShorthandPropertyAPIScrollPaddingInline.cpp
@@ -0,0 +1,21 @@ +// Copyright 2017 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 "core/css/properties/CSSShorthandPropertyAPIScrollPaddingInline.h" + +#include "core/StylePropertyShorthand.h" +#include "core/css/parser/CSSPropertyParserHelpers.h" + +namespace blink { + +bool CSSShorthandPropertyAPIScrollPaddingInline::parseShorthand( + bool important, + CSSParserTokenRange& range, + const CSSParserContext& context, + bool, + HeapVector<CSSProperty, 256>& properties) { + return CSSPropertyParserHelpers::ConsumeShorthandVia2LonghandAPIs( + scrollPaddingInlineShorthand(), important, context, range, properties); +} +} // namespace blink
diff --git a/third_party/WebKit/Source/core/css/properties/CSSShorthandPropertyAPIScrollSnapMarginBlock.cpp b/third_party/WebKit/Source/core/css/properties/CSSShorthandPropertyAPIScrollSnapMarginBlock.cpp new file mode 100644 index 0000000..ac315e25 --- /dev/null +++ b/third_party/WebKit/Source/core/css/properties/CSSShorthandPropertyAPIScrollSnapMarginBlock.cpp
@@ -0,0 +1,21 @@ +// Copyright 2017 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 "core/css/properties/CSSShorthandPropertyAPIScrollSnapMarginBlock.h" + +#include "core/StylePropertyShorthand.h" +#include "core/css/parser/CSSPropertyParserHelpers.h" + +namespace blink { + +bool CSSShorthandPropertyAPIScrollSnapMarginBlock::parseShorthand( + bool important, + CSSParserTokenRange& range, + const CSSParserContext& context, + bool, + HeapVector<CSSProperty, 256>& properties) { + return CSSPropertyParserHelpers::ConsumeShorthandVia2LonghandAPIs( + scrollSnapMarginBlockShorthand(), important, context, range, properties); +} +} // namespace blink
diff --git a/third_party/WebKit/Source/core/css/properties/CSSShorthandPropertyAPIScrollSnapMarginInline.cpp b/third_party/WebKit/Source/core/css/properties/CSSShorthandPropertyAPIScrollSnapMarginInline.cpp new file mode 100644 index 0000000..c76ec0f9 --- /dev/null +++ b/third_party/WebKit/Source/core/css/properties/CSSShorthandPropertyAPIScrollSnapMarginInline.cpp
@@ -0,0 +1,21 @@ +// Copyright 2017 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 "core/css/properties/CSSShorthandPropertyAPIScrollSnapMarginInline.h" + +#include "core/StylePropertyShorthand.h" +#include "core/css/parser/CSSPropertyParserHelpers.h" + +namespace blink { + +bool CSSShorthandPropertyAPIScrollSnapMarginInline::parseShorthand( + bool important, + CSSParserTokenRange& range, + const CSSParserContext& context, + bool, + HeapVector<CSSProperty, 256>& properties) { + return CSSPropertyParserHelpers::ConsumeShorthandVia2LonghandAPIs( + scrollSnapMarginInlineShorthand(), important, context, range, properties); +} +} // namespace blink
diff --git a/third_party/WebKit/Source/core/editing/FrameSelection.cpp b/third_party/WebKit/Source/core/editing/FrameSelection.cpp index 20af29d4..1a781f93 100644 --- a/third_party/WebKit/Source/core/editing/FrameSelection.cpp +++ b/third_party/WebKit/Source/core/editing/FrameSelection.cpp
@@ -1163,6 +1163,13 @@ return layout_selection_->SelectionStartEnd(); } +base::Optional<int> FrameSelection::LayoutSelectionStart() const { + return layout_selection_->SelectionStart(); +} +base::Optional<int> FrameSelection::LayoutSelectionEnd() const { + return layout_selection_->SelectionEnd(); +} + void FrameSelection::ClearLayoutSelection() { layout_selection_->ClearSelection(); }
diff --git a/third_party/WebKit/Source/core/editing/FrameSelection.h b/third_party/WebKit/Source/core/editing/FrameSelection.h index 17e3f40d..4f0b8c95 100644 --- a/third_party/WebKit/Source/core/editing/FrameSelection.h +++ b/third_party/WebKit/Source/core/editing/FrameSelection.h
@@ -244,6 +244,8 @@ FrameCaret& FrameCaretForTesting() const { return *frame_caret_; } std::pair<int, int> LayoutSelectionStartEnd(); + base::Optional<int> LayoutSelectionStart() const; + base::Optional<int> LayoutSelectionEnd() const; void ClearLayoutSelection(); DECLARE_TRACE();
diff --git a/third_party/WebKit/Source/core/editing/LayoutSelection.cpp b/third_party/WebKit/Source/core/editing/LayoutSelection.cpp index f23f783..2606def 100644 --- a/third_party/WebKit/Source/core/editing/LayoutSelection.cpp +++ b/third_party/WebKit/Source/core/editing/LayoutSelection.cpp
@@ -297,6 +297,20 @@ return std::make_pair(paint_range_.StartOffset(), paint_range_.EndOffset()); } +base::Optional<int> LayoutSelection::SelectionStart() const { + DCHECK(!HasPendingSelection()); + if (paint_range_.IsNull()) + return {}; + return paint_range_.StartOffset(); +} + +base::Optional<int> LayoutSelection::SelectionEnd() const { + DCHECK(!HasPendingSelection()); + if (paint_range_.IsNull()) + return {}; + return paint_range_.EndOffset(); +} + void LayoutSelection::ClearSelection() { // For querying Layer::compositingState() // This is correct, since destroying layout objects needs to cause eager paint
diff --git a/third_party/WebKit/Source/core/editing/LayoutSelection.h b/third_party/WebKit/Source/core/editing/LayoutSelection.h index 84b27263..f56967b 100644 --- a/third_party/WebKit/Source/core/editing/LayoutSelection.h +++ b/third_party/WebKit/Source/core/editing/LayoutSelection.h
@@ -100,7 +100,10 @@ void InvalidatePaintForSelection(); void ClearSelection(); + // TODO(yoiciho): Replace SelectionStartEnd with SelectionStart/End. std::pair<int, int> SelectionStartEnd(); + base::Optional<int> SelectionStart() const; + base::Optional<int> SelectionEnd() const; void OnDocumentShutdown(); DECLARE_TRACE();
diff --git a/third_party/WebKit/Source/core/page/TouchAdjustment.cpp b/third_party/WebKit/Source/core/page/TouchAdjustment.cpp index d3656cc..2963e93 100644 --- a/third_party/WebKit/Source/core/page/TouchAdjustment.cpp +++ b/third_party/WebKit/Source/core/page/TouchAdjustment.cpp
@@ -204,6 +204,8 @@ } else { if (text_layout_object->GetSelectionState() == SelectionState::kNone) return AppendBasicSubtargetsForNode(node, subtargets); + const FrameSelection& frame_selection = + text_layout_object->GetFrame()->Selection(); // If selected, make subtargets out of only the selected part of the text. int start_pos, end_pos; switch (text_layout_object->GetSelectionState()) { @@ -212,15 +214,16 @@ end_pos = text_layout_object->TextLength(); break; case SelectionState::kStart: - std::tie(start_pos, end_pos) = text_layout_object->SelectionStartEnd(); + start_pos = frame_selection.LayoutSelectionStart().value(); end_pos = text_layout_object->TextLength(); break; case SelectionState::kEnd: - std::tie(start_pos, end_pos) = text_layout_object->SelectionStartEnd(); start_pos = 0; + end_pos = frame_selection.LayoutSelectionEnd().value(); break; case SelectionState::kStartAndEnd: - std::tie(start_pos, end_pos) = text_layout_object->SelectionStartEnd(); + start_pos = frame_selection.LayoutSelectionStart().value(); + end_pos = frame_selection.LayoutSelectionEnd().value(); break; default: NOTREACHED();
diff --git a/third_party/WebKit/Source/core/streams/README.md b/third_party/WebKit/Source/core/streams/README.md index 44a89f51..78e10fc 100644 --- a/third_party/WebKit/Source/core/streams/README.md +++ b/third_party/WebKit/Source/core/streams/README.md
@@ -1,26 +1,8 @@ -This directory contains the Blink implementation of the WHATWG Streams standard: -https://streams.spec.whatwg.org/. There is also a legacy streams implementation. +# core/streams/ -## V8 Extras ReadableStream Implementation +This directory contains the Blink implementation of [the WHATWG Streams standard][1]. -- ByteLengthQueuingStrategy.js -- CountQueuingStrategy.js -- ReadableStream.js -- ReadableStreamExperimentalPipeTo.js -- ReadableStreamController.h -- ReadableStreamOperations.{cpp,h} -- UnderlyingSourceBase.{cpp,h,idl} -- WritableStream.js +We use [V8 extras][2] to implement it. -These files implement ReadableStream using [V8 extras][1]. All new code should -use this implementation. - -[1]: https://docs.google.com/document/d/1AT5-T0aHGp7Lt29vPWFr2-qG8r3l9CByyvKwEuA8Ec0 - -## Old streams - -- Stream.{cpp,h,idl} - -These files support an old streams spec. They should eventually be removed, but -right now XMLHttpRequest and Media Streams Extension code in Blink still -depends on them. +[1]: https://streams.spec.whatwg.org/ +[2]: https://docs.google.com/document/d/1AT5-T0aHGp7Lt29vPWFr2-qG8r3l9CByyvKwEuA8Ec0
diff --git a/third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerGlobalScope.cpp b/third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerGlobalScope.cpp index d3aeb17..d9832983 100644 --- a/third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerGlobalScope.cpp +++ b/third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerGlobalScope.cpp
@@ -84,6 +84,7 @@ startup_data->worker_v8_settings_.v8_cache_options_); context->ApplyContentSecurityPolicyFromVector( *startup_data->content_security_policy_headers_); + context->SetWorkerSettings(std::move(startup_data->worker_settings_)); if (!startup_data->referrer_policy_.IsNull()) context->ParseAndSetReferrerPolicy(startup_data->referrer_policy_); context->SetAddressSpace(startup_data->address_space_);
diff --git a/third_party/WebKit/Source/platform/loader/fetch/MemoryCacheCorrectnessTest.cpp b/third_party/WebKit/Source/platform/loader/fetch/MemoryCacheCorrectnessTest.cpp index 36ed404..e5b4f76b 100644 --- a/third_party/WebKit/Source/platform/loader/fetch/MemoryCacheCorrectnessTest.cpp +++ b/third_party/WebKit/Source/platform/loader/fetch/MemoryCacheCorrectnessTest.cpp
@@ -61,7 +61,6 @@ if (response.Url().IsNull()) response.SetURL(KURL(kParsedURLString, kResourceURL)); ResourceRequest request(response.Url()); - request.SetFetchCredentialsMode(WebURLRequest::kFetchCredentialsModeOmit); MockResource* resource = MockResource::Create(request); resource->SetResponse(response); resource->Finish(); @@ -353,7 +352,6 @@ KURL redirect_target_url(kParsedURLString, kRedirectTargetUrlString); ResourceRequest request(redirect_url); - request.SetFetchCredentialsMode(WebURLRequest::kFetchCredentialsModeOmit); MockResource* first_resource = MockResource::Create(request); ResourceResponse fresh301_response; @@ -431,7 +429,7 @@ TEST_F(MemoryCacheCorrectnessTest, PostToSameURLTwice) { ResourceRequest request1(KURL(kParsedURLString, kResourceURL)); request1.SetHTTPMethod(HTTPNames::POST); - RawResource* resource1 = RawResource::Create(request1, Resource::kRaw); + RawResource* resource1 = RawResource::CreateForTest(request1, Resource::kRaw); resource1->SetStatus(ResourceStatus::kPending); GetMemoryCache()->Add(resource1); @@ -491,7 +489,6 @@ KURL redirect_target_url(kParsedURLString, kRedirectTargetUrlString); ResourceRequest request(redirect_url); - request.SetFetchCredentialsMode(WebURLRequest::kFetchCredentialsModeOmit); MockResource* first_resource = MockResource::Create(request); ResourceResponse fresh302_response; @@ -532,7 +529,6 @@ KURL redirect_target_url(kParsedURLString, kRedirectTargetUrlString); ResourceRequest request(redirect_url); - request.SetFetchCredentialsMode(WebURLRequest::kFetchCredentialsModeOmit); MockResource* first_resource = MockResource::Create(request); ResourceResponse fresh302_response;
diff --git a/third_party/WebKit/Source/platform/loader/fetch/RawResource.h b/third_party/WebKit/Source/platform/loader/fetch/RawResource.h index f80ae88..189e65c0 100644 --- a/third_party/WebKit/Source/platform/loader/fetch/RawResource.h +++ b/third_party/WebKit/Source/platform/loader/fetch/RawResource.h
@@ -52,14 +52,13 @@ static RawResource* FetchManifest(FetchParameters&, ResourceFetcher*); // Exposed for testing - static RawResource* Create(ResourceRequest request, Type type) { - request.SetFetchCredentialsMode(WebURLRequest::kFetchCredentialsModeOmit); + static RawResource* CreateForTest(ResourceRequest request, Type type) { ResourceLoaderOptions options; return new RawResource(request, type, options); } static RawResource* CreateForTest(const KURL& url, Type type) { ResourceRequest request(url); - return Create(request, type); + return CreateForTest(request, type); } static RawResource* CreateForTest(const char* url, Type type) { return CreateForTest(KURL(kParsedURLString, url), type);
diff --git a/third_party/WebKit/Source/platform/loader/fetch/RawResourceTest.cpp b/third_party/WebKit/Source/platform/loader/fetch/RawResourceTest.cpp index 5e1fc108..f925f9fe 100644 --- a/third_party/WebKit/Source/platform/loader/fetch/RawResourceTest.cpp +++ b/third_party/WebKit/Source/platform/loader/fetch/RawResourceTest.cpp
@@ -111,7 +111,8 @@ ResourceRequest jpeg_request; jpeg_request.SetHTTPAccept("image/jpeg"); - RawResource* jpeg_resource(RawResource::Create(jpeg_request, Resource::kRaw)); + RawResource* jpeg_resource( + RawResource::CreateForTest(jpeg_request, Resource::kRaw)); ResourceRequest png_request; png_request.SetHTTPAccept("image/png"); @@ -579,7 +580,7 @@ ResourceRequest request("data:text/html,"); request.SetHTTPHeaderField( HTTPNames::X_DevTools_Emulate_Network_Conditions_Client_Id, "Foo"); - Resource* raw = RawResource::Create(request, Resource::kRaw); + Resource* raw = RawResource::CreateForTest(request, Resource::kRaw); EXPECT_TRUE( raw->CanReuse(FetchParameters(ResourceRequest("data:text/html,")))); }
diff --git a/third_party/WebKit/Source/platform/loader/fetch/Resource.cpp b/third_party/WebKit/Source/platform/loader/fetch/Resource.cpp index 88bf5f93..ef87feef 100644 --- a/third_party/WebKit/Source/platform/loader/fetch/Resource.cpp +++ b/third_party/WebKit/Source/platform/loader/fetch/Resource.cpp
@@ -893,7 +893,9 @@ if (existing_was_with_fetcher_cors_suppressed) return new_mode != WebURLRequest::kFetchRequestModeCORS; - return existing_mode == new_mode; + return existing_mode == new_mode && + new_request.GetFetchCredentialsMode() == + resource_request_.GetFetchCredentialsMode(); } void Resource::Prune() {
diff --git a/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp b/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp index e7ea140e..4fd85f2 100644 --- a/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp +++ b/third_party/WebKit/Source/platform/loader/fetch/ResourceFetcher.cpp
@@ -62,14 +62,6 @@ namespace { -// Events for UMA. Do not reorder or delete. Add new events at the end, but -// before SriResourceIntegrityMismatchEventCount. -enum SriResourceIntegrityMismatchEvent { - kCheckingForIntegrityMismatch = 0, - kRefetchDueToIntegrityMismatch = 1, - kSriResourceIntegrityMismatchEventCount -}; - #define DEFINE_SINGLE_RESOURCE_HISTOGRAM(prefix, name) \ case Resource::k##name: { \ DEFINE_THREAD_SAFE_STATIC_LOCAL( \ @@ -110,14 +102,6 @@ } } -void RecordSriResourceIntegrityMismatchEvent( - SriResourceIntegrityMismatchEvent event) { - DEFINE_THREAD_SAFE_STATIC_LOCAL(EnumerationHistogram, integrity_histogram, - ("sri.resource_integrity_mismatch_event", - kSriResourceIntegrityMismatchEventCount)); - integrity_histogram.Count(event); -} - ResourceLoadPriority TypeToPriority(Resource::Type type) { switch (type) { case Resource::kMainResource: @@ -911,7 +895,6 @@ Resource* resource = it->value; - RecordSriResourceIntegrityMismatchEvent(kCheckingForIntegrityMismatch); if (resource->MustRefetchDueToIntegrityMetadata(params)) return nullptr; @@ -1029,9 +1012,7 @@ // resource must be made to get the raw data. This is expected to be an // uncommon case, however, as it implies two same-origin requests to the same // resource, but with different integrity metadata. - RecordSriResourceIntegrityMismatchEvent(kCheckingForIntegrityMismatch); if (existing_resource->MustRefetchDueToIntegrityMetadata(fetch_params)) { - RecordSriResourceIntegrityMismatchEvent(kRefetchDueToIntegrityMismatch); return kReload; }
diff --git a/third_party/WebKit/Source/platform/loader/fetch/ResourceTest.cpp b/third_party/WebKit/Source/platform/loader/fetch/ResourceTest.cpp index 0c46634..2b233ec 100644 --- a/third_party/WebKit/Source/platform/loader/fetch/ResourceTest.cpp +++ b/third_party/WebKit/Source/platform/loader/fetch/ResourceTest.cpp
@@ -125,7 +125,7 @@ ResourceRequest old_request(url); old_request.SetHTTPHeaderField(HTTPNames::User_Agent, "something"); old_request.SetHTTPHeaderField(HTTPNames::Referer, "http://foo.com"); - resource = RawResource::Create(old_request, Resource::kRaw); + resource = RawResource::CreateForTest(old_request, Resource::kRaw); resource->ResponseReceived(response, nullptr); resource->Finish();
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index f320c9f..66ef55a 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml
@@ -74169,6 +74169,9 @@ <histogram name="sri.resource_integrity_mismatch_event" enum="SRIResourceIntegrityMismatchEvent"> + <obsolete> + Removed 07/2017. Reference: crbug.com/708041 + </obsolete> <owner>jww@chromium.org</owner> <summary> When resources are checked for mismatching integrity and whether the
diff --git a/tools/metrics/ukm/ukm.xml b/tools/metrics/ukm/ukm.xml index 07d6aeb..0e5863b 100644 --- a/tools/metrics/ukm/ukm.xml +++ b/tools/metrics/ukm/ukm.xml
@@ -708,6 +708,28 @@ enum ManagerAutofillEvent. </summary> </metric> + <metric name="Saving.Prompt.Interaction"> + <summary> + Records how the user interacted with a saving prompt. Recorded values + correspond to the enum PasswordFormMetricsRecorder::BubbleDismissalReason. + </summary> + </metric> + <metric name="Saving.Prompt.Shown"> + <summary> + Records, for each password form seen by the password manager, whether the + user was shown a prompt that asked for permission to save new credentials. + In case a store() via the Credential Management API, a virtual password + form is created, for which this metric is recorded. Recorded values are 0 + and 1 for false and true. + </summary> + </metric> + <metric name="Saving.Prompt.Trigger"> + <summary> + Records the trigger of each password save bubble that was shown to the + user to ask for permission to save new credentials. Recorded values + correspond to the enum PasswordFormMetricsRecorder::BubbleTrigger. + </summary> + </metric> <metric name="Submission.Observed"> <summary> Records whether a submission of a password form has been observered. The @@ -808,6 +830,26 @@ enum SuppressedAccountExistence. </summary> </metric> + <metric name="Updating.Prompt.Interaction"> + <summary> + Records how the user interacted with an updating prompt. Recorded values + correspond to the enum PasswordFormMetricsRecorder::BubbleDismissalReason. + </summary> + </metric> + <metric name="Updating.Prompt.Shown"> + <summary> + Records, for each password form seen by the password manager, whether the + user was shown a prompt that asked for permission to update existing + credentials. Recorded values are 0 and 1 for false and true. + </summary> + </metric> + <metric name="Updating.Prompt.Trigger"> + <summary> + Records the trigger of each password update bubble that was shown to the + user to ask for permission to update existing credentials. Recorded values + correspond to the enum PasswordFormMetricsRecorder::BubbleTrigger. + </summary> + </metric> </event> <event name="PageWithPassword">
diff --git a/tools/traffic_annotation/auditor/tests/extractor_outputs/bad_call.txt b/tools/traffic_annotation/auditor/tests/extractor_outputs/bad_call.txt new file mode 100644 index 0000000..ea130c4 --- /dev/null +++ b/tools/traffic_annotation/auditor/tests/extractor_outputs/bad_call.txt
@@ -0,0 +1,4 @@ +headless/public/util/http_url_fetcher.cc +headless::HttpURLFetcher::Delegate::Delegate +net::URLRequestContext::CreateRequest +1 \ No newline at end of file
diff --git a/tools/traffic_annotation/auditor/tests/extractor_outputs/bad_syntax_annotation1.txt b/tools/traffic_annotation/auditor/tests/extractor_outputs/bad_syntax_annotation1.txt new file mode 100644 index 0000000..8be2614 --- /dev/null +++ b/tools/traffic_annotation/auditor/tests/extractor_outputs/bad_syntax_annotation1.txt
@@ -0,0 +1,36 @@ +chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc +OnGetTokenSuccess +166 +Definition +supervised_user_refresh_token_fetcher + + + semantics + sender: "Supervised Users" + description: + "Fetches an OAuth2 refresh token scoped down to the Supervised " + "User Sync scope and tied to the given Supervised User ID, " + "identifying the Supervised User Profile to be created." + trigger: + "Called when creating a new Supervised User profile in Chromium " + "to fetch OAuth credentials for using Sync with the new profile." + data: + "The request is authenticated with an OAuth2 access token " + "identifying the Google account and contains the following " + "information:\n* The Supervised User ID, a randomly generated " + "64-bit identifier for the profile.\n* The device name, to " + "identify the refresh token in account management." + destination: GOOGLE_OWNED_SERVICE + } + policy { + cookies_allowed: false + setting: + "Users can disable this feature by toggling 'Let anyone add a " + "person to Chrome' in Chromium settings, under People." + chrome_policy { + SupervisedUserCreationEnabled { + policy_options {mode: MANDATORY} + SupervisedUserCreationEnabled: false + } + } + } \ No newline at end of file
diff --git a/tools/traffic_annotation/auditor/tests/extractor_outputs/bad_syntax_annotation2.txt b/tools/traffic_annotation/auditor/tests/extractor_outputs/bad_syntax_annotation2.txt new file mode 100644 index 0000000..dbb0d6f --- /dev/null +++ b/tools/traffic_annotation/auditor/tests/extractor_outputs/bad_syntax_annotation2.txt
@@ -0,0 +1,36 @@ +chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc +OnGetTokenSuccess +166 +Definition +supervised_user_refresh_token_fetcher + + + Semantics { + sender: "Supervised Users" + description: + "Fetches an OAuth2 refresh token scoped down to the Supervised " + "User Sync scope and tied to the given Supervised User ID, " + "identifying the Supervised User Profile to be created." + trigger: + "Called when creating a new Supervised User profile in Chromium " + "to fetch OAuth credentials for using Sync with the new profile." + data: + "The request is authenticated with an OAuth2 access token " + "identifying the Google account and contains the following " + "information:\n* The Supervised User ID, a randomly generated " + "64-bit identifier for the profile.\n* The device name, to " + "identify the refresh token in account management." + destination: GOOGLE_OWNED_SERVICE + } + policy { + cookies_allowed: false + setting: + "Users can disable this feature by toggling 'Let anyone add a " + "person to Chrome' in Chromium settings, under People." + chrome_policy { + SupervisedUserCreationEnabled { + policy_options {mode: MANDATORY} + SupervisedUserCreationEnabled: false + } + } + } \ No newline at end of file
diff --git a/tools/traffic_annotation/auditor/tests/extractor_outputs/bad_syntax_annotation3.txt b/tools/traffic_annotation/auditor/tests/extractor_outputs/bad_syntax_annotation3.txt new file mode 100644 index 0000000..bb6ee7b --- /dev/null +++ b/tools/traffic_annotation/auditor/tests/extractor_outputs/bad_syntax_annotation3.txt
@@ -0,0 +1,36 @@ +chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc +OnGetTokenSuccess +166 +Definition +supervised_user_refresh_token_fetcher + + + Semantics { + sender: "Supervised Users + description: + "Fetches an OAuth2 refresh token scoped down to the Supervised " + "User Sync scope and tied to the given Supervised User ID, " + "identifying the Supervised User Profile to be created." + trigger: + "Called when creating a new Supervised User profile in Chromium " + "to fetch OAuth credentials for using Sync with the new profile." + data: + "The request is authenticated with an OAuth2 access token " + "identifying the Google account and contains the following " + "information:\n* The Supervised User ID, a randomly generated " + "64-bit identifier for the profile.\n* The device name, to " + "identify the refresh token in account management." + destination: GOOGLE_OWNED_SERVICE + } + policy { + cookies_allowed: false + setting: + "Users can disable this feature by toggling 'Let anyone add a " + "person to Chrome' in Chromium settings, under People." + chrome_policy { + SupervisedUserCreationEnabled { + policy_options {mode: MANDATORY} + SupervisedUserCreationEnabled: false + } + } + } \ No newline at end of file
diff --git a/tools/traffic_annotation/auditor/tests/extractor_outputs/bad_syntax_annotation4.txt b/tools/traffic_annotation/auditor/tests/extractor_outputs/bad_syntax_annotation4.txt new file mode 100644 index 0000000..8133f72 --- /dev/null +++ b/tools/traffic_annotation/auditor/tests/extractor_outputs/bad_syntax_annotation4.txt
@@ -0,0 +1,34 @@ +chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc +OnGetTokenSuccess +166 +Definition +supervised_user_refresh_token_fetcher + + + Semantics { + sender: "Supervised Users + description: + "Fetches an OAuth2 refresh token scoped down to the Supervised " + "User Sync scope and tied to the given Supervised User ID, " + "identifying the Supervised User Profile to be created." + trigger: + "Called when creating a new Supervised User profile in Chromium " + "to fetch OAuth credentials for using Sync with the new profile." + data: + "The request is authenticated with an OAuth2 access token " + "identifying the Google account and contains the following " + "information:\n* The Supervised User ID, a randomly generated " + "64-bit identifier for the profile.\n* The device name, to " + "identify the refresh token in account management." + destination: GOOGLE_OWNED_SERVICE + cookies_allowed: false + setting: + "Users can disable this feature by toggling 'Let anyone add a " + "person to Chrome' in Chromium settings, under People." + chrome_policy { + SupervisedUserCreationEnabled { + policy_options {mode: MANDATORY} + SupervisedUserCreationEnabled: false + } + } + } \ No newline at end of file
diff --git a/tools/traffic_annotation/auditor/tests/extractor_outputs/fatal_annotation1.txt b/tools/traffic_annotation/auditor/tests/extractor_outputs/fatal_annotation1.txt new file mode 100644 index 0000000..b4999cb --- /dev/null +++ b/tools/traffic_annotation/auditor/tests/extractor_outputs/fatal_annotation1.txt
@@ -0,0 +1,35 @@ +chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc +OnGetTokenSuccess +Definition +supervised_user_refresh_token_fetcher + + + semantics { + sender: "Supervised Users" + description: + "Fetches an OAuth2 refresh token scoped down to the Supervised " + "User Sync scope and tied to the given Supervised User ID, " + "identifying the Supervised User Profile to be created." + trigger: + "Called when creating a new Supervised User profile in Chromium " + "to fetch OAuth credentials for using Sync with the new profile." + data: + "The request is authenticated with an OAuth2 access token " + "identifying the Google account and contains the following " + "information:\n* The Supervised User ID, a randomly generated " + "64-bit identifier for the profile.\n* The device name, to " + "identify the refresh token in account management." + destination: GOOGLE_OWNED_SERVICE + } + policy { + cookies_allowed: false + setting: + "Users can disable this feature by toggling 'Let anyone add a " + "person to Chrome' in Chromium settings, under People." + chrome_policy { + SupervisedUserCreationEnabled { + policy_options {mode: MANDATORY} + SupervisedUserCreationEnabled: false + } + } + } \ No newline at end of file
diff --git a/tools/traffic_annotation/auditor/tests/extractor_outputs/fatal_annotation2.txt b/tools/traffic_annotation/auditor/tests/extractor_outputs/fatal_annotation2.txt new file mode 100644 index 0000000..501d3b8fd --- /dev/null +++ b/tools/traffic_annotation/auditor/tests/extractor_outputs/fatal_annotation2.txt
@@ -0,0 +1,36 @@ +chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc +OnGetTokenSuccess +122 +definition +supervised_user_refresh_token_fetcher + + + semantics { + sender: "Supervised Users" + description: + "Fetches an OAuth2 refresh token scoped down to the Supervised " + "User Sync scope and tied to the given Supervised User ID, " + "identifying the Supervised User Profile to be created." + trigger: + "Called when creating a new Supervised User profile in Chromium " + "to fetch OAuth credentials for using Sync with the new profile." + data: + "The request is authenticated with an OAuth2 access token " + "identifying the Google account and contains the following " + "information:\n* The Supervised User ID, a randomly generated " + "64-bit identifier for the profile.\n* The device name, to " + "identify the refresh token in account management." + destination: GOOGLE_OWNED_SERVICE + } + policy { + cookies_allowed: false + setting: + "Users can disable this feature by toggling 'Let anyone add a " + "person to Chrome' in Chromium settings, under People." + chrome_policy { + SupervisedUserCreationEnabled { + policy_options {mode: MANDATORY} + SupervisedUserCreationEnabled: false + } + } + } \ No newline at end of file
diff --git a/tools/traffic_annotation/auditor/tests/extractor_outputs/fatal_annotation3.txt b/tools/traffic_annotation/auditor/tests/extractor_outputs/fatal_annotation3.txt new file mode 100644 index 0000000..7e89f6f --- /dev/null +++ b/tools/traffic_annotation/auditor/tests/extractor_outputs/fatal_annotation3.txt
@@ -0,0 +1,5 @@ +chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc +OnGetTokenSuccess +122 +definition +supervised_user_refresh_token_fetcher
diff --git a/tools/traffic_annotation/auditor/tests/extractor_outputs/good_branched_completing_annotation.txt b/tools/traffic_annotation/auditor/tests/extractor_outputs/good_branched_completing_annotation.txt new file mode 100644 index 0000000..c1c8ad2 --- /dev/null +++ b/tools/traffic_annotation/auditor/tests/extractor_outputs/good_branched_completing_annotation.txt
@@ -0,0 +1,22 @@ +chrome/service/cloud_print/cloud_print_url_fetcher.cc +cloud_print::CloudPrintURLFetcher::StartRequestHelper +265 +BranchedCompleting +cloud_print + + + semantics { + sender: "Cloud Print" + destination: GOOGLE_OWNED_SERVICE + } + policy { + cookies_allowed: false + setting: + "This feature cannot be disabled by settings." + chrome_policy { + CloudPrintProxyEnabled { + policy_options {mode: MANDATORY} + CloudPrintProxyEnabled: false + } + } + } \ No newline at end of file
diff --git a/tools/traffic_annotation/auditor/tests/extractor_outputs/good_call.txt b/tools/traffic_annotation/auditor/tests/extractor_outputs/good_call.txt new file mode 100644 index 0000000..e74c34d2 --- /dev/null +++ b/tools/traffic_annotation/auditor/tests/extractor_outputs/good_call.txt
@@ -0,0 +1,5 @@ +headless/public/util/http_url_fetcher.cc +headless::HttpURLFetcher::Delegate::Delegate +100 +net::URLRequestContext::CreateRequest +1 \ No newline at end of file
diff --git a/tools/traffic_annotation/auditor/tests/extractor_outputs/good_complete_annotation.txt b/tools/traffic_annotation/auditor/tests/extractor_outputs/good_complete_annotation.txt new file mode 100644 index 0000000..ba7ab2c --- /dev/null +++ b/tools/traffic_annotation/auditor/tests/extractor_outputs/good_complete_annotation.txt
@@ -0,0 +1,36 @@ +chrome/browser/supervised_user/legacy/supervised_user_refresh_token_fetcher.cc +OnGetTokenSuccess +166 +Definition +supervised_user_refresh_token_fetcher + + + semantics { + sender: "Supervised Users" + description: + "Fetches an OAuth2 refresh token scoped down to the Supervised " + "User Sync scope and tied to the given Supervised User ID, " + "identifying the Supervised User Profile to be created." + trigger: + "Called when creating a new Supervised User profile in Chromium " + "to fetch OAuth credentials for using Sync with the new profile." + data: + "The request is authenticated with an OAuth2 access token " + "identifying the Google account and contains the following " + "information:\n* The Supervised User ID, a randomly generated " + "64-bit identifier for the profile.\n* The device name, to " + "identify the refresh token in account management." + destination: GOOGLE_OWNED_SERVICE + } + policy { + cookies_allowed: false + setting: + "Users can disable this feature by toggling 'Let anyone add a " + "person to Chrome' in Chromium settings, under People." + chrome_policy { + SupervisedUserCreationEnabled { + policy_options {mode: MANDATORY} + SupervisedUserCreationEnabled: false + } + } + } \ No newline at end of file
diff --git a/tools/traffic_annotation/auditor/tests/extractor_outputs/good_completing_annotation.txt b/tools/traffic_annotation/auditor/tests/extractor_outputs/good_completing_annotation.txt new file mode 100644 index 0000000..a29327e --- /dev/null +++ b/tools/traffic_annotation/auditor/tests/extractor_outputs/good_completing_annotation.txt
@@ -0,0 +1,22 @@ +chrome/service/cloud_print/cloud_print_url_fetcher.cc +cloud_print::CloudPrintURLFetcher::StartRequestHelper +265 +Completing +cloud_print + + + semantics { + sender: "Cloud Print" + destination: GOOGLE_OWNED_SERVICE + } + policy { + cookies_allowed: false + setting: + "This feature cannot be disabled by settings." + chrome_policy { + CloudPrintProxyEnabled { + policy_options {mode: MANDATORY} + CloudPrintProxyEnabled: false + } + } + } \ No newline at end of file
diff --git a/tools/traffic_annotation/auditor/tests/extractor_outputs/good_partial_annotation.txt b/tools/traffic_annotation/auditor/tests/extractor_outputs/good_partial_annotation.txt new file mode 100644 index 0000000..07422ab --- /dev/null +++ b/tools/traffic_annotation/auditor/tests/extractor_outputs/good_partial_annotation.txt
@@ -0,0 +1,26 @@ +components/browsing_data/core/counters/history_counter.cc +browsing_data::HistoryCounter::Count +98 +Partial +web_history_counter +web_history_service + + semantics { + description: + "If history sync is enabled, this queries history.google.com to " + "determine if there is any synced history. This information is " + "displayed in the Clear Browsing Data dialog." + trigger: + "Checking the 'Browsing history' option in the Clear Browsing Data " + "dialog, or enabling history sync while the dialog is open." + data: + "A version info token to resolve transaction conflicts, and an " + "OAuth2 token authenticating the user." + } + policy { + chrome_policy { + SyncDisabled { + SyncDisabled: true + } + } + } \ No newline at end of file
diff --git a/tools/traffic_annotation/auditor/tests/extractor_outputs/good_test_annotation.txt b/tools/traffic_annotation/auditor/tests/extractor_outputs/good_test_annotation.txt new file mode 100644 index 0000000..07bd917 --- /dev/null +++ b/tools/traffic_annotation/auditor/tests/extractor_outputs/good_test_annotation.txt
@@ -0,0 +1,7 @@ +google_apis/drive/request_sender_unittest.cc +google_apis::(anonymous namespace)::RequestSenderTest::RequestSenderTest +67 +Definition +test + +Traffic annotation for unit, browser and other tests \ No newline at end of file
diff --git a/tools/traffic_annotation/auditor/tests/extractor_outputs/missing_annotation.txt b/tools/traffic_annotation/auditor/tests/extractor_outputs/missing_annotation.txt new file mode 100644 index 0000000..2efb3c3 --- /dev/null +++ b/tools/traffic_annotation/auditor/tests/extractor_outputs/missing_annotation.txt
@@ -0,0 +1,7 @@ +net/url_request/url_request_context.cc +net::URLRequestContext::CreateRequest +120 +Definition +missing + +Function called without traffic annotation.
diff --git a/tools/traffic_annotation/auditor/tests/extractor_outputs/no_annotation.txt b/tools/traffic_annotation/auditor/tests/extractor_outputs/no_annotation.txt new file mode 100644 index 0000000..e8b2838 --- /dev/null +++ b/tools/traffic_annotation/auditor/tests/extractor_outputs/no_annotation.txt
@@ -0,0 +1,7 @@ +components/download/internal/controller_impl.cc +download::ControllerImpl::UpdateDriverState +656 +Definition +undefined + +Nothing here yet. \ No newline at end of file
diff --git a/tools/traffic_annotation/auditor/traffic_annotation_auditor.cc b/tools/traffic_annotation/auditor/traffic_annotation_auditor.cc index bc965a59..cbe0120d 100644 --- a/tools/traffic_annotation/auditor/traffic_annotation_auditor.cc +++ b/tools/traffic_annotation/auditor/traffic_annotation_auditor.cc
@@ -506,6 +506,7 @@ return false; } +// static const std::map<int, std::string>& TrafficAnnotationAuditor::GetReservedUniqueIDs() { return kReservedAnnotations;
diff --git a/tools/traffic_annotation/auditor/traffic_annotation_auditor.h b/tools/traffic_annotation/auditor/traffic_annotation_auditor.h index f46b781..17b3516 100644 --- a/tools/traffic_annotation/auditor/traffic_annotation_auditor.h +++ b/tools/traffic_annotation/auditor/traffic_annotation_auditor.h
@@ -83,9 +83,20 @@ int line_; }; +// Base class for Annotation and Call instances. +class InstanceBase { + public: + InstanceBase(){}; + virtual ~InstanceBase(){}; + virtual AuditorResult Deserialize( + const std::vector<std::string>& serialized_lines, + int start_line, + int end_line) = 0; +}; + // Holds an instance of network traffic annotation. // TODO(rhalavati): Check if this class can also be reused in clang tool. -class AnnotationInstance { +class AnnotationInstance : public InstanceBase { public: // Annotation Type. enum class AnnotationType { @@ -115,7 +126,7 @@ // FATAL, furthur processing of the text should be stopped. AuditorResult Deserialize(const std::vector<std::string>& serialized_lines, int start_line, - int end_line); + int end_line) override; // Protobuf of the annotation. traffic_annotation::NetworkTrafficAnnotation proto; @@ -134,7 +145,7 @@ // Holds an instance of calling a function that might have a network traffic // annotation argument. // TODO(rhalavati): Check if this class can also be reused in clang tool. -class CallInstance { +class CallInstance : public InstanceBase { public: CallInstance(); CallInstance(const CallInstance& other); @@ -152,7 +163,7 @@ // FATAL, further processing of the text should be stopped. AuditorResult Deserialize(const std::vector<std::string>& serialized_lines, int start_line, - int end_line); + int end_line) override; std::string file_path; uint32_t line_number; @@ -205,7 +216,7 @@ // texts. This list includes all unique ids that are defined in // net/traffic_annotation/network_traffic_annotation.h and // net/traffic_annotation/network_traffic_annotation_test_helper.h - const std::map<int, std::string>& GetReservedUniqueIDs(); + static const std::map<int, std::string>& GetReservedUniqueIDs(); std::string clang_tool_raw_output() const { return clang_tool_raw_output_; }; @@ -217,14 +228,20 @@ return extracted_annotations_; } + void SetExtractedAnnotationsForTest( + const std::vector<AnnotationInstance>& annotations) { + extracted_annotations_ = annotations; + } + const std::vector<CallInstance>& extracted_calls() const { return extracted_calls_; } const std::vector<AuditorResult>& errors() const { return errors_; } - private: + void ClearErrorsForTest() { errors_.clear(); } + private: const base::FilePath source_path_; const base::FilePath build_path_;
diff --git a/tools/traffic_annotation/auditor/traffic_annotation_auditor_ui.cc b/tools/traffic_annotation/auditor/traffic_annotation_auditor_ui.cc index 1cfc8d5..c1eb252 100644 --- a/tools/traffic_annotation/auditor/traffic_annotation_auditor_ui.cc +++ b/tools/traffic_annotation/auditor/traffic_annotation_auditor_ui.cc
@@ -183,7 +183,7 @@ } const std::map<int, std::string> reserved_ids = - auditor.GetReservedUniqueIDs(); + TrafficAnnotationAuditor::GetReservedUniqueIDs(); for (const auto& item : reserved_ids) items.push_back(item);
diff --git a/tools/traffic_annotation/auditor/traffic_annotation_auditor_unittest.cc b/tools/traffic_annotation/auditor/traffic_annotation_auditor_unittest.cc index ee350ab..5241b1e 100644 --- a/tools/traffic_annotation/auditor/traffic_annotation_auditor_unittest.cc +++ b/tools/traffic_annotation/auditor/traffic_annotation_auditor_unittest.cc
@@ -6,6 +6,8 @@ #include "base/files/file_path.h" #include "base/files/file_util.h" #include "base/path_service.h" +#include "base/strings/string_split.h" +#include "base/strings/string_util.h" #include "net/traffic_annotation/network_traffic_annotation.h" #include "testing/gtest/include/gtest/gtest.h" #include "tools/traffic_annotation/auditor/traffic_annotation_file_filter.h" @@ -37,21 +39,45 @@ return; } - git_list_mock_file_ = source_path_.Append(FILE_PATH_LITERAL("tools")) - .Append(FILE_PATH_LITERAL("traffic_annotation")) - .Append(FILE_PATH_LITERAL("auditor")) - .Append(FILE_PATH_LITERAL("tests")) - .Append(FILE_PATH_LITERAL("git_list.txt")); + tests_folder_ = source_path_.Append(FILE_PATH_LITERAL("tools")) + .Append(FILE_PATH_LITERAL("traffic_annotation")) + .Append(FILE_PATH_LITERAL("auditor")) + .Append(FILE_PATH_LITERAL("tests")); } + const base::FilePath source_path() const { return source_path_; } + const base::FilePath build_path() const { return build_path_; } + const base::FilePath tests_folder() const { return tests_folder_; }; + protected: + // Deserializes an annotation or a call instance from a sample file similar to + // clang tool outputs. + AuditorResult::ResultType Deserialize(const std::string& file_name, + InstanceBase* instance); + + private: base::FilePath source_path_; base::FilePath build_path_; // Currently stays empty. Will be set if access // to a compiled build directory would be // granted. - base::FilePath git_list_mock_file_; + base::FilePath tests_folder_; }; +AuditorResult::ResultType TrafficAnnotationAuditorTest::Deserialize( + const std::string& file_name, + InstanceBase* instance) { + std::string file_content; + EXPECT_TRUE(base::ReadFileToString( + tests_folder_.Append(FILE_PATH_LITERAL("extractor_outputs")) + .AppendASCII(file_name), + &file_content)); + base::RemoveChars(file_content, "\r", &file_content); + std::vector<std::string> lines = base::SplitString( + file_content, "\n", base::KEEP_WHITESPACE, base::SPLIT_WANT_ALL); + + return instance->Deserialize(lines, 0, static_cast<int>(lines.size())).type(); +} + // Tests if the two hash computation functions have the same result. TEST_F(TrafficAnnotationAuditorTest, HashFunctionCheck) { TEST_HASH_CODE("test"); @@ -67,8 +93,9 @@ // TrafficAnnotationFileFilter::IsFileRelevant. TEST_F(TrafficAnnotationAuditorTest, GetFilesFromGit) { TrafficAnnotationFileFilter filter; - filter.SetGitMockFileForTesting(git_list_mock_file_); - filter.GetFilesFromGit(source_path_); + filter.SetGitMockFileForTesting( + tests_folder().Append(FILE_PATH_LITERAL("git_list.txt"))); + filter.GetFilesFromGit(source_path()); const std::vector<std::string> git_files = filter.git_files(); @@ -88,8 +115,9 @@ // of files, given a mock git list file. TEST_F(TrafficAnnotationAuditorTest, RelevantFilesReceived) { TrafficAnnotationFileFilter filter; - filter.SetGitMockFileForTesting(git_list_mock_file_); - filter.GetFilesFromGit(source_path_); + filter.SetGitMockFileForTesting( + tests_folder().Append(FILE_PATH_LITERAL("git_list.txt"))); + filter.GetFilesFromGit(source_path()); unsigned int git_files_count = filter.git_files().size(); @@ -124,7 +152,7 @@ // Inherently checks if TrafficAnnotationFileFilter::LoadWhiteList works and // AuditorException rules are correctly deserialized. TEST_F(TrafficAnnotationAuditorTest, IsWhitelisted) { - TrafficAnnotationAuditor auditor(source_path_, build_path_); + TrafficAnnotationAuditor auditor(source_path(), build_path()); for (unsigned int i = 0; i < static_cast<unsigned int>( @@ -147,4 +175,149 @@ AuditorException::ExceptionType::MISSING)); EXPECT_TRUE(auditor.IsWhitelisted("net/url_request/url_request_context.cc", AuditorException::ExceptionType::MISSING)); -} \ No newline at end of file +} + +// Tests if annotation instances are corrrectly deserialized. +TEST_F(TrafficAnnotationAuditorTest, AnnotationDeserialization) { + struct AnnotationSample { + std::string file_name; + AuditorResult::ResultType result_type; + AnnotationInstance::AnnotationType annotation_type; + }; + + AnnotationSample test_cases[] = { + {"good_complete_annotation.txt", AuditorResult::ResultType::RESULT_OK, + AnnotationInstance::AnnotationType::ANNOTATION_COMPLETE}, + {"good_branched_completing_annotation.txt", + AuditorResult::ResultType::RESULT_OK, + AnnotationInstance::AnnotationType::ANNOTATION_BRANCHED_COMPLETING}, + {"good_completing_annotation.txt", AuditorResult::ResultType::RESULT_OK, + AnnotationInstance::AnnotationType::ANNOTATION_COMPLETENG}, + {"good_partial_annotation.txt", AuditorResult::ResultType::RESULT_OK, + AnnotationInstance::AnnotationType::ANNOTATION_PARTIAL}, + {"good_test_annotation.txt", AuditorResult::ResultType::RESULT_IGNORE}, + {"missing_annotation.txt", AuditorResult::ResultType::ERROR_MISSING}, + {"no_annotation.txt", AuditorResult::ResultType::ERROR_NO_ANNOTATION}, + {"fatal_annotation1.txt", AuditorResult::ResultType::ERROR_FATAL}, + {"fatal_annotation2.txt", AuditorResult::ResultType::ERROR_FATAL}, + {"fatal_annotation3.txt", AuditorResult::ResultType::ERROR_FATAL}, + {"bad_syntax_annotation1.txt", AuditorResult::ResultType::ERROR_SYNTAX}, + {"bad_syntax_annotation2.txt", AuditorResult::ResultType::ERROR_SYNTAX}, + {"bad_syntax_annotation3.txt", AuditorResult::ResultType::ERROR_SYNTAX}, + {"bad_syntax_annotation4.txt", AuditorResult::ResultType::ERROR_SYNTAX}, + }; + + for (const auto& test_case : test_cases) { + // Check if deserialization result is as expected. + AnnotationInstance annotation; + AuditorResult::ResultType result_type = + Deserialize(test_case.file_name, &annotation); + EXPECT_EQ(result_type, test_case.result_type); + + if (result_type == AuditorResult::ResultType::RESULT_OK) + EXPECT_EQ(annotation.annotation_type, test_case.annotation_type); + + // Content checks for one complete sample. + if (test_case.file_name != "good_complete_annotation.txt") + continue; + + EXPECT_EQ(annotation.proto.unique_id(), + "supervised_user_refresh_token_fetcher"); + EXPECT_EQ(annotation.proto.source().file(), + "chrome/browser/supervised_user/legacy/" + "supervised_user_refresh_token_fetcher.cc"); + EXPECT_EQ(annotation.proto.source().function(), "OnGetTokenSuccess"); + EXPECT_EQ(annotation.proto.source().line(), 166); + EXPECT_EQ(annotation.proto.semantics().sender(), "Supervised Users"); + EXPECT_EQ(annotation.proto.policy().cookies_allowed(), false); + } +} + +// Tests if call instances are corrrectly deserialized. +TEST_F(TrafficAnnotationAuditorTest, CallDeserialization) { + struct CallSample { + std::string file_name; + AuditorResult::ResultType result_type; + }; + + CallSample test_cases[] = { + {"good_call.txt", AuditorResult::ResultType::RESULT_OK}, + {"bad_call.txt", AuditorResult::ResultType::ERROR_FATAL}, + }; + + for (const auto& test_case : test_cases) { + // Check if deserialization result is as expected. + CallInstance call; + AuditorResult::ResultType result_type = + Deserialize(test_case.file_name, &call); + EXPECT_EQ(result_type, test_case.result_type); + + // Content checks for one complete sample. + if (test_case.file_name != "good_call.txt") + continue; + + EXPECT_EQ(call.file_path, "headless/public/util/http_url_fetcher.cc"); + EXPECT_EQ(call.line_number, 100u); + EXPECT_EQ(call.function_context, + "headless::HttpURLFetcher::Delegate::Delegate"); + EXPECT_EQ(call.function_name, "net::URLRequestContext::CreateRequest"); + EXPECT_EQ(call.is_annotated, true); + } +} + +// Tests if TrafficAnnotationAuditor::CheckDuplicateHashes works as expected. +TEST_F(TrafficAnnotationAuditorTest, CheckDuplicateHashes) { + // Load a valid annotation. + AnnotationInstance test_case; + EXPECT_EQ(Deserialize("good_complete_annotation.txt", &test_case), + AuditorResult::ResultType::RESULT_OK); + + const std::map<int, std::string>& reserved_words = + TrafficAnnotationAuditor::GetReservedUniqueIDs(); + + TrafficAnnotationAuditor auditor(source_path(), build_path()); + std::vector<AnnotationInstance> annotations; + + // Check for reserved words hash code duplication errors. + for (const auto& reserved_word : reserved_words) { + test_case.unique_id_hash_code = reserved_word.first; + annotations.push_back(test_case); + } + + auditor.SetExtractedAnnotationsForTest(annotations); + auditor.CheckDuplicateHashes(); + EXPECT_EQ(auditor.errors().size(), reserved_words.size()); + for (const auto& error : auditor.errors()) { + EXPECT_EQ(error.type(), + AuditorResult::ResultType::ERROR_RESERVED_UNIQUE_ID_HASH_CODE); + } + + // Check if several different hash codes result in no error. + annotations.clear(); + for (int i = 0; i < 10; i++) { + // Ensure that the test id is not a reserved hash code. + EXPECT_EQ(reserved_words.find(i), reserved_words.end()); + test_case.unique_id_hash_code = i; + annotations.push_back(test_case); + } + auditor.SetExtractedAnnotationsForTest(annotations); + auditor.ClearErrorsForTest(); + auditor.CheckDuplicateHashes(); + EXPECT_EQ(auditor.errors().size(), 0u); + + // Check if repeating the same hash codes results in errors. + annotations.clear(); + for (int i = 0; i < 10; i++) { + test_case.unique_id_hash_code = i; + annotations.push_back(test_case); + annotations.push_back(test_case); + } + auditor.SetExtractedAnnotationsForTest(annotations); + auditor.ClearErrorsForTest(); + auditor.CheckDuplicateHashes(); + EXPECT_EQ(auditor.errors().size(), 10u); + for (const auto& error : auditor.errors()) { + EXPECT_EQ(error.type(), + AuditorResult::ResultType::ERROR_DUPLICATE_UNIQUE_ID_HASH_CODE); + } +}
diff --git a/ui/file_manager/file_manager/foreground/css/file_manager.css b/ui/file_manager/file_manager/foreground/css/file_manager.css index f45347e..46259a7 100644 --- a/ui/file_manager/file_manager/foreground/css/file_manager.css +++ b/ui/file_manager/file_manager/foreground/css/file_manager.css
@@ -433,6 +433,16 @@ display: none; } +#selection-menu-button > .icon { + background-image: -webkit-image-set( + url(../images/files/ui/menu.png) 1x, + url(../images/files/ui/2x/menu.png) 2x); +} + +body:not(.check-select) #selection-menu-button { + display: none; +} + .cloud-import-combo-button { height: 32px; margin: 8px; @@ -1723,6 +1733,10 @@ -webkit-padding-end: 8px; } +cr-menu#file-context-menu.toolbar-menu > .hide-on-toolbar { + display: none; +} + cr-menu.chrome-menu hr { color: transparent; font-size: 0;
diff --git a/ui/file_manager/file_manager/foreground/images/files/ui/2x/menu.png b/ui/file_manager/file_manager/foreground/images/files/ui/2x/menu.png new file mode 100644 index 0000000..2b34db4 --- /dev/null +++ b/ui/file_manager/file_manager/foreground/images/files/ui/2x/menu.png Binary files differ
diff --git a/ui/file_manager/file_manager/foreground/images/files/ui/menu.png b/ui/file_manager/file_manager/foreground/images/files/ui/menu.png new file mode 100644 index 0000000..5fe650f --- /dev/null +++ b/ui/file_manager/file_manager/foreground/images/files/ui/menu.png Binary files differ
diff --git a/ui/file_manager/file_manager/foreground/js/compiled_resources2.gyp b/ui/file_manager/file_manager/foreground/js/compiled_resources2.gyp index b762652..a06d604 100644 --- a/ui/file_manager/file_manager/foreground/js/compiled_resources2.gyp +++ b/ui/file_manager/file_manager/foreground/js/compiled_resources2.gyp
@@ -189,6 +189,7 @@ 'quick_view_uma', 'scan_controller', 'search_controller', + 'selection_menu_controller', 'sort_menu_controller', 'spinner_controller', 'task_controller', @@ -504,6 +505,15 @@ 'includes': ['../../../compile_js2.gypi'], }, { + 'target_name': 'selection_menu_controller', + 'dependencies': [ + '../elements/compiled_resources2.gyp:files_toggle_ripple', + 'directory_model', + 'file_manager_commands', + ], + 'includes': ['../../../compile_js2.gypi'], + }, + { 'target_name': 'share_client', 'dependencies': [ '../../../externs/compiled_resources2.gyp:entry_location',
diff --git a/ui/file_manager/file_manager/foreground/js/file_manager.js b/ui/file_manager/file_manager/foreground/js/file_manager.js index e111901b..b83139c 100644 --- a/ui/file_manager/file_manager/foreground/js/file_manager.js +++ b/ui/file_manager/file_manager/foreground/js/file_manager.js
@@ -217,6 +217,14 @@ this.gearMenuController_ = null; /** + * Controller for the context menu opened by the action bar button in the + * check-select mode. + * @type {SelectionMenuController} + * @private + */ + this.selectionMenuController_ = null; + + /** * Toolbar controller. * @type {ToolbarController} * @private @@ -554,6 +562,9 @@ this.ui_.gearMenu, this.directoryModel_, this.commandHandler_); + this.selectionMenuController_ = new SelectionMenuController( + this.ui_.selectionMenuButton, + util.queryDecoratedElement('#file-context-menu', cr.ui.Menu)); this.toolbarController_ = new ToolbarController( this.ui_.toolbar, this.ui_.dialogNavigationList,
diff --git a/ui/file_manager/file_manager/foreground/js/main_scripts.js b/ui/file_manager/file_manager/foreground/js/main_scripts.js index 747437e9..4ca7f2a 100644 --- a/ui/file_manager/file_manager/foreground/js/main_scripts.js +++ b/ui/file_manager/file_manager/foreground/js/main_scripts.js
@@ -145,6 +145,7 @@ // <include src="quick_view_uma.js"> // <include src="scan_controller.js"> // <include src="search_controller.js"> +// <include src="selection_menu_controller.js"> // <include src="share_client.js"> // <include src="spinner_controller.js"> // <include src="task_controller.js">
diff --git a/ui/file_manager/file_manager/foreground/js/selection_menu_controller.js b/ui/file_manager/file_manager/foreground/js/selection_menu_controller.js new file mode 100644 index 0000000..7a613c4 --- /dev/null +++ b/ui/file_manager/file_manager/foreground/js/selection_menu_controller.js
@@ -0,0 +1,52 @@ +// Copyright 2017 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. + +/** + * @param {!cr.ui.MenuButton} selectionMenuButton + * @param {!cr.ui.Menu} menu + * @constructor + * @struct + */ +function SelectionMenuController(selectionMenuButton, menu) { + /** + * @type {!FilesToggleRipple} + * @const + * @private + */ + this.toggleRipple_ = + /** @type {!FilesToggleRipple} */ ( + queryRequiredElement('files-toggle-ripple', selectionMenuButton)); + + /** + * @type {!cr.ui.Menu} + * @const + */ + this.menu_ = menu; + + selectionMenuButton.addEventListener('menushow', this.onShowMenu_.bind(this)); + selectionMenuButton.addEventListener('menuhide', this.onHideMenu_.bind(this)); +} + +/** + * Class name to indicate if the menu was opened by the toolbar button or not. + * @type {string} + * @const + */ +SelectionMenuController.TOOLBAR_MENU = 'toolbar-menu'; + +/** + * @private + */ +SelectionMenuController.prototype.onShowMenu_ = function() { + this.menu_.classList.toggle(SelectionMenuController.TOOLBAR_MENU, true); + this.toggleRipple_.activated = true; +}; + +/** + * @private + */ +SelectionMenuController.prototype.onHideMenu_ = function() { + this.menu_.classList.toggle(SelectionMenuController.TOOLBAR_MENU, false); + this.toggleRipple_.activated = false; +};
diff --git a/ui/file_manager/file_manager/foreground/js/ui/actions_submenu.js b/ui/file_manager/file_manager/foreground/js/ui/actions_submenu.js index 1657f21c7..98238a95 100644 --- a/ui/file_manager/file_manager/foreground/js/ui/actions_submenu.js +++ b/ui/file_manager/file_manager/foreground/js/ui/actions_submenu.js
@@ -62,6 +62,7 @@ if (shareAction) { var menuItem = this.addMenuItem_({}); menuItem.command = '#share'; + menuItem.classList.toggle('hide-on-toolbar', true); delete remainingActions[ActionsModel.CommonActionId.SHARE]; } util.queryDecoratedElement('#share', cr.ui.Command).canExecuteChange();
diff --git a/ui/file_manager/file_manager/foreground/js/ui/file_manager_ui.js b/ui/file_manager/file_manager/foreground/js/ui/file_manager_ui.js index 1369f73..b445455 100644 --- a/ui/file_manager/file_manager/foreground/js/ui/file_manager_ui.js +++ b/ui/file_manager/file_manager/foreground/js/ui/file_manager_ui.js
@@ -220,6 +220,14 @@ this.gearMenu = new GearMenu(this.gearButton.menu); /** + * The button to open context menu in the check-select mode. + * @type {!cr.ui.MenuButton} + * @const + */ + this.selectionMenuButton = + util.queryDecoratedElement('#selection-menu-button', cr.ui.MenuButton); + + /** * Directory tree. * @type {DirectoryTree} */
diff --git a/ui/file_manager/file_manager/main.html b/ui/file_manager/file_manager/main.html index ca9b12b..0075dea 100644 --- a/ui/file_manager/file_manager/main.html +++ b/ui/file_manager/file_manager/main.html
@@ -173,10 +173,10 @@ <cr-menu id="file-context-menu" class="chrome-menu files-menu" menu-item-selector="cr-menu-item,hr" showShortcuts> <cr-menu-item id="default-task-menu-item" command="#default-task" - visibleif="full-page" hidden></cr-menu-item> + visibleif="full-page" class="hide-on-toolbar" hidden></cr-menu-item> <cr-menu-item command="#open-with" - visibleif="full-page" hidden></cr-menu-item> - <hr id="tasks-separator" visibleif="full-page" hidden> + visibleif="full-page" class="hide-on-toolbar" hidden></cr-menu-item> + <hr id="tasks-separator" visibleif="full-page" class="hide-on-toolbar" hidden> <hr id="actions-separator" hidden> <cr-menu-item command="#cut" visibleif="full-page"></cr-menu-item> <cr-menu-item command="#copy" visibleif="full-page"></cr-menu-item> @@ -185,12 +185,13 @@ <hr visibleif="full-page"> <cr-menu-item command="#get-info"></cr-menu-item> <cr-menu-item command="#rename"></cr-menu-item> - <cr-menu-item command="#delete" i18n-content="DELETE_BUTTON_LABEL"></cr-menu-item> + <cr-menu-item command="#delete" i18n-content="DELETE_BUTTON_LABEL" + class="hide-on-toolbar"></cr-menu-item> <cr-menu-item command="#zip-selection"></cr-menu-item> <cr-menu-item command="#set-wallpaper"></cr-menu-item> - <hr visibleif="saveas-file full-page"> - <cr-menu-item command="#new-folder" - visibleif="saveas-file full-page"></cr-menu-item> + <hr visibleif="saveas-file full-page" class="hide-on-toolbar"> + <cr-menu-item command="#new-folder" visibleif="saveas-file full-page" + class="hide-on-toolbar"></cr-menu-item> </cr-menu> <cr-menu id="roots-context-menu" class="chrome-menu files-menu" @@ -380,6 +381,12 @@ <files-toggle-ripple></files-toggle-ripple> <div class="icon"></div> </button> + <!-- TODO(yamaguchi): Set a proper tooltip. --> + <button id="selection-menu-button" class="icon-button" tabindex="19" + menu="#file-context-menu" aria-activedescendant="file-context-menu"> + <files-toggle-ripple></files-toggle-ripple> + <div class="icon"></div> + </button> </div> <div id="cloud-import-details" class="hidden" hidden>
diff --git a/ui/keyboard/keyboard_util.cc b/ui/keyboard/keyboard_util.cc index 0df61bca..15b70b0 100644 --- a/ui/keyboard/keyboard_util.cc +++ b/ui/keyboard/keyboard_util.cc
@@ -44,6 +44,7 @@ CHECK(!details.dispatcher_destroyed); } +bool g_keyboard_load_time_logged = false; base::LazyInstance<base::Time>::DestructorAtExit g_keyboard_load_time_start = LAZY_INSTANCE_INITIALIZER; @@ -282,7 +283,7 @@ } void MarkKeyboardLoadStarted() { - if (g_keyboard_load_time_start.Get().is_null()) + if (!g_keyboard_load_time_logged) g_keyboard_load_time_start.Get() = base::Time::Now(); } @@ -292,13 +293,12 @@ if (g_keyboard_load_time_start.Get().is_null()) return; - static bool logged = false; - if (!logged) { + if (!g_keyboard_load_time_logged) { // Log the delta only once. UMA_HISTOGRAM_TIMES( "VirtualKeyboard.InitLatency.FirstLoad", base::Time::Now() - g_keyboard_load_time_start.Get()); - logged = true; + g_keyboard_load_time_logged = true; } }
