diff --git a/DEPS b/DEPS index 4d1d781..090f44c 100644 --- a/DEPS +++ b/DEPS
@@ -40,7 +40,7 @@ # Three lines of non-changing comments so that # the commit queue can handle CLs rolling Skia # and whatever else without interference from each other. - 'skia_revision': '99d4721171f9b8f23cd907ef3da938c4c5579ae9', + 'skia_revision': 'd689f7a5313a7fd501c2572c493fe6d91a88cba4', # Three lines of non-changing comments so that # the commit queue can handle CLs rolling V8 # and whatever else without interference from each other.
diff --git a/base/json/json_parser.cc b/base/json/json_parser.cc index e85c3d25..814b9c4 100644 --- a/base/json/json_parser.cc +++ b/base/json/json_parser.cc
@@ -144,6 +144,12 @@ explicit JSONStringValue(StringPiece piece) : Value(Type::STRING), string_piece_(piece) {} + ~JSONStringValue() override { + // Ugly hack that prevents ~Value() from trying to destroy string_value_. + // TODO(crbug.com/646113): Clean this up when StringValue will be removed. + type_ = Type::NONE; + } + // Overridden from Value: bool GetAsString(std::string* out_value) const override { string_piece_.CopyToString(out_value); @@ -153,6 +159,12 @@ *out_value = UTF8ToUTF16(string_piece_); return true; } + // base::Value::GetAsString contains a proper implementation now, so the old + // behavior is copied here. + // TODO(crbug.com/646113): Clean this up when StringValue will be removed. + bool GetAsString(const StringValue** out_value) const override { + return false; + } bool GetAsString(StringPiece* out_value) const override { *out_value = string_piece_; return true;
diff --git a/base/test/values_test_util.h b/base/test/values_test_util.h index 886c17d..04af92e5 100644 --- a/base/test/values_test_util.h +++ b/base/test/values_test_util.h
@@ -13,8 +13,8 @@ namespace base { class DictionaryValue; class ListValue; -class StringValue; class Value; +using StringValue = Value; // All the functions below expect that the value for the given key in // the given dictionary equals the given expected value.
diff --git a/base/value_conversions.h b/base/value_conversions.h index 452c587..6ce1bff 100644 --- a/base/value_conversions.h +++ b/base/value_conversions.h
@@ -14,8 +14,8 @@ class FilePath; class TimeDelta; -class StringValue; class Value; +using StringValue = Value; // The caller takes ownership of the returned value. BASE_EXPORT StringValue* CreateFilePathValue(const FilePath& in_value);
diff --git a/base/values.cc b/base/values.cc index 82b137a..145c0c11 100644 --- a/base/values.cc +++ b/base/values.cc
@@ -77,7 +77,8 @@ bool IsAssignmentSafe(Value::Type lhs, Value::Type rhs) { auto IsImplemented = [](Value::Type type) { return type == Value::Type::NONE || type == Value::Type::BOOLEAN || - type == Value::Type::INTEGER || type == Value::Type::DOUBLE; + type == Value::Type::INTEGER || type == Value::Type::DOUBLE || + type == Value::Type::STRING; }; return lhs == rhs || (IsImplemented(lhs) && IsImplemented(rhs)); @@ -91,13 +92,11 @@ } Value::Value(const Value& that) { - InternalCopyFrom(that); + InternalCopyConstructFrom(that); } Value::Value(Value&& that) { - // TODO(crbug.com/646113): Implement InternalMoveFrom for types where moving - // and copying differ. - InternalCopyFrom(that); + InternalMoveConstructFrom(std::move(that)); } Value::Value() : type_(Type::NONE) {} @@ -117,10 +116,12 @@ case Type::DOUBLE: double_value_ = 0.0; return; + case Type::STRING: + string_value_.Init(); + return; // TODO(crbug.com/646113): Implement these once the corresponding derived // classes are removed. - case Type::STRING: case Type::BINARY: case Type::LIST: case Type::DICTIONARY: @@ -140,10 +141,40 @@ } } +Value::Value(const char* in_string) : type_(Type::STRING) { + string_value_.Init(in_string); + DCHECK(IsStringUTF8(*string_value_)); +} + +Value::Value(const std::string& in_string) : type_(Type::STRING) { + string_value_.Init(in_string); + DCHECK(IsStringUTF8(*string_value_)); +} + +Value::Value(std::string&& in_string) : type_(Type::STRING) { + string_value_.Init(std::move(in_string)); + DCHECK(IsStringUTF8(*string_value_)); +} + +Value::Value(const char16* in_string) : type_(Type::STRING) { + string_value_.Init(UTF16ToUTF8(in_string)); +} + +Value::Value(const string16& in_string) : type_(Type::STRING) { + string_value_.Init(UTF16ToUTF8(in_string)); +} + +Value::Value(StringPiece in_string) : Value(in_string.as_string()) {} + Value& Value::operator=(const Value& that) { if (this != &that) { DCHECK(IsAssignmentSafe(type_, that.type_)); - InternalCopyFrom(that); + if (type_ == that.type_) { + InternalCopyAssignFrom(that); + } else { + InternalCleanup(); + InternalCopyConstructFrom(that); + } } return *this; @@ -151,16 +182,21 @@ Value& Value::operator=(Value&& that) { if (this != &that) { - // TODO(crbug.com/646113): Implement InternalMoveFrom for types where moving - // and copying differ. DCHECK(IsAssignmentSafe(type_, that.type_)); - InternalCopyFrom(that); + if (type_ == that.type_) { + InternalMoveAssignFrom(std::move(that)); + } else { + InternalCleanup(); + InternalMoveConstructFrom(std::move(that)); + } } return *this; } -Value::~Value() {} +Value::~Value() { + InternalCleanup(); +} // static const char* Value::GetTypeName(Value::Type type) { @@ -188,6 +224,11 @@ return 0.0; } +const std::string& Value::GetString() const { + CHECK(is_string()); + return *string_value_; +} + bool Value::GetAsBoolean(bool* out_value) const { if (out_value && is_bool()) { *out_value = bool_value_; @@ -217,19 +258,35 @@ } bool Value::GetAsString(std::string* out_value) const { - return false; + if (out_value && is_string()) { + *out_value = *string_value_; + return true; + } + return is_string(); } bool Value::GetAsString(string16* out_value) const { - return false; + if (out_value && is_string()) { + *out_value = UTF8ToUTF16(*string_value_); + return true; + } + return is_string(); } bool Value::GetAsString(const StringValue** out_value) const { - return false; + if (out_value && is_string()) { + *out_value = static_cast<const StringValue*>(this); + return true; + } + return is_string(); } bool Value::GetAsString(StringPiece* out_value) const { - return false; + if (out_value && is_string()) { + *out_value = *string_value_; + return true; + } + return is_string(); } bool Value::GetAsBinary(const BinaryValue** out_value) const { @@ -267,6 +324,10 @@ return new FundamentalValue(int_value_); case Type::DOUBLE: return new FundamentalValue(double_value_); + // For now, make StringValues for backward-compatibility. Convert to + // Value when that code is deleted. + case Type::STRING: + return new StringValue(*string_value_); default: // All other types should be handled by subclasses. @@ -292,6 +353,11 @@ return int_value_ == other->int_value_; case Type::DOUBLE: return double_value_ == other->double_value_; + // TODO(crbug.com/646113): Simplify this once JSONStringValue is removed. + case Type::STRING: { + std::string lhs, rhs; + return GetAsString(&lhs) && other->GetAsString(&rhs) && lhs == rhs; + } default: // This method should only be getting called for the above types -- all // subclasses need to provide their own implementation;. @@ -307,8 +373,7 @@ return a->Equals(b); } -void Value::InternalCopyFrom(const Value& that) { - type_ = that.type_; +void Value::InternalCopyFundamentalValue(const Value& that) { switch (type_) { case Type::NONE: // Nothing to do. @@ -324,9 +389,28 @@ double_value_ = that.double_value_; return; + default: + NOTREACHED(); + } +} + +void Value::InternalCopyConstructFrom(const Value& that) { + type_ = that.type_; + + switch (type_) { + case Type::NONE: + case Type::BOOLEAN: + case Type::INTEGER: + case Type::DOUBLE: + InternalCopyFundamentalValue(that); + return; + + case Type::STRING: + string_value_.Init(*that.string_value_); + return; + // TODO(crbug.com/646113): Implement these once the corresponding derived // classes are removed. - case Type::STRING: case Type::BINARY: case Type::LIST: case Type::DICTIONARY: @@ -334,60 +418,98 @@ } } -///////////////////// StringValue //////////////////// +void Value::InternalMoveConstructFrom(Value&& that) { + type_ = that.type_; -StringValue::StringValue(StringPiece in_value) - : Value(Type::STRING), value_(in_value.as_string()) { - DCHECK(IsStringUTF8(in_value)); + switch (type_) { + case Type::NONE: + case Type::BOOLEAN: + case Type::INTEGER: + case Type::DOUBLE: + InternalCopyFundamentalValue(that); + return; + + case Type::STRING: + string_value_.InitFromMove(std::move(that.string_value_)); + return; + + // TODO(crbug.com/646113): Implement these once the corresponding derived + // classes are removed. + case Type::BINARY: + case Type::LIST: + case Type::DICTIONARY: + return; + } } -StringValue::StringValue(const string16& in_value) - : Value(Type::STRING), value_(UTF16ToUTF8(in_value)) {} +void Value::InternalCopyAssignFrom(const Value& that) { + type_ = that.type_; -StringValue::~StringValue() { + switch (type_) { + case Type::NONE: + case Type::BOOLEAN: + case Type::INTEGER: + case Type::DOUBLE: + InternalCopyFundamentalValue(that); + return; + + case Type::STRING: + *string_value_ = *that.string_value_; + return; + + // TODO(crbug.com/646113): Implement these once the corresponding derived + // classes are removed. + case Type::BINARY: + case Type::LIST: + case Type::DICTIONARY: + return; + } } -std::string* StringValue::GetString() { - return &value_; +void Value::InternalMoveAssignFrom(Value&& that) { + type_ = that.type_; + + switch (type_) { + case Type::NONE: + case Type::BOOLEAN: + case Type::INTEGER: + case Type::DOUBLE: + InternalCopyFundamentalValue(that); + return; + + case Type::STRING: + *string_value_ = std::move(*that.string_value_); + return; + + // TODO(crbug.com/646113): Implement these once the corresponding derived + // classes are removed. + case Type::BINARY: + case Type::LIST: + case Type::DICTIONARY: + return; + } } -const std::string& StringValue::GetString() const { - return value_; -} +void Value::InternalCleanup() { + switch (type_) { + case Type::NONE: + case Type::BOOLEAN: + case Type::INTEGER: + case Type::DOUBLE: + // Nothing to do + return; -bool StringValue::GetAsString(std::string* out_value) const { - if (out_value) - *out_value = value_; - return true; -} + case Type::STRING: + string_value_.Destroy(); + return; -bool StringValue::GetAsString(string16* out_value) const { - if (out_value) - *out_value = UTF8ToUTF16(value_); - return true; -} - -bool StringValue::GetAsString(const StringValue** out_value) const { - if (out_value) - *out_value = this; - return true; -} - -bool StringValue::GetAsString(StringPiece* out_value) const { - if (out_value) - *out_value = value_; - return true; -} - -StringValue* StringValue::DeepCopy() const { - return new StringValue(value_); -} - -bool StringValue::Equals(const Value* other) const { - if (other->GetType() != GetType()) - return false; - std::string lhs, rhs; - return GetAsString(&lhs) && other->GetAsString(&rhs) && lhs == rhs; + // TODO(crbug.com/646113): Implement these once the corresponding derived + // classes are removed. + case Type::BINARY: + case Type::LIST: + case Type::DICTIONARY: + return; + } } ///////////////////// BinaryValue ////////////////////
diff --git a/base/values.h b/base/values.h index 22d6b231..e021f7be 100644 --- a/base/values.h +++ b/base/values.h
@@ -30,6 +30,7 @@ #include "base/base_export.h" #include "base/compiler_specific.h" #include "base/macros.h" +#include "base/memory/manual_constructor.h" #include "base/strings/string16.h" #include "base/strings/string_piece.h" @@ -38,9 +39,9 @@ class BinaryValue; class DictionaryValue; class ListValue; -class StringValue; class Value; using FundamentalValue = Value; +using StringValue = Value; // The Value class is the base class for Values. A Value can be instantiated // via the Create*Value() factory methods, or by directly creating instances of @@ -71,6 +72,19 @@ explicit Value(int in_int); explicit Value(double in_double); + // Value(const char*) and Value(const char16*) are required despite + // Value(const std::string&) and Value(const string16&) because otherwise the + // compiler will choose the Value(bool) constructor for these arguments. + // Value(std::string&&) allow for efficient move construction. + // Value(StringPiece) exists due to many callsites passing StringPieces as + // arguments. + explicit Value(const char* in_string); + explicit Value(const std::string& in_string); + explicit Value(std::string&& in_string); + explicit Value(const char16* in_string); + explicit Value(const string16& in_string); + explicit Value(StringPiece in_string); + Value& operator=(const Value& that); Value& operator=(Value&& that); @@ -101,6 +115,7 @@ bool GetBool() const; int GetInt() const; double GetDouble() const; // Implicitly converts from int if necessary. + const std::string& GetString() 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 @@ -137,44 +152,26 @@ // NULLs are considered equal but different from Value::CreateNullValue(). static bool Equals(const Value* a, const Value* b); - private: - void InternalCopyFrom(const Value& that); - + protected: + // TODO(crbug.com/646113): Make this private once JSONStringValue is removed. Type type_; + private: + void InternalCopyFundamentalValue(const Value& that); + void InternalCopyConstructFrom(const Value& that); + void InternalMoveConstructFrom(Value&& that); + void InternalCopyAssignFrom(const Value& that); + void InternalMoveAssignFrom(Value&& that); + void InternalCleanup(); + union { bool bool_value_; int int_value_; double double_value_; + ManualConstructor<std::string> string_value_; }; }; -class BASE_EXPORT StringValue : public Value { - public: - // Initializes a StringValue with a UTF-8 narrow character string. - explicit StringValue(StringPiece in_value); - - // Initializes a StringValue with a string16. - explicit StringValue(const string16& in_value); - - ~StringValue() override; - - // Returns |value_| as a pointer or reference. - std::string* GetString(); - const std::string& GetString() const; - - // Overridden from Value: - bool GetAsString(std::string* out_value) const override; - bool GetAsString(string16* out_value) const override; - bool GetAsString(const StringValue** out_value) const override; - bool GetAsString(StringPiece* out_value) const override; - StringValue* DeepCopy() const override; - bool Equals(const Value* other) const override; - - private: - std::string value_; -}; - class BASE_EXPORT BinaryValue: public Value { public: // Creates a BinaryValue with a null buffer and size of 0. @@ -545,11 +542,6 @@ BASE_EXPORT std::ostream& operator<<(std::ostream& out, const Value& value); BASE_EXPORT inline std::ostream& operator<<(std::ostream& out, - const StringValue& value) { - return out << static_cast<const Value&>(value); -} - -BASE_EXPORT inline std::ostream& operator<<(std::ostream& out, const DictionaryValue& value) { return out << static_cast<const Value&>(value); }
diff --git a/base/values_unittest.cc b/base/values_unittest.cc index 511d9f983..9f2d9d62 100644 --- a/base/values_unittest.cc +++ b/base/values_unittest.cc
@@ -40,6 +40,48 @@ EXPECT_EQ(-4.655, value.GetDouble()); } +TEST(ValuesTest, ConstructStringFromConstCharPtr) { + const char* str = "foobar"; + StringValue value(str); + EXPECT_EQ(Value::Type::STRING, value.type()); + EXPECT_EQ("foobar", value.GetString()); +} + +TEST(ValuesTest, ConstructStringFromStdStringConstRef) { + std::string str = "foobar"; + StringValue value(str); + EXPECT_EQ(Value::Type::STRING, value.type()); + EXPECT_EQ("foobar", value.GetString()); +} + +TEST(ValuesTest, ConstructStringFromStdStringRefRef) { + std::string str = "foobar"; + StringValue value(std::move(str)); + EXPECT_EQ(Value::Type::STRING, value.type()); + EXPECT_EQ("foobar", value.GetString()); +} + +TEST(ValuesTest, ConstructStringFromConstChar16Ptr) { + string16 str = ASCIIToUTF16("foobar"); + StringValue value(str.c_str()); + EXPECT_EQ(Value::Type::STRING, value.type()); + EXPECT_EQ("foobar", value.GetString()); +} + +TEST(ValuesTest, ConstructStringFromString16) { + string16 str = ASCIIToUTF16("foobar"); + StringValue value(str); + EXPECT_EQ(Value::Type::STRING, value.type()); + EXPECT_EQ("foobar", value.GetString()); +} + +TEST(ValuesTest, ConstructStringFromStringPiece) { + StringPiece str = "foobar"; + StringValue value(str); + EXPECT_EQ(Value::Type::STRING, value.type()); + EXPECT_EQ("foobar", value.GetString()); +} + // Group of tests for the copy constructors and copy-assigmnent. For equality // checks comparisons of the interesting fields are done instead of relying on // Equals being correct. @@ -91,6 +133,19 @@ EXPECT_EQ(value.GetDouble(), blank.GetDouble()); } +TEST(ValuesTest, CopyString) { + StringValue value("foobar"); + StringValue copied_value(value); + EXPECT_EQ(value.type(), copied_value.type()); + EXPECT_EQ(value.GetString(), copied_value.GetString()); + + Value blank; + + blank = value; + EXPECT_EQ(value.type(), blank.type()); + EXPECT_EQ(value.GetString(), blank.GetString()); +} + // Group of tests for the move constructors and move-assigmnent. TEST(ValuesTest, MoveBool) { FundamentalValue true_value(true); @@ -140,6 +195,19 @@ EXPECT_EQ(654.38, blank.GetDouble()); } +TEST(ValuesTest, MoveString) { + StringValue value("foobar"); + StringValue moved_value(std::move(value)); + EXPECT_EQ(Value::Type::STRING, moved_value.type()); + EXPECT_EQ("foobar", moved_value.GetString()); + + Value blank; + + blank = StringValue("foobar"); + EXPECT_EQ(Value::Type::STRING, blank.type()); + EXPECT_EQ("foobar", blank.GetString()); +} + TEST(ValuesTest, Basic) { // Test basic dictionary getting/setting DictionaryValue settings;
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ChromeFeatureList.java b/chrome/android/java/src/org/chromium/chrome/browser/ChromeFeatureList.java index 5428229e..6e6102d 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/ChromeFeatureList.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/ChromeFeatureList.java
@@ -86,7 +86,6 @@ public static final String NTP_SNIPPETS_INCREASED_VISIBILITY = "NTPSnippetsIncreasedVisibility"; public static final String NTP_SNIPPETS_SAVE_TO_OFFLINE = "NTPSaveToOffline"; public static final String NTP_SNIPPETS_OFFLINE_BADGE = "NTPOfflineBadge"; - public static final String NTP_SUGGESTIONS_SECTION_DISMISSAL = "NTPSuggestionsSectionDismissal"; public static final String NTP_SUGGESTIONS_STANDALONE_UI = "NTPSuggestionsStandaloneUI"; public static final String SERVICE_WORKER_PAYMENT_APPS = "ServiceWorkerPaymentApps"; public static final String TAB_REPARENTING = "TabReparenting";
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SectionList.java b/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SectionList.java index 4d090a9b..26c56d1e 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SectionList.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SectionList.java
@@ -12,7 +12,6 @@ import org.chromium.chrome.browser.ntp.snippets.CategoryStatus.CategoryStatusEnum; import org.chromium.chrome.browser.ntp.snippets.SnippetArticle; import org.chromium.chrome.browser.ntp.snippets.SnippetsBridge; -import org.chromium.chrome.browser.ntp.snippets.SnippetsConfig; import org.chromium.chrome.browser.ntp.snippets.SuggestionsSource; import org.chromium.chrome.browser.offlinepages.OfflinePageBridge; import org.chromium.chrome.browser.suggestions.SuggestionsRanker; @@ -222,8 +221,6 @@ */ @Override public void dismissSection(SuggestionsSection section) { - assert SnippetsConfig.isSectionDismissalEnabled(); - mUiDelegate.getSuggestionsSource().dismissCategory(section.getCategory()); removeSection(section); }
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSection.java b/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSection.java index c085f9fd..b3eca40 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSection.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSection.java
@@ -15,7 +15,6 @@ import org.chromium.chrome.browser.ntp.snippets.SnippetArticle; import org.chromium.chrome.browser.ntp.snippets.SnippetArticleViewHolder; import org.chromium.chrome.browser.ntp.snippets.SnippetsBridge; -import org.chromium.chrome.browser.ntp.snippets.SnippetsConfig; import org.chromium.chrome.browser.ntp.snippets.SuggestionsSource; import org.chromium.chrome.browser.offlinepages.ClientId; import org.chromium.chrome.browser.offlinepages.OfflinePageBridge; @@ -468,9 +467,7 @@ * (as opposed to individual items in it). */ private Set<Integer> getSectionDismissalRange() { - if (hasSuggestions() || !SnippetsConfig.isSectionDismissalEnabled()) { - return Collections.emptySet(); - } + if (hasSuggestions()) return Collections.emptySet(); if (!mMoreButton.isVisible()) { assert getStartingOffsetForChild(mStatus) == 1;
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetsConfig.java b/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetsConfig.java index 6ea61fe0..26a1ff1 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetsConfig.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/ntp/snippets/SnippetsConfig.java
@@ -20,10 +20,6 @@ return ChromeFeatureList.isEnabled(ChromeFeatureList.NTP_SNIPPETS_OFFLINE_BADGE); } - public static boolean isSectionDismissalEnabled() { - return ChromeFeatureList.isEnabled(ChromeFeatureList.NTP_SUGGESTIONS_SECTION_DISMISSAL); - } - /** https://crbug.com/660837 */ public static boolean isIncreasedCardVisibilityEnabled() { return ChromeFeatureList.isEnabled(ChromeFeatureList.NTP_SNIPPETS_INCREASED_VISIBILITY);
diff --git a/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java b/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java index 52125c7..6cd872f 100644 --- a/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java +++ b/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/NewTabPageAdapterTest.java
@@ -50,7 +50,6 @@ import org.chromium.base.ContextUtils; import org.chromium.base.test.util.Feature; import org.chromium.chrome.R; -import org.chromium.chrome.browser.ChromeFeatureList; import org.chromium.chrome.browser.DisableHistogramsRule; import org.chromium.chrome.browser.EnableFeatures; import org.chromium.chrome.browser.ntp.ContextMenuManager; @@ -822,7 +821,6 @@ @Test @Feature({"Ntp"}) - @EnableFeatures(ChromeFeatureList.NTP_SUGGESTIONS_SECTION_DISMISSAL) public void testAllDismissedVisibility() { ArgumentCaptor<DestructionObserver> observers = ArgumentCaptor.forClass(DestructionObserver.class);
diff --git a/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSectionTest.java b/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSectionTest.java index 198e424e..3e10559 100644 --- a/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSectionTest.java +++ b/chrome/android/junit/src/org/chromium/chrome/browser/ntp/cards/SuggestionsSectionTest.java
@@ -37,7 +37,6 @@ import org.chromium.base.Callback; import org.chromium.base.test.util.Feature; -import org.chromium.chrome.browser.ChromeFeatureList; import org.chromium.chrome.browser.DisableHistogramsRule; import org.chromium.chrome.browser.EnableFeatures; import org.chromium.chrome.browser.ntp.cards.ContentSuggestionsTestUtils.CategoryInfoBuilder; @@ -92,7 +91,6 @@ @Test @Feature({"Ntp"}) - @EnableFeatures(ChromeFeatureList.NTP_SUGGESTIONS_SECTION_DISMISSAL) public void testDismissSibling() { List<SnippetArticle> snippets = createDummySuggestions(3, TEST_CATEGORY_ID); SuggestionsSection section = createSectionWithReloadAction(true); @@ -118,32 +116,6 @@ @Test @Feature({"Ntp"}) - @EnableFeatures({}) - public void testDismissSiblingWithSectionDismissalDisabled() { - List<SnippetArticle> snippets = createDummySuggestions(3, TEST_CATEGORY_ID); - SuggestionsSection section = createSectionWithReloadAction(true); - - section.setStatus(CategoryStatus.AVAILABLE); - assertNotNull(section.getActionItemForTesting()); - - // Without snippets. - assertEquals(ItemViewType.HEADER, section.getItemViewType(0)); - assertEquals(Collections.emptySet(), section.getItemDismissalGroup(0)); - assertEquals(ItemViewType.STATUS, section.getItemViewType(1)); - assertEquals(Collections.emptySet(), section.getItemDismissalGroup(1)); - assertEquals(ItemViewType.ACTION, section.getItemViewType(2)); - assertEquals(Collections.emptySet(), section.getItemDismissalGroup(2)); - - // With snippets. - section.setSuggestions(snippets, CategoryStatus.AVAILABLE, /* replaceExisting = */ true); - assertEquals(ItemViewType.HEADER, section.getItemViewType(0)); - assertEquals(Collections.emptySet(), section.getItemDismissalGroup(0)); - assertEquals(ItemViewType.SNIPPET, section.getItemViewType(1)); - assertEquals(Collections.singleton(1), section.getItemDismissalGroup(1)); - } - - @Test - @Feature({"Ntp"}) public void testAddSuggestionsNotification() { final int suggestionCount = 5; List<SnippetArticle> snippets = createDummySuggestions(suggestionCount, @@ -259,7 +231,6 @@ @Test @Feature({"Ntp"}) - @EnableFeatures({ChromeFeatureList.NTP_SUGGESTIONS_SECTION_DISMISSAL}) public void testDismissSection() { SuggestionsSection section = createSectionWithReloadAction(false); section.setStatus(CategoryStatus.AVAILABLE); @@ -724,7 +695,6 @@ @Test @Feature({"Ntp"}) - @EnableFeatures(ChromeFeatureList.NTP_SUGGESTIONS_SECTION_DISMISSAL) public void testGetItemDismissalGroupWithSuggestions() { List<SnippetArticle> suggestions = createDummySuggestions(5, TEST_CATEGORY_ID); SuggestionsSection section = createSectionWithReloadAction(false); @@ -736,7 +706,6 @@ @Test @Feature({"Ntp"}) - @EnableFeatures(ChromeFeatureList.NTP_SUGGESTIONS_SECTION_DISMISSAL) public void testGetItemDismissalGroupWithActionItem() { SuggestionsSection section = createSectionWithReloadAction(true); assertThat(section.getItemDismissalGroup(1).size(), is(2)); @@ -745,7 +714,6 @@ @Test @Feature({"Ntp"}) - @EnableFeatures(ChromeFeatureList.NTP_SUGGESTIONS_SECTION_DISMISSAL) public void testGetItemDismissalGroupWithoutActionItem() { SuggestionsSection section = createSectionWithReloadAction(false); assertThat(section.getItemDismissalGroup(1).size(), is(1));
diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd index a969e90..ccf4d0a4 100644 --- a/chrome/app/generated_resources.grd +++ b/chrome/app/generated_resources.grd
@@ -6761,18 +6761,6 @@ <message name="IDS_FLAGS_DATASAVER_PROMPT_DEMO_MODE" desc="The value of the Data Saver prompt flag which always shows prompt on network properties change."> Demo mode </message> - <message name="IDS_FLAGS_SUPERVISED_USER_SAFESITES_NAME" desc="Name of the flag to enable/disable the supervised user SafeSites filtering feature."> - Child account SafeSites filtering - </message> - <message name="IDS_FLAGS_SUPERVISED_USER_SAFESITES_DESCRIPTION" desc="Description for the flag to enable/disable the supervised user SafeSites filtering feature."> - Enable or disable SafeSites filtering for child accounts. - </message> - <message name="IDS_SUPERVISED_USER_SAFESITES_BLACKLIST_ONLY" desc="Description for the choice to enable only the static blacklist in the supervised user SafeSites filtering feature."> - Static blacklist only - </message> - <message name="IDS_SUPERVISED_USER_SAFESITES_ONLINE_CHECK_ONLY" desc="Description for the choice to enable only the online check in the supervised user SafeSites filtering feature."> - Online check only - </message> <message name="IDS_FLAGS_DISABLE_UNIFIED_MEDIA_PIPELINE_NAME" desc="Title for the flag to disable the unified media pipeline on Android."> Disables the unified media pipeline on Android. </message> @@ -8049,7 +8037,7 @@ <!-- Supervised User Block Interstitial data --> <message name="IDS_BLOCK_INTERSTITIAL_DEFAULT_FEEDBACK_TEXT" desc="The default text for feedback about a blocked site."> - <ph name="REASON">$1<ex>This site was blocked due to the SafeSites filter.</ex></ph> + <ph name="REASON">$1<ex>It may have mature content.</ex></ph> I don't think this site should be blocked! </message>
diff --git a/chrome/browser/about_flags.cc b/chrome/browser/about_flags.cc index 78ebb3d..d9688de 100644 --- a/chrome/browser/about_flags.cc +++ b/chrome/browser/about_flags.cc
@@ -418,22 +418,6 @@ }; #endif // OS_CHROMEOS -const FeatureEntry::Choice kSupervisedUserSafeSitesChoices[] = { - { IDS_GENERIC_EXPERIMENT_CHOICE_DEFAULT, "", "" }, - { IDS_GENERIC_EXPERIMENT_CHOICE_ENABLED, - switches::kSupervisedUserSafeSites, - "enabled" }, - { IDS_GENERIC_EXPERIMENT_CHOICE_DISABLED, - switches::kSupervisedUserSafeSites, - "disabled" }, - { IDS_SUPERVISED_USER_SAFESITES_BLACKLIST_ONLY, - switches::kSupervisedUserSafeSites, - "blacklist-only" }, - { IDS_SUPERVISED_USER_SAFESITES_ONLINE_CHECK_ONLY, - switches::kSupervisedUserSafeSites, - "online-check-only" } -}; - const FeatureEntry::Choice kV8CacheOptionsChoices[] = { { IDS_GENERIC_EXPERIMENT_CHOICE_DEFAULT, "", "" }, { IDS_GENERIC_EXPERIMENT_CHOICE_DISABLED, switches::kV8CacheOptions, "none" }, @@ -1600,9 +1584,6 @@ IDS_FLAGS_DATASAVER_PROMPT_DESCRIPTION, kOsCrOS, MULTI_VALUE_TYPE(kDataSaverPromptChoices)}, #endif // OS_CHROMEOS - {"supervised-user-safesites", IDS_FLAGS_SUPERVISED_USER_SAFESITES_NAME, - IDS_FLAGS_SUPERVISED_USER_SAFESITES_DESCRIPTION, kOsAll, - MULTI_VALUE_TYPE(kSupervisedUserSafeSitesChoices)}, #if defined(OS_ANDROID) {"enable-autofill-keyboard-accessory-view", IDS_FLAGS_AUTOFILL_ACCESSORY_VIEW_NAME,
diff --git a/chrome/browser/android/chrome_feature_list.cc b/chrome/browser/android/chrome_feature_list.cc index 593b2ca1..9854124 100644 --- a/chrome/browser/android/chrome_feature_list.cc +++ b/chrome/browser/android/chrome_feature_list.cc
@@ -65,7 +65,6 @@ &ntp_snippets::kForeignSessionsSuggestionsFeature, &ntp_snippets::kOfflineBadgeFeature, &ntp_snippets::kSaveToOfflineFeature, - &ntp_snippets::kSectionDismissalFeature, &offline_pages::kBackgroundLoaderForDownloadsFeature, &offline_pages::kOfflinePagesCTFeature, // See crbug.com/620421. &offline_pages::kOfflinePagesSharingFeature,
diff --git a/chrome/browser/android/vr_shell/OWNERS b/chrome/browser/android/vr_shell/OWNERS index 1b51b3c..0b3ab0a 100644 --- a/chrome/browser/android/vr_shell/OWNERS +++ b/chrome/browser/android/vr_shell/OWNERS
@@ -4,3 +4,4 @@ bshe@chromium.org girard@chromium.org mthiesse@chromium.org +cjgrant@chromium.org
diff --git a/chrome/browser/chromeos/input_method/candidate_window_controller_impl.cc b/chrome/browser/chromeos/input_method/candidate_window_controller_impl.cc index 1c84d39..a291ad1 100644 --- a/chrome/browser/chromeos/input_method/candidate_window_controller_impl.cc +++ b/chrome/browser/chromeos/input_method/candidate_window_controller_impl.cc
@@ -12,6 +12,7 @@ #include "ash/wm/window_util.h" #include "base/logging.h" #include "chrome/browser/chromeos/input_method/mode_indicator_controller.h" +#include "chrome/browser/ui/ash/ash_util.h" #include "ui/base/ime/ime_bridge.h" #include "ui/chromeos/ime/infolist_window.h" #include "ui/views/widget/widget.h" @@ -44,12 +45,19 @@ if (candidate_window_view_) return; - aura::Window* active_window = ash::wm::GetActiveWindow(); - candidate_window_view_ = - new ui::ime::CandidateWindowView(ash::Shell::GetContainer( - active_window ? active_window->GetRootWindow() - : ash::Shell::GetTargetRootWindow(), - ash::kShellWindowId_SettingBubbleContainer)); + // TODO(moshayedi): crbug.com/684658. Setting parent is nullptr in mash is + // just for the sake of not crashing. It doesn't provide the same behaviour + // as we have in ChromeOS. For example, candidate pop-up disappears when + // dragging the window in mash, but it shouldn't. + gfx::NativeView parent = nullptr; + if (!chrome::IsRunningInMash()) { + aura::Window* active_window = ash::wm::GetActiveWindow(); + parent = ash::Shell::GetContainer( + active_window ? active_window->GetRootWindow() + : ash::Shell::GetTargetRootWindow(), + ash::kShellWindowId_SettingBubbleContainer); + } + candidate_window_view_ = new ui::ime::CandidateWindowView(parent); candidate_window_view_->AddObserver(this); candidate_window_view_->SetCursorBounds(cursor_bounds_, composition_head_); views::Widget* widget = candidate_window_view_->InitWidget();
diff --git a/chrome/browser/chromeos/login/screens/user_image_model.cc b/chrome/browser/chromeos/login/screens/user_image_model.cc index 797a4d5b..28604c3 100644 --- a/chrome/browser/chromeos/login/screens/user_image_model.cc +++ b/chrome/browser/chromeos/login/screens/user_image_model.cc
@@ -12,6 +12,7 @@ const char UserImageModel::kContextKeyProfilePictureDataURL[] = "profilePictureDataURL"; const char UserImageModel::kContextKeySelectedImageURL[] = "selectedImageURL"; +const char UserImageModel::kContextKeyHasGaiaAccount[] = "hasGaiaAccount"; UserImageModel::UserImageModel(BaseScreenDelegate* base_screen_delegate) : BaseScreen(base_screen_delegate, OobeScreen::SCREEN_USER_IMAGE_PICKER) {}
diff --git a/chrome/browser/chromeos/login/screens/user_image_model.h b/chrome/browser/chromeos/login/screens/user_image_model.h index 9e9a0617..2f75110 100644 --- a/chrome/browser/chromeos/login/screens/user_image_model.h +++ b/chrome/browser/chromeos/login/screens/user_image_model.h
@@ -19,6 +19,7 @@ static const char kContextKeyIsCameraPresent[]; static const char kContextKeySelectedImageURL[]; static const char kContextKeyProfilePictureDataURL[]; + static const char kContextKeyHasGaiaAccount[]; explicit UserImageModel(BaseScreenDelegate* base_screen_delegate); ~UserImageModel() override;
diff --git a/chrome/browser/chromeos/login/screens/user_image_screen.cc b/chrome/browser/chromeos/login/screens/user_image_screen.cc index 99352fe..73e2202 100644 --- a/chrome/browser/chromeos/login/screens/user_image_screen.cc +++ b/chrome/browser/chromeos/login/screens/user_image_screen.cc
@@ -280,8 +280,14 @@ kContextKeySelectedImageURL, default_user_image::GetDefaultImageUrl(selected_image_)); - // Start fetching the profile image. - GetUserImageManager()->DownloadProfileImage(kProfileDownloadReason); + const user_manager::User* user = GetUser(); + // Fetch profile image for GAIA accounts. + if (user && user->HasGaiaAccount()) { + GetContextEditor().SetBoolean(kContextKeyHasGaiaAccount, true); + GetUserImageManager()->DownloadProfileImage(kProfileDownloadReason); + } else { + GetContextEditor().SetBoolean(kContextKeyHasGaiaAccount, false); + } } void UserImageScreen::Hide() {
diff --git a/chrome/browser/chromeos/settings/system_settings_provider.h b/chrome/browser/chromeos/settings/system_settings_provider.h index 802beb5..0a59bc5 100644 --- a/chrome/browser/chromeos/settings/system_settings_provider.h +++ b/chrome/browser/chromeos/settings/system_settings_provider.h
@@ -15,7 +15,8 @@ #include "third_party/icu/source/i18n/unicode/timezone.h" namespace base { -class StringValue; +class Value; +using StringValue = Value; } namespace chromeos {
diff --git a/chrome/browser/extensions/api/streams_private/streams_private_api.cc b/chrome/browser/extensions/api/streams_private/streams_private_api.cc index dc12dac..21c333a6 100644 --- a/chrome/browser/extensions/api/streams_private/streams_private_api.cc +++ b/chrome/browser/extensions/api/streams_private/streams_private_api.cc
@@ -37,9 +37,8 @@ while (headers->EnumerateHeaderLines(&iter, &header_name, &header_value)) { base::Value* existing_value = NULL; if (result->Get(header_name, &existing_value)) { - base::StringValue* existing_string_value = - static_cast<base::StringValue*>(existing_value); - existing_string_value->GetString()->append(", ").append(header_value); + *existing_value = + base::Value(existing_value->GetString() + ", " + header_value); } else { result->SetString(header_name, header_value); }
diff --git a/chrome/browser/extensions/webstore_data_fetcher.cc b/chrome/browser/extensions/webstore_data_fetcher.cc index c914ff3..e8e8b69 100644 --- a/chrome/browser/extensions/webstore_data_fetcher.cc +++ b/chrome/browser/extensions/webstore_data_fetcher.cc
@@ -37,15 +37,26 @@ WebstoreDataFetcher::~WebstoreDataFetcher() {} +void WebstoreDataFetcher::SetJsonPostData(const std::string& json) { + json_post_data_ = json; +} + void WebstoreDataFetcher::Start() { GURL webstore_data_url(extension_urls::GetWebstoreItemJsonDataURL(id_)); - + net::URLFetcher::RequestType request_type = + json_post_data_.empty() ? net::URLFetcher::GET : net::URLFetcher::POST; webstore_data_url_fetcher_ = - net::URLFetcher::Create(webstore_data_url, net::URLFetcher::GET, this); + net::URLFetcher::Create(webstore_data_url, request_type, this); webstore_data_url_fetcher_->SetRequestContext(request_context_); webstore_data_url_fetcher_->SetReferrer(referrer_url_.spec()); webstore_data_url_fetcher_->SetLoadFlags(net::LOAD_DO_NOT_SAVE_COOKIES | net::LOAD_DISABLE_CACHE); + + if (!json_post_data_.empty()) { + webstore_data_url_fetcher_->SetUploadData("application/json", + json_post_data_); + } + if (max_auto_retries_ > 0) { webstore_data_url_fetcher_->SetMaxRetriesOn5xx(max_auto_retries_); webstore_data_url_fetcher_->SetAutomaticallyRetryOnNetworkChanges(
diff --git a/chrome/browser/extensions/webstore_data_fetcher.h b/chrome/browser/extensions/webstore_data_fetcher.h index bcb48a5c..7179fb7c 100644 --- a/chrome/browser/extensions/webstore_data_fetcher.h +++ b/chrome/browser/extensions/webstore_data_fetcher.h
@@ -37,6 +37,10 @@ const std::string webstore_item_id); ~WebstoreDataFetcher() override; + // Makes this request use a POST instead of GET, and sends |json| in the + // body of the request. If |json| is empty, this is a no-op. + void SetJsonPostData(const std::string& json); + void Start(); void set_max_auto_retries(int max_retries) { @@ -54,6 +58,7 @@ net::URLRequestContextGetter* request_context_; GURL referrer_url_; std::string id_; + std::string json_post_data_; // For fetching webstore JSON data. std::unique_ptr<net::URLFetcher> webstore_data_url_fetcher_;
diff --git a/chrome/browser/extensions/webstore_inline_installer.cc b/chrome/browser/extensions/webstore_inline_installer.cc index 9e992bc..38931a3 100644 --- a/chrome/browser/extensions/webstore_inline_installer.cc +++ b/chrome/browser/extensions/webstore_inline_installer.cc
@@ -4,11 +4,17 @@ #include "chrome/browser/extensions/webstore_inline_installer.h" +#include <utility> + +#include "base/json/json_writer.h" #include "base/strings/stringprintf.h" +#include "base/values.h" +#include "chrome/browser/extensions/webstore_data_fetcher.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/ui/browser_finder.h" #include "chrome/browser/ui/exclusive_access/fullscreen_controller.h" #include "content/public/browser/navigation_details.h" +#include "content/public/browser/navigation_entry.h" #include "content/public/browser/web_contents.h" using content::WebContents; @@ -95,6 +101,41 @@ return true; } +std::string WebstoreInlineInstaller::GetJsonPostData() { + // web_contents() might return null during tab destruction. This object would + // also be destroyed shortly thereafter but check to be on the safe side. + if (!web_contents()) + return std::string(); + + content::NavigationController& navigation_controller = + web_contents()->GetController(); + content::NavigationEntry* navigation_entry = + navigation_controller.GetLastCommittedEntry(); + + if (navigation_entry) { + const std::vector<GURL>& redirect_urls = + navigation_entry->GetRedirectChain(); + + if (!redirect_urls.empty()) { + base::DictionaryValue dictionary; + dictionary.SetString("id", id()); + dictionary.SetString("referrer", requestor_url_.spec()); + std::unique_ptr<base::ListValue> redirect_chain = + base::MakeUnique<base::ListValue>(); + for (const GURL& url : redirect_urls) { + redirect_chain->AppendString(url.spec()); + } + dictionary.Set("redirect_chain", std::move(redirect_chain)); + + std::string json; + base::JSONWriter::Write(dictionary, &json); + return json; + } + } + + return std::string(); +} + bool WebstoreInlineInstaller::CheckRequestorAlive() const { // The frame or tab may have gone away - cancel installation in that case. return host_ != nullptr && web_contents() != nullptr;
diff --git a/chrome/browser/extensions/webstore_inline_installer.h b/chrome/browser/extensions/webstore_inline_installer.h index 9e11fa4..bc711be 100644 --- a/chrome/browser/extensions/webstore_inline_installer.h +++ b/chrome/browser/extensions/webstore_inline_installer.h
@@ -5,6 +5,7 @@ #ifndef CHROME_BROWSER_EXTENSIONS_WEBSTORE_INLINE_INSTALLER_H_ #define CHROME_BROWSER_EXTENSIONS_WEBSTORE_INLINE_INSTALLER_H_ +#include <memory> #include <string> #include "base/macros.h" @@ -49,6 +50,7 @@ ~WebstoreInlineInstaller() override; // Implementations WebstoreStandaloneInstaller Template Method's hooks. + std::string GetJsonPostData() override; bool CheckRequestorAlive() const override; const GURL& GetRequestorURL() const override; bool ShouldShowPostInstallUI() const override;
diff --git a/chrome/browser/extensions/webstore_inline_installer_browsertest.cc b/chrome/browser/extensions/webstore_inline_installer_browsertest.cc index 5ae1fef..35d5edc9 100644 --- a/chrome/browser/extensions/webstore_inline_installer_browsertest.cc +++ b/chrome/browser/extensions/webstore_inline_installer_browsertest.cc
@@ -4,9 +4,11 @@ #include "chrome/browser/extensions/webstore_inline_installer.h" +#include "base/json/json_reader.h" #include "base/macros.h" #include "base/memory/ptr_util.h" #include "base/strings/utf_string_conversions.h" +#include "base/values.h" #include "chrome/browser/content_settings/host_content_settings_map_factory.h" #include "chrome/browser/extensions/extension_install_prompt.h" #include "chrome/browser/extensions/extension_service.h" @@ -25,6 +27,8 @@ #include "extensions/browser/extension_registry.h" #include "extensions/browser/extension_system.h" #include "extensions/common/permissions/permission_set.h" +#include "net/dns/mock_host_resolver.h" +#include "net/test/embedded_test_server/http_request.h" #include "url/gurl.h" using content::WebContents; @@ -40,6 +44,9 @@ const char kTestDataPath[] = "extensions/api_test/webstore_inline_install"; const char kCrxFilename[] = "extension.crx"; +const char kRedirect1Domain[] = "redirect1.com"; +const char kRedirect2Domain[] = "redirect2.com"; + // A struct for letting us store the actual parameters that were passed to // the install callback. struct InstallResult { @@ -363,6 +370,75 @@ RunTest("runTest"); } +class WebstoreInlineInstallerRedirectTest : public WebstoreInlineInstallerTest { + public: + WebstoreInlineInstallerRedirectTest() : cws_request_received_(false) {} + ~WebstoreInlineInstallerRedirectTest() override {} + + void SetUpInProcessBrowserTestFixture() override { + WebstoreInstallerTest::SetUpInProcessBrowserTestFixture(); + host_resolver()->AddRule(kRedirect1Domain, "127.0.0.1"); + host_resolver()->AddRule(kRedirect2Domain, "127.0.0.1"); + } + + void ProcessServerRequest( + const net::test_server::HttpRequest& request) override { + if (request.content.find("redirect_chain") != std::string::npos) { + cws_request_received_ = true; + + std::unique_ptr<base::Value> contents = + base::JSONReader::Read(request.content); + ASSERT_EQ(base::Value::Type::DICTIONARY, contents->GetType()); + cws_request_json_data_ = base::DictionaryValue::From(std::move(contents)); + } + } + + bool cws_request_received_; + std::unique_ptr<base::DictionaryValue> cws_request_json_data_; +}; + +// Test that an install from a page arrived at via redirects includes the +// redirect information in the webstore request. +IN_PROC_BROWSER_TEST_F(WebstoreInlineInstallerRedirectTest, + IncludesRedirectData) { + // Hand craft a url that will cause the test server to issue redirects. + const std::vector<std::string> redirects = {kRedirect1Domain, + kRedirect2Domain}; + net::HostPortPair host_port = embedded_test_server()->host_port_pair(); + std::string redirect_chain; + for (const auto& redirect : redirects) { + std::string redirect_url = base::StringPrintf( + "http://%s:%d/server-redirect?", redirect.c_str(), host_port.port()); + redirect_chain += redirect_url; + } + const GURL install_url = + GURL(redirect_chain + + GenerateTestServerUrl(kAppDomain, "install.html").spec()); + + AutoAcceptInstall(); + ui_test_utils::NavigateToURL(browser(), install_url); + RunTest("runTest"); + + EXPECT_TRUE(cws_request_received_); + ASSERT_NE(nullptr, cws_request_json_data_); + + base::ListValue* redirect_list = nullptr; + cws_request_json_data_->GetList("redirect_chain", &redirect_list); + ASSERT_NE(nullptr, redirect_list); + + // Check that the expected domains are in the redirect list. + const std::vector<std::string> expected_redirect_domains = { + kRedirect1Domain, kRedirect2Domain, kAppDomain}; + ASSERT_EQ(expected_redirect_domains.size(), redirect_list->GetSize()); + int i = 0; + for (const auto& value : *redirect_list) { + std::string value_string; + ASSERT_TRUE(value->GetAsString(&value_string)); + GURL redirect_url(value_string); + EXPECT_EQ(expected_redirect_domains[i++], redirect_url.host()); + } +} + class WebstoreInlineInstallerListenerTest : public WebstoreInlineInstallerTest { public: WebstoreInlineInstallerListenerTest() {}
diff --git a/chrome/browser/extensions/webstore_installer_test.cc b/chrome/browser/extensions/webstore_installer_test.cc index 1cd0398..967a525 100644 --- a/chrome/browser/extensions/webstore_installer_test.cc +++ b/chrome/browser/extensions/webstore_installer_test.cc
@@ -37,6 +37,8 @@ using extensions::WebstoreInlineInstallerFactory; using extensions::WebstoreStandaloneInstaller; +using net::test_server::HttpRequest; + WebstoreInstallerTest::WebstoreInstallerTest( const std::string& webstore_domain, const std::string& test_data_path, @@ -54,6 +56,9 @@ void WebstoreInstallerTest::SetUpCommandLine(base::CommandLine* command_line) { ExtensionBrowserTest::SetUpCommandLine(command_line); + + embedded_test_server()->RegisterRequestMonitor(base::Bind( + &WebstoreInstallerTest::ProcessServerRequest, base::Unretained(this))); // We start the test server now instead of in // SetUpInProcessBrowserTestFixture so that we can get its port number. ASSERT_TRUE(embedded_test_server()->Start()); @@ -137,6 +142,8 @@ ExecuteJavaScriptWithUserGestureForTests(base::UTF8ToUTF16(script)); } +void WebstoreInstallerTest::ProcessServerRequest(const HttpRequest& request) {} + void WebstoreInstallerTest::AutoAcceptInstall() { install_auto_confirm_.reset(); // Destroy any old override first. install_auto_confirm_.reset(new extensions::ScopedTestDialogAutoConfirm(
diff --git a/chrome/browser/extensions/webstore_installer_test.h b/chrome/browser/extensions/webstore_installer_test.h index 4c67ef2b..5f6f98a1 100644 --- a/chrome/browser/extensions/webstore_installer_test.h +++ b/chrome/browser/extensions/webstore_installer_test.h
@@ -21,6 +21,12 @@ class WebContents; } +namespace net { +namespace test_server { +struct HttpRequest; +} +} + class WebstoreInstallerTest : public ExtensionBrowserTest { public: WebstoreInstallerTest(const std::string& webstore_domain, @@ -54,6 +60,10 @@ // Runs a test without waiting for any results from the renderer. void RunTestAsync(const std::string& test_function_name); + // Can be overridden to inspect requests to the embedded test server. + virtual void ProcessServerRequest( + const net::test_server::HttpRequest& request); + // Configures command line switches to simulate a user accepting the install // prompt. void AutoAcceptInstall();
diff --git a/chrome/browser/extensions/webstore_standalone_installer.cc b/chrome/browser/extensions/webstore_standalone_installer.cc index 84f1905..425fb1f 100644 --- a/chrome/browser/extensions/webstore_standalone_installer.cc +++ b/chrome/browser/extensions/webstore_standalone_installer.cc
@@ -69,6 +69,11 @@ profile_->GetRequestContext(), GetRequestorURL(), id_)); + + std::string json_post_data = GetJsonPostData(); + if (!json_post_data.empty()) + webstore_data_fetcher_->SetJsonPostData(json_post_data); + webstore_data_fetcher_->Start(); } @@ -159,6 +164,10 @@ // Default implementation sets no properties. } +std::string WebstoreStandaloneInstaller::GetJsonPostData() { + return std::string(); +} + void WebstoreStandaloneInstaller::OnManifestParsed() { ProceedWithInstallPrompt(); }
diff --git a/chrome/browser/extensions/webstore_standalone_installer.h b/chrome/browser/extensions/webstore_standalone_installer.h index 73110be..400d947 100644 --- a/chrome/browser/extensions/webstore_standalone_installer.h +++ b/chrome/browser/extensions/webstore_standalone_installer.h
@@ -91,6 +91,11 @@ // Allows subclasses to set properties of the install data. virtual void InitInstallData(ActiveInstallData* install_data) const; + // Gives subclasses an opportunity to provide extra post data in the form of + // serialized JSON to the webstore data request before sending. The default + // implementation returns an empty string. + virtual std::string GetJsonPostData(); + // Called at certain check points of the workflow to decide whether it makes // sense to proceed with installation. A requestor can be a website that // initiated an inline installation, or a command line option.
diff --git a/chrome/browser/ntp_tiles/chrome_most_visited_sites_factory.cc b/chrome/browser/ntp_tiles/chrome_most_visited_sites_factory.cc index 4122c379..ffa82726 100644 --- a/chrome/browser/ntp_tiles/chrome_most_visited_sites_factory.cc +++ b/chrome/browser/ntp_tiles/chrome_most_visited_sites_factory.cc
@@ -108,6 +108,11 @@ // static std::unique_ptr<ntp_tiles::MostVisitedSites> ChromeMostVisitedSitesFactory::NewForProfile(Profile* profile) { + // MostVisitedSites doesn't exist in incognito profiles. + if (profile->IsOffTheRecord()) { + return nullptr; + } + return base::MakeUnique<ntp_tiles::MostVisitedSites>( profile->GetPrefs(), TopSitesFactory::GetForProfile(profile), SuggestionsServiceFactory::GetForProfile(profile),
diff --git a/chrome/browser/password_manager/password_manager_browsertest.cc b/chrome/browser/password_manager/password_manager_browsertest.cc index 05f545d..9d46ce9 100644 --- a/chrome/browser/password_manager/password_manager_browsertest.cc +++ b/chrome/browser/password_manager/password_manager_browsertest.cc
@@ -1493,6 +1493,49 @@ base::ASCIIToUTF16("password")); } +// Tests that after HTTP -> HTTPS migration the credential is autofilled. +IN_PROC_BROWSER_TEST_F(PasswordManagerBrowserTestBase, + HttpMigratedCredentialAutofilled) { + net::EmbeddedTestServer https_test_server( + net::EmbeddedTestServer::TYPE_HTTPS); + https_test_server.ServeFilesFromSourceDirectory( + base::FilePath(FILE_PATH_LITERAL("chrome/test/data"))); + ASSERT_TRUE(https_test_server.Start()); + + // Add an http credential to the password store. + GURL https_origin = https_test_server.base_url(); + ASSERT_TRUE(https_origin.SchemeIs(url::kHttpsScheme)); + GURL::Replacements rep; + rep.SetSchemeStr(url::kHttpScheme); + GURL http_origin = https_origin.ReplaceComponents(rep); + autofill::PasswordForm http_form; + http_form.signon_realm = http_origin.spec(); + http_form.origin = http_origin; + // Assume that the previous action was already HTTPS one matching the current + // page. + http_form.action = https_origin; + http_form.username_value = base::ASCIIToUTF16("user"); + http_form.password_value = base::ASCIIToUTF16("12345"); + scoped_refptr<password_manager::TestPasswordStore> password_store = + static_cast<password_manager::TestPasswordStore*>( + PasswordStoreFactory::GetForProfile( + browser()->profile(), ServiceAccessType::IMPLICIT_ACCESS).get()); + password_store->AddLogin(http_form); + + NavigationObserver form_observer(WebContents()); + ui_test_utils::NavigateToURL( + browser(), https_test_server.GetURL("/password/password_form.html")); + form_observer.Wait(); + WaitForPasswordStore(); + + // Let the user interact with the page, so that DOM gets modification events, + // needed for autofilling fields. + content::SimulateMouseClickAt( + WebContents(), 0, blink::WebMouseEvent::Button::Left, gfx::Point(1, 1)); + WaitForElementValue("username_field", "user"); + CheckElementValue("password_field", "12345"); +} + IN_PROC_BROWSER_TEST_F(PasswordManagerBrowserTestBase, PromptWhenPasswordFormWithoutUsernameFieldSubmitted) { scoped_refptr<password_manager::TestPasswordStore> password_store =
diff --git a/chrome/browser/resources/chromeos/login/oobe_screen_user_image.js b/chrome/browser/resources/chromeos/login/oobe_screen_user_image.js index 0628c6a7..c6589fb7c 100644 --- a/chrome/browser/resources/chromeos/login/oobe_screen_user_image.js +++ b/chrome/browser/resources/chromeos/login/oobe_screen_user_image.js
@@ -10,6 +10,7 @@ var CONTEXT_KEY_IS_CAMERA_PRESENT = 'isCameraPresent'; var CONTEXT_KEY_SELECTED_IMAGE_URL = 'selectedImageURL'; var CONTEXT_KEY_PROFILE_PICTURE_DATA_URL = 'profilePictureDataURL'; + var CONTEXT_KEY_HAS_GAIA_ACCOUNT = 'hasGaiaAccount'; var UserImagesGrid = options.UserImagesGrid; var ButtonImages = UserImagesGrid.ButtonImages; @@ -47,26 +48,6 @@ loadTimeData.getString('takePhoto'), loadTimeData.getString('photoFromCamera')); - this.profileImageLoading = true; - - // Profile image data (if present). - this.profileImage_ = imageGrid.addItem( - ButtonImages.PROFILE_PICTURE, // Image URL. - loadTimeData.getString('profilePhoto'), // Title. - undefined, // Click handler. - 0, // Position. - function(el) { - // Custom decorator for Profile image element. - var spinner = el.ownerDocument.createElement('div'); - spinner.className = 'spinner'; - var spinnerBg = el.ownerDocument.createElement('div'); - spinnerBg.className = 'spinner-bg'; - spinnerBg.appendChild(spinner); - el.appendChild(spinnerBg); - el.id = 'profile-image'; - }); - this.profileImage_.type = 'profile'; - $('take-photo').addEventListener( 'click', this.handleTakePhoto_.bind(this)); $('discard-photo').addEventListener( @@ -91,6 +72,31 @@ }); this.context.addObserver(CONTEXT_KEY_SELECTED_IMAGE_URL, this.setSelectedImage_); + this.context.addObserver(CONTEXT_KEY_HAS_GAIA_ACCOUNT, + function(hasGaiaAccount) { + if (!hasGaiaAccount) { + imageGrid.removeItem(self.profileImage_); + } else { + self.profileImageLoading = true; + // Profile image data (if present). + self.profileImage_ = imageGrid.addItem( + ButtonImages.PROFILE_PICTURE, // Image URL. + loadTimeData.getString('profilePhoto'), // Title. + undefined, // Click handler. + 0, // Position. + function(el) { + // Custom decorator for Profile image element. + var spinner = el.ownerDocument.createElement('div'); + spinner.className = 'spinner'; + var spinnerBg = el.ownerDocument.createElement('div'); + spinnerBg.className = 'spinner-bg'; + spinnerBg.appendChild(spinner); + el.appendChild(spinnerBg); + el.id = 'profile-image'; + }); + self.profileImage_.type = 'profile'; + } + }); this.context.addObserver(CONTEXT_KEY_PROFILE_PICTURE_DATA_URL, function(url) { self.profileImageLoading = false;
diff --git a/chrome/browser/resources/vr_shell/OWNERS b/chrome/browser/resources/vr_shell/OWNERS index 4adfe100..4df2af8 100644 --- a/chrome/browser/resources/vr_shell/OWNERS +++ b/chrome/browser/resources/vr_shell/OWNERS
@@ -1,3 +1,4 @@ bshe@chromium.org girard@chromium.org mthiesse@chromium.org +cjgrant@chromium.org
diff --git a/chrome/browser/search/instant_service.cc b/chrome/browser/search/instant_service.cc index 4a1c3b1..e313d56 100644 --- a/chrome/browser/search/instant_service.cc +++ b/chrome/browser/search/instant_service.cc
@@ -103,7 +103,8 @@ if (base::FeatureList::IsEnabled(kNtpTilesFeature)) { most_visited_sites_ = ChromeMostVisitedSitesFactory::NewForProfile(profile_); - most_visited_sites_->SetMostVisitedURLsObserver(this, 8); + if (most_visited_sites_) + most_visited_sites_->SetMostVisitedURLsObserver(this, 8); } else { top_sites_ = TopSitesFactory::GetForProfile(profile_); if (top_sites_) {
diff --git a/chrome/browser/supervised_user/experimental/supervised_user_filtering_switches.cc b/chrome/browser/supervised_user/experimental/supervised_user_filtering_switches.cc index 5014f821..1868fa3 100644 --- a/chrome/browser/supervised_user/experimental/supervised_user_filtering_switches.cc +++ b/chrome/browser/supervised_user/experimental/supervised_user_filtering_switches.cc
@@ -35,24 +35,6 @@ if (!profile->GetPrefs()->GetBoolean(prefs::kSupervisedUserSafeSites)) return SafeSitesState::DISABLED; - std::string arg = base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII( - switches::kSupervisedUserSafeSites); - if (!arg.empty()) { - if (arg == "enabled") - return SafeSitesState::ENABLED; - if (arg == "disabled") - return SafeSitesState::DISABLED; - if (arg == "blacklist-only") - return SafeSitesState::BLACKLIST_ONLY; - if (arg == "online-check-only") - return SafeSitesState::ONLINE_CHECK_ONLY; - - LOG(WARNING) << "Invalid value \"" << arg << "\" specified for flag \"" - << switches::kSupervisedUserSafeSites - << "\", defaulting to \"disabled\""; - return SafeSitesState::DISABLED; - } - // If no cmdline arg is specified, evaluate the field trial. if (trial_group == "Disabled") return SafeSitesState::DISABLED;
diff --git a/chrome/browser/ui/blocked_content/popup_blocker_tab_helper.cc b/chrome/browser/ui/blocked_content/popup_blocker_tab_helper.cc index ab19dac..35f36bf7 100644 --- a/chrome/browser/ui/blocked_content/popup_blocker_tab_helper.cc +++ b/chrome/browser/ui/blocked_content/popup_blocker_tab_helper.cc
@@ -14,8 +14,8 @@ #include "chrome/common/render_messages.h" #include "components/content_settings/core/browser/host_content_settings_map.h" #include "content/public/browser/navigation_controller.h" -#include "content/public/browser/navigation_details.h" #include "content/public/browser/navigation_entry.h" +#include "content/public/browser/navigation_handle.h" #include "content/public/browser/render_view_host.h" #include "content/public/browser/web_contents.h" #include "third_party/WebKit/public/web/WebWindowFeatures.h" @@ -47,13 +47,16 @@ PopupBlockerTabHelper::~PopupBlockerTabHelper() { } -void PopupBlockerTabHelper::DidNavigateMainFrame( - const content::LoadCommittedDetails& details, - const content::FrameNavigateParams& params) { +void PopupBlockerTabHelper::DidFinishNavigation( + content::NavigationHandle* navigation_handle) { // Clear all page actions, blocked content notifications and browser actions - // for this tab, unless this is an in-page navigation. - if (details.is_in_page) + // for this tab, unless this is an in-page navigation. Also only consider + // main frame navigations that successfully committed. + if (!navigation_handle->IsInMainFrame() || + !navigation_handle->HasCommitted() || + navigation_handle->IsSamePage()) { return; + } // Close blocked popups. if (!blocked_popups_.IsEmpty()) {
diff --git a/chrome/browser/ui/blocked_content/popup_blocker_tab_helper.h b/chrome/browser/ui/blocked_content/popup_blocker_tab_helper.h index 0b700b71..0e0fabc 100644 --- a/chrome/browser/ui/blocked_content/popup_blocker_tab_helper.h +++ b/chrome/browser/ui/blocked_content/popup_blocker_tab_helper.h
@@ -53,9 +53,8 @@ PopupIdMap GetBlockedPopupRequests(); // content::WebContentsObserver overrides: - void DidNavigateMainFrame( - const content::LoadCommittedDetails& details, - const content::FrameNavigateParams& params) override; + void DidFinishNavigation( + content::NavigationHandle* navigation_handle) override; private: struct BlockedRequest;
diff --git a/chrome/browser/ui/webui/options/manage_profile_handler.h b/chrome/browser/ui/webui/options/manage_profile_handler.h index 9208771..0e0cc31 100644 --- a/chrome/browser/ui/webui/options/manage_profile_handler.h +++ b/chrome/browser/ui/webui/options/manage_profile_handler.h
@@ -15,7 +15,8 @@ #include "components/sync/driver/sync_service_observer.h" namespace base { -class StringValue; +class Value; +using StringValue = Value; } namespace options {
diff --git a/chrome/browser/ui/webui/vr_shell/OWNERS b/chrome/browser/ui/webui/vr_shell/OWNERS index 4adfe100..4df2af8 100644 --- a/chrome/browser/ui/webui/vr_shell/OWNERS +++ b/chrome/browser/ui/webui/vr_shell/OWNERS
@@ -1,3 +1,4 @@ bshe@chromium.org girard@chromium.org mthiesse@chromium.org +cjgrant@chromium.org
diff --git a/chrome/common/chrome_switches.cc b/chrome/common/chrome_switches.cc index bb414a62..ca32982 100644 --- a/chrome/common/chrome_switches.cc +++ b/chrome/common/chrome_switches.cc
@@ -792,10 +792,6 @@ // Used for testing. const char kSupervisedUserId[] = "managed-user-id"; -// Enables/disables SafeSites filtering for supervised users. Possible values -// are "enabled", "disabled", "blacklist-only", and "online-check-only". -const char kSupervisedUserSafeSites[] = "supervised-user-safesites"; - // Used to authenticate requests to the Sync service for supervised users. // Setting this switch also causes Sync to be set up for a supervised user. const char kSupervisedUserSyncToken[] = "managed-user-sync-token";
diff --git a/chrome/common/chrome_switches.h b/chrome/common/chrome_switches.h index 9bb2cbf..9d3daa3 100644 --- a/chrome/common/chrome_switches.h +++ b/chrome/common/chrome_switches.h
@@ -224,7 +224,6 @@ extern const char kStartMaximized[]; extern const char kStartStackProfiler[]; extern const char kSupervisedUserId[]; -extern const char kSupervisedUserSafeSites[]; extern const char kSupervisedUserSyncToken[]; extern const char kSystemLogUploadFrequency[]; extern const char kTestName[];
diff --git a/chromeos/network/network_state_unittest.cc b/chromeos/network/network_state_unittest.cc index 6cd28ae..5fdb1d6 100644 --- a/chromeos/network/network_state_unittest.cc +++ b/chromeos/network/network_state_unittest.cc
@@ -27,7 +27,11 @@ explicit TestStringValue(const std::string& in_value) : base::Value(Type::STRING), value_(in_value) {} - ~TestStringValue() override {} + ~TestStringValue() override { + // Ugly hack that prevents ~Value() from trying to destroy string_value_. + // TODO(crbug.com/646113): Clean this up when StringValue will be removed. + type_ = Type::NONE; + } // Overridden from Value: bool GetAsString(std::string* out_value) const override {
diff --git a/components/ntp_snippets/features.cc b/components/ntp_snippets/features.cc index b680661..8c40e5a7 100644 --- a/components/ntp_snippets/features.cc +++ b/components/ntp_snippets/features.cc
@@ -36,15 +36,9 @@ const base::Feature kContentSuggestionsSource{ "NTPSnippets", base::FEATURE_ENABLED_BY_DEFAULT}; -const base::Feature kSectionDismissalFeature{ - "NTPSuggestionsSectionDismissal", base::FEATURE_ENABLED_BY_DEFAULT}; - const base::Feature kForeignSessionsSuggestionsFeature{ "NTPForeignSessionsSuggestions", base::FEATURE_DISABLED_BY_DEFAULT}; -const base::Feature kFetchMoreFeature{"NTPSuggestionsFetchMore", - base::FEATURE_ENABLED_BY_DEFAULT}; - const base::Feature kPreferAmpUrlsFeature{"NTPPreferAmpUrls", base::FEATURE_ENABLED_BY_DEFAULT};
diff --git a/components/ntp_snippets/features.h b/components/ntp_snippets/features.h index bf9b944..cc8be99 100644 --- a/components/ntp_snippets/features.h +++ b/components/ntp_snippets/features.h
@@ -32,9 +32,6 @@ // Feature to allow offline badges to appear on snippets. extern const base::Feature kOfflineBadgeFeature; -// Feature to allow dismissing sections. -extern const base::Feature kSectionDismissalFeature; - // Feature to allow specification of content suggestions source. // TODO(peconn): Figure out how to remove this, it is useful to specify the // source, but you shouldn't be able to disable it. @@ -43,9 +40,6 @@ // Feature to allow UI as specified here: https://crbug.com/660837. extern const base::Feature kIncreasedVisibility; -// Feature to enable the Fetch More action -extern const base::Feature kFetchMoreFeature; - // Feature to prefer AMP URLs over regular URLs when available. extern const base::Feature kPreferAmpUrlsFeature;
diff --git a/components/ntp_snippets/remote/json_request.cc b/components/ntp_snippets/remote/json_request.cc index 9c24b7f8..8e522936 100644 --- a/components/ntp_snippets/remote/json_request.cc +++ b/components/ntp_snippets/remote/json_request.cc
@@ -141,7 +141,7 @@ ContentSuggestionsCardLayout::FULL_CARD, // TODO(dgn): merge has_more_action and has_reload_action when we remove // the kFetchMoreFeature flag. See https://crbug.com/667752 - /*has_more_action=*/base::FeatureList::IsEnabled(kFetchMoreFeature), + /*has_more_action=*/true, /*has_reload_action=*/true, /*has_view_all_action=*/false, /*show_if_empty=*/true, @@ -154,8 +154,7 @@ title, ContentSuggestionsCardLayout::FULL_CARD, // TODO(dgn): merge has_more_action and has_reload_action when we remove // the kFetchMoreFeature flag. See https://crbug.com/667752 - /*has_more_action=*/allow_fetching_more_results && - base::FeatureList::IsEnabled(kFetchMoreFeature), + /*has_more_action=*/allow_fetching_more_results, /*has_reload_action=*/allow_fetching_more_results, /*has_view_all_action=*/false, /*show_if_empty=*/false,
diff --git a/components/ntp_snippets/remote/remote_suggestions_fetcher.cc b/components/ntp_snippets/remote/remote_suggestions_fetcher.cc index b4afcee5..56eaa12 100644 --- a/components/ntp_snippets/remote/remote_suggestions_fetcher.cc +++ b/components/ntp_snippets/remote/remote_suggestions_fetcher.cc
@@ -213,7 +213,7 @@ ContentSuggestionsCardLayout::FULL_CARD, // TODO(dgn): merge has_more_action and has_reload_action when we remove // the kFetchMoreFeature flag. See https://crbug.com/667752 - /*has_more_action=*/base::FeatureList::IsEnabled(kFetchMoreFeature), + /*has_more_action=*/true, /*has_reload_action=*/true, /*has_view_all_action=*/false, /*show_if_empty=*/true, @@ -226,8 +226,7 @@ title, ContentSuggestionsCardLayout::FULL_CARD, // TODO(dgn): merge has_more_action and has_reload_action when we remove // the kFetchMoreFeature flag. See https://crbug.com/667752 - /*has_more_action=*/allow_fetching_more_results && - base::FeatureList::IsEnabled(kFetchMoreFeature), + /*has_more_action=*/allow_fetching_more_results, /*has_reload_action=*/allow_fetching_more_results, /*has_view_all_action=*/false, /*show_if_empty=*/false,
diff --git a/components/password_manager/core/browser/form_fetcher_impl_unittest.cc b/components/password_manager/core/browser/form_fetcher_impl_unittest.cc index 9e3e072..4dd2d59 100644 --- a/components/password_manager/core/browser/form_fetcher_impl_unittest.cc +++ b/components/password_manager/core/browser/form_fetcher_impl_unittest.cc
@@ -490,10 +490,6 @@ form_fetcher_->OnGetPasswordStoreResults(MakeResults(empty_forms)); ASSERT_TRUE(migrator_ptr); - // Setting action equal to origin is necessary in order to mirror the behavior - // in HttpPasswordMigrator. - https_form.action = https_form.origin; - // Now perform the actual migration. EXPECT_CALL(*mock_store_, AddLogin(https_form)); EXPECT_CALL(*static_cast<MockFormFetcherImpl*>(form_fetcher_.get()),
diff --git a/components/password_manager/core/browser/http_password_migrator.cc b/components/password_manager/core/browser/http_password_migrator.cc index 8f5552a..da6eb67c 100644 --- a/components/password_manager/core/browser/http_password_migrator.cc +++ b/components/password_manager/core/browser/http_password_migrator.cc
@@ -47,7 +47,10 @@ rep.SetSchemeStr(url::kHttpsScheme); form->origin = form->origin.ReplaceComponents(rep); form->signon_realm = form->origin.spec(); - form->action = form->origin; + // If |action| is not HTTPS then it's most likely obsolete. Otherwise, it + // may still be valid. + if (!form->action.SchemeIs(url::kHttpsScheme)) + form->action = form->origin; form->form_data = autofill::FormData(); form->generation_upload_status = autofill::PasswordForm::NO_SIGNAL_SENT; form->skip_zero_click = false;
diff --git a/components/password_manager/core/browser/http_password_migrator_unittest.cc b/components/password_manager/core/browser/http_password_migrator_unittest.cc index ab7bb55..e536f10 100644 --- a/components/password_manager/core/browser/http_password_migrator_unittest.cc +++ b/components/password_manager/core/browser/http_password_migrator_unittest.cc
@@ -28,7 +28,7 @@ PasswordForm form; form.origin = GURL(kTestHttpURL); form.signon_realm = form.origin.spec(); - form.action = GURL(kTestHttpURL); + form.action = GURL("https://example.org/action.html"); form.username_value = base::ASCIIToUTF16("user"); form.password_value = base::ASCIIToUTF16("password"); return form; @@ -116,7 +116,6 @@ PasswordForm expected_form = form; expected_form.origin = GURL(kTestHttpsURL); expected_form.signon_realm = expected_form.origin.spec(); - expected_form.action = expected_form.origin; EXPECT_CALL(store(), AddLogin(expected_form)); EXPECT_CALL(consumer(), ProcessForms(ElementsAre(Pointee(expected_form))));
diff --git a/components/subresource_filter/content/browser/BUILD.gn b/components/subresource_filter/content/browser/BUILD.gn index 5fe23a1..e49864e 100644 --- a/components/subresource_filter/content/browser/BUILD.gn +++ b/components/subresource_filter/content/browser/BUILD.gn
@@ -10,6 +10,8 @@ "content_subresource_filter_driver.h", "content_subresource_filter_driver_factory.cc", "content_subresource_filter_driver_factory.h", + "verified_ruleset_dealer.cc", + "verified_ruleset_dealer.h", ] deps = [ "//base", @@ -29,6 +31,7 @@ sources = [ "content_ruleset_service_delegate_unittest.cc", "content_subresource_filter_driver_factory_unittest.cc", + "verified_ruleset_dealer_unittest.cc", ] deps = [ ":browser", @@ -37,6 +40,8 @@ "//components/subresource_filter/content/common", "//components/subresource_filter/core/browser", "//components/subresource_filter/core/browser:test_support", + "//components/subresource_filter/core/common", + "//components/subresource_filter/core/common:test_support", "//content/test:test_support", "//ipc", "//ipc:test_support",
diff --git a/components/subresource_filter/content/browser/verified_ruleset_dealer.cc b/components/subresource_filter/content/browser/verified_ruleset_dealer.cc new file mode 100644 index 0000000..cb6b79d --- /dev/null +++ b/components/subresource_filter/content/browser/verified_ruleset_dealer.cc
@@ -0,0 +1,81 @@ +// 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 "components/subresource_filter/content/browser/verified_ruleset_dealer.h" + +#include "base/bind.h" +#include "base/bind_helpers.h" +#include "base/files/file.h" +#include "base/location.h" +#include "base/logging.h" +#include "components/subresource_filter/core/common/indexed_ruleset.h" +#include "components/subresource_filter/core/common/memory_mapped_ruleset.h" + +namespace subresource_filter { + +VerifiedRulesetDealer::VerifiedRulesetDealer() = default; +VerifiedRulesetDealer::~VerifiedRulesetDealer() = default; + +void VerifiedRulesetDealer::SetRulesetFile(base::File ruleset_file) { + RulesetDealer::SetRulesetFile(std::move(ruleset_file)); + status_ = RulesetVerificationStatus::NOT_VERIFIED; +} + +scoped_refptr<const MemoryMappedRuleset> VerifiedRulesetDealer::GetRuleset() { + DCHECK(CalledOnValidThread()); + + // TODO(pkalinnikov): Record verification status to a histogram. + switch (status_) { + case RulesetVerificationStatus::NOT_VERIFIED: { + auto ruleset = RulesetDealer::GetRuleset(); + if (ruleset && + IndexedRulesetMatcher::Verify(ruleset->data(), ruleset->length())) { + status_ = RulesetVerificationStatus::INTACT; + return ruleset; + } + status_ = RulesetVerificationStatus::CORRUPT; + return nullptr; + } + case RulesetVerificationStatus::INTACT: { + auto ruleset = RulesetDealer::GetRuleset(); + DCHECK(ruleset); + return ruleset; + } + case RulesetVerificationStatus::CORRUPT: + return nullptr; + default: + NOTREACHED(); + return nullptr; + } +} + +VerifiedRulesetDealer::Handle::Handle( + scoped_refptr<base::SequencedTaskRunner> task_runner) + : task_runner_(task_runner.get()), + dealer_(new VerifiedRulesetDealer, + base::OnTaskRunnerDeleter(std::move(task_runner))) {} + +VerifiedRulesetDealer::Handle::~Handle() = default; + +void VerifiedRulesetDealer::Handle::GetDealerAsync( + base::Callback<void(VerifiedRulesetDealer*)> callback) { + DCHECK(thread_checker_.CalledOnValidThread()); + + // NOTE: Properties of the sequenced |task_runner| guarantee that the + // |callback| will always be provided with a valid pointer, because the + // corresponding task will be posted *before* a task to delete the pointer + // upon destruction of |this| Handler. + task_runner_->PostTask(FROM_HERE, + base::Bind(std::move(callback), dealer_.get())); +} + +void VerifiedRulesetDealer::Handle::SetRulesetFile(base::File file) { + DCHECK(thread_checker_.CalledOnValidThread()); + task_runner_->PostTask( + FROM_HERE, + base::Bind(&VerifiedRulesetDealer::SetRulesetFile, + base::Unretained(dealer_.get()), base::Passed(&file))); +} + +} // namespace subresource_filter
diff --git a/components/subresource_filter/content/browser/verified_ruleset_dealer.h b/components/subresource_filter/content/browser/verified_ruleset_dealer.h new file mode 100644 index 0000000..187b7ed --- /dev/null +++ b/components/subresource_filter/content/browser/verified_ruleset_dealer.h
@@ -0,0 +1,92 @@ +// 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_SUBRESOURCE_FILTER_CONTENT_BROWSER_VERIFIED_RULESET_DEALER_H_ +#define COMPONENTS_SUBRESOURCE_FILTER_CONTENT_BROWSER_VERIFIED_RULESET_DEALER_H_ + +#include <memory> + +#include "base/callback_forward.h" +#include "base/macros.h" +#include "base/memory/ref_counted.h" +#include "base/sequenced_task_runner.h" +#include "base/threading/thread_checker.h" +#include "components/subresource_filter/content/common/ruleset_dealer.h" + +namespace base { +class File; +} // namespace base + +namespace subresource_filter { + +class MemoryMappedRuleset; + +// The integrity verification status of a given ruleset version. +// +// A ruleset file starts from the NOT_VERIFIED state, after which it can be +// classified as INTACT or CORRUPT upon integrity verification. +enum class RulesetVerificationStatus { + NOT_VERIFIED, + INTACT, + CORRUPT, +}; + +// This class is the same as RulesetDealer, but additionally does a one-time +// integrity checking on the ruleset before handing it out from GetRuleset(). +// +// The |status| of verification is persisted throughout the entire lifetime of +// |this| object, and is reset to NOT_VERIFIED only when a new ruleset is +// supplied to SetRulesetFile() method. +class VerifiedRulesetDealer : public RulesetDealer { + public: + class Handle; + + VerifiedRulesetDealer(); + ~VerifiedRulesetDealer() override; + + // RulesetDealer: + void SetRulesetFile(base::File ruleset_file) override; + scoped_refptr<const MemoryMappedRuleset> GetRuleset() override; + + // For tests only. + RulesetVerificationStatus status() const { return status_; } + + private: + RulesetVerificationStatus status_ = RulesetVerificationStatus::NOT_VERIFIED; + + DISALLOW_COPY_AND_ASSIGN(VerifiedRulesetDealer); +}; + +// The UI-thread handle that owns a VerifiedRulesetDealer living on a dedicated +// sequenced |task_runner|. Provides asynchronous access to the instance, and +// destroys it asynchronously. +class VerifiedRulesetDealer::Handle { + public: + // Creates a VerifiedRulesetDealer that is owned by this handle, accessed + // through this handle, but lives on |task_runner|. + explicit Handle(scoped_refptr<base::SequencedTaskRunner> task_runner); + ~Handle(); + + // Invokes |callback| on |task_runner|, providing a pointer to the underlying + // VerifiedRulesetDealer as an argument. The pointer is guaranteed to be valid + // at least until the callback returns. + void GetDealerAsync(base::Callback<void(VerifiedRulesetDealer*)> callback); + + // Sets the ruleset |file| that the VerifiedRulesetDealer should distribute + // from now on. + void SetRulesetFile(base::File file); + + private: + // Note: Raw pointer, |dealer_| already holds a reference to |task_runner_|. + base::SequencedTaskRunner* task_runner_; + std::unique_ptr<VerifiedRulesetDealer, base::OnTaskRunnerDeleter> dealer_; + + base::ThreadChecker thread_checker_; + + DISALLOW_COPY_AND_ASSIGN(Handle); +}; + +} // namespace subresource_filter + +#endif // COMPONENTS_SUBRESOURCE_FILTER_CONTENT_BROWSER_VERIFIED_RULESET_DEALER_H_
diff --git a/components/subresource_filter/content/browser/verified_ruleset_dealer_unittest.cc b/components/subresource_filter/content/browser/verified_ruleset_dealer_unittest.cc new file mode 100644 index 0000000..4182e1c8 --- /dev/null +++ b/components/subresource_filter/content/browser/verified_ruleset_dealer_unittest.cc
@@ -0,0 +1,339 @@ +// 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 "components/subresource_filter/content/browser/verified_ruleset_dealer.h" + +#include <memory> +#include <vector> + +#include "base/bind.h" +#include "base/files/file.h" +#include "base/test/test_simple_task_runner.h" +#include "components/subresource_filter/core/common/memory_mapped_ruleset.h" +#include "components/subresource_filter/core/common/test_ruleset_creator.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace subresource_filter { + +namespace { + +// TODO(pkalinnikov): Consider putting this to a test_support for this test file +// and SubresourceFilterRulesetDealerTest. +class TestRulesets { + public: + TestRulesets() = default; + + void CreateRulesets(bool many_rules = false) { + if (many_rules) { + ASSERT_NO_FATAL_FAILURE( + test_ruleset_creator_.CreateRulesetToDisallowURLsWithManySuffixes( + kTestRulesetSuffix1, kNumberOfRulesInBigRuleset, + &test_ruleset_pair_1_)); + ASSERT_NO_FATAL_FAILURE( + test_ruleset_creator_.CreateRulesetToDisallowURLsWithManySuffixes( + kTestRulesetSuffix2, kNumberOfRulesInBigRuleset, + &test_ruleset_pair_2_)); + } else { + ASSERT_NO_FATAL_FAILURE( + test_ruleset_creator_.CreateRulesetToDisallowURLsWithPathSuffix( + kTestRulesetSuffix1, &test_ruleset_pair_1_)); + ASSERT_NO_FATAL_FAILURE( + test_ruleset_creator_.CreateRulesetToDisallowURLsWithPathSuffix( + kTestRulesetSuffix2, &test_ruleset_pair_2_)); + } + } + + const testing::TestRuleset& indexed_1() const { + return test_ruleset_pair_1_.indexed; + } + + const testing::TestRuleset& indexed_2() const { + return test_ruleset_pair_2_.indexed; + } + + private: + static constexpr const char kTestRulesetSuffix1[] = "foo"; + static constexpr const char kTestRulesetSuffix2[] = "bar"; + static constexpr int kNumberOfRulesInBigRuleset = 500; + + testing::TestRulesetCreator test_ruleset_creator_; + testing::TestRulesetPair test_ruleset_pair_1_; + testing::TestRulesetPair test_ruleset_pair_2_; + + DISALLOW_COPY_AND_ASSIGN(TestRulesets); +}; + +constexpr const char TestRulesets::kTestRulesetSuffix1[]; +constexpr const char TestRulesets::kTestRulesetSuffix2[]; +constexpr int TestRulesets::kNumberOfRulesInBigRuleset; + +std::vector<uint8_t> ReadRulesetContents(const MemoryMappedRuleset* ruleset) { + return std::vector<uint8_t>(ruleset->data(), + ruleset->data() + ruleset->length()); +} + +} // namespace + +// Tests for VerifiedRulesetDealer. -------------------------------------------- +// +// Note that VerifiedRulesetDealer uses RulesetDealer very directly to provide +// MemoryMappedRulesets. Many aspects of its work, e.g., lifetime of a +// MemoryMappedRuleset, its lazy creation, etc., are covered with tests to +// RulesetDealer, therefore these aspects are not tested here. + +class SubresourceFilterVerifiedRulesetDealerTest : public ::testing::Test { + public: + SubresourceFilterVerifiedRulesetDealerTest() = default; + + protected: + void SetUp() override { + rulesets_.CreateRulesets(true /* many_rules */); + ruleset_dealer_.reset(new VerifiedRulesetDealer); + } + + const TestRulesets& rulesets() const { return rulesets_; } + VerifiedRulesetDealer* ruleset_dealer() { return ruleset_dealer_.get(); } + + bool has_cached_ruleset() const { + return ruleset_dealer_->has_cached_ruleset(); + } + + private: + TestRulesets rulesets_; + std::unique_ptr<VerifiedRulesetDealer> ruleset_dealer_; + + DISALLOW_COPY_AND_ASSIGN(SubresourceFilterVerifiedRulesetDealerTest); +}; + +TEST_F(SubresourceFilterVerifiedRulesetDealerTest, + RulesetIsMemoryMappedAndVerifiedLazily) { + ruleset_dealer()->SetRulesetFile( + testing::TestRuleset::Open(rulesets().indexed_1())); + + EXPECT_TRUE(ruleset_dealer()->IsRulesetFileAvailable()); + EXPECT_FALSE(has_cached_ruleset()); + EXPECT_EQ(RulesetVerificationStatus::NOT_VERIFIED, + ruleset_dealer()->status()); + + scoped_refptr<const MemoryMappedRuleset> ref_to_ruleset = + ruleset_dealer()->GetRuleset(); + + EXPECT_TRUE(ruleset_dealer()->IsRulesetFileAvailable()); + EXPECT_TRUE(ref_to_ruleset); + EXPECT_TRUE(has_cached_ruleset()); + EXPECT_EQ(RulesetVerificationStatus::INTACT, ruleset_dealer()->status()); +} + +TEST_F(SubresourceFilterVerifiedRulesetDealerTest, + CorruptedRulesetIsNeitherProvidedNorCached) { + testing::TestRuleset::CorruptByTruncating(rulesets().indexed_1(), 123); + + ruleset_dealer()->SetRulesetFile( + testing::TestRuleset::Open(rulesets().indexed_1())); + + EXPECT_TRUE(ruleset_dealer()->IsRulesetFileAvailable()); + EXPECT_FALSE(has_cached_ruleset()); + EXPECT_EQ(RulesetVerificationStatus::NOT_VERIFIED, + ruleset_dealer()->status()); + + scoped_refptr<const MemoryMappedRuleset> ref_to_ruleset = + ruleset_dealer()->GetRuleset(); + + EXPECT_TRUE(ruleset_dealer()->IsRulesetFileAvailable()); + EXPECT_FALSE(ref_to_ruleset); + EXPECT_FALSE(has_cached_ruleset()); + EXPECT_EQ(RulesetVerificationStatus::CORRUPT, ruleset_dealer()->status()); +} + +TEST_F(SubresourceFilterVerifiedRulesetDealerTest, + TruncatingFileMakesRulesetInvalid) { + testing::TestRuleset::CorruptByTruncating(rulesets().indexed_1(), 4096); + ruleset_dealer()->SetRulesetFile( + testing::TestRuleset::Open(rulesets().indexed_1())); + scoped_refptr<const MemoryMappedRuleset> ref_to_ruleset = + ruleset_dealer()->GetRuleset(); + + EXPECT_TRUE(ruleset_dealer()->IsRulesetFileAvailable()); + EXPECT_FALSE(ref_to_ruleset); + EXPECT_FALSE(has_cached_ruleset()); + EXPECT_EQ(RulesetVerificationStatus::CORRUPT, ruleset_dealer()->status()); +} + +TEST_F(SubresourceFilterVerifiedRulesetDealerTest, + FillingRangeMakesRulesetInvalid) { + testing::TestRuleset::CorruptByFilling(rulesets().indexed_1(), + 2501 /* from */, 4000 /* to */, + 255 /* fill_with */); + ruleset_dealer()->SetRulesetFile( + testing::TestRuleset::Open(rulesets().indexed_1())); + scoped_refptr<const MemoryMappedRuleset> ref_to_ruleset = + ruleset_dealer()->GetRuleset(); + + EXPECT_TRUE(ruleset_dealer()->IsRulesetFileAvailable()); + EXPECT_FALSE(ref_to_ruleset); + EXPECT_FALSE(has_cached_ruleset()); + EXPECT_EQ(RulesetVerificationStatus::CORRUPT, ruleset_dealer()->status()); +} + +TEST_F(SubresourceFilterVerifiedRulesetDealerTest, + RulesetIsVerifiedAfterUpdate) { + testing::TestRuleset::CorruptByTruncating(rulesets().indexed_1(), 123); + + ruleset_dealer()->SetRulesetFile( + testing::TestRuleset::Open(rulesets().indexed_1())); + + EXPECT_TRUE(ruleset_dealer()->IsRulesetFileAvailable()); + EXPECT_FALSE(has_cached_ruleset()); + EXPECT_EQ(RulesetVerificationStatus::NOT_VERIFIED, + ruleset_dealer()->status()); + + scoped_refptr<const MemoryMappedRuleset> ref_to_ruleset = + ruleset_dealer()->GetRuleset(); + + EXPECT_TRUE(ruleset_dealer()->IsRulesetFileAvailable()); + EXPECT_FALSE(ref_to_ruleset); + EXPECT_FALSE(has_cached_ruleset()); + EXPECT_EQ(RulesetVerificationStatus::CORRUPT, ruleset_dealer()->status()); + + ruleset_dealer()->SetRulesetFile( + testing::TestRuleset::Open(rulesets().indexed_2())); + EXPECT_EQ(RulesetVerificationStatus::NOT_VERIFIED, + ruleset_dealer()->status()); + ref_to_ruleset = ruleset_dealer()->GetRuleset(); + + EXPECT_TRUE(ruleset_dealer()->IsRulesetFileAvailable()); + EXPECT_TRUE(ref_to_ruleset); + EXPECT_TRUE(has_cached_ruleset()); + EXPECT_EQ(RulesetVerificationStatus::INTACT, ruleset_dealer()->status()); +} + +// Tests for VerifiedRulesetDealer::Handle. ------------------------------------ + +namespace { + +class TestVerifiedRulesetDealerClient { + public: + TestVerifiedRulesetDealerClient() = default; + + base::Callback<void(VerifiedRulesetDealer*)> GetCallback() { + return base::Bind(&TestVerifiedRulesetDealerClient::Callback, + base::Unretained(this)); + } + + void ExpectRulesetState(bool expected_availability, + RulesetVerificationStatus expected_status = + RulesetVerificationStatus::NOT_VERIFIED) const { + ASSERT_EQ(1, invocation_counter_); + EXPECT_EQ(expected_availability, is_ruleset_file_available_); + EXPECT_FALSE(has_cached_ruleset_); + EXPECT_EQ(expected_status, status_); + } + + void ExpectRulesetContents( + const std::vector<uint8_t> expected_contents) const { + ExpectRulesetState(true, RulesetVerificationStatus::INTACT); + EXPECT_TRUE(ruleset_is_created_); + EXPECT_EQ(expected_contents, contents_); + } + + private: + void Callback(VerifiedRulesetDealer* dealer) { + ++invocation_counter_; + ASSERT_TRUE(dealer); + + is_ruleset_file_available_ = dealer->IsRulesetFileAvailable(); + has_cached_ruleset_ = dealer->has_cached_ruleset(); + status_ = dealer->status(); + + auto ruleset = dealer->GetRuleset(); + ruleset_is_created_ = !!ruleset; + if (ruleset_is_created_) + contents_ = ReadRulesetContents(ruleset.get()); + } + + bool is_ruleset_file_available_ = false; + bool has_cached_ruleset_ = false; + RulesetVerificationStatus status_ = RulesetVerificationStatus::NOT_VERIFIED; + + bool ruleset_is_created_ = false; + std::vector<uint8_t> contents_; + + int invocation_counter_ = 0; + + DISALLOW_COPY_AND_ASSIGN(TestVerifiedRulesetDealerClient); +}; + +} // namespace + +class SubresourceFilterVerifiedRulesetDealerHandleTest + : public ::testing::Test { + public: + SubresourceFilterVerifiedRulesetDealerHandleTest() = default; + + protected: + void SetUp() override { + rulesets_.CreateRulesets(false /* many_rules */); + task_runner_ = new base::TestSimpleTaskRunner; + } + + const TestRulesets& rulesets() const { return rulesets_; } + base::TestSimpleTaskRunner* task_runner() { return task_runner_.get(); } + + private: + TestRulesets rulesets_; + scoped_refptr<base::TestSimpleTaskRunner> task_runner_; + + DISALLOW_COPY_AND_ASSIGN(SubresourceFilterVerifiedRulesetDealerHandleTest); +}; + +TEST_F(SubresourceFilterVerifiedRulesetDealerHandleTest, + RulesetIsMappedLazily) { + TestVerifiedRulesetDealerClient before_set_ruleset; + TestVerifiedRulesetDealerClient after_set_ruleset; + TestVerifiedRulesetDealerClient after_warm_up; + + std::unique_ptr<VerifiedRulesetDealer::Handle> dealer_handle( + new VerifiedRulesetDealer::Handle(task_runner())); + dealer_handle->GetDealerAsync(before_set_ruleset.GetCallback()); + dealer_handle->SetRulesetFile( + testing::TestRuleset::Open(rulesets().indexed_1())); + dealer_handle->GetDealerAsync(after_set_ruleset.GetCallback()); + dealer_handle->GetDealerAsync(after_warm_up.GetCallback()); + dealer_handle.reset(nullptr); + task_runner()->RunUntilIdle(); + + before_set_ruleset.ExpectRulesetState(false); + after_set_ruleset.ExpectRulesetState(true); + after_warm_up.ExpectRulesetContents(rulesets().indexed_1().contents); +} + +TEST_F(SubresourceFilterVerifiedRulesetDealerHandleTest, RulesetFileIsUpdated) { + TestVerifiedRulesetDealerClient after_set_ruleset_1; + TestVerifiedRulesetDealerClient read_ruleset_1; + TestVerifiedRulesetDealerClient after_set_ruleset_2; + TestVerifiedRulesetDealerClient read_ruleset_2; + + std::unique_ptr<VerifiedRulesetDealer::Handle> dealer_handle( + new VerifiedRulesetDealer::Handle(task_runner())); + + dealer_handle->SetRulesetFile( + testing::TestRuleset::Open(rulesets().indexed_1())); + dealer_handle->GetDealerAsync(after_set_ruleset_1.GetCallback()); + dealer_handle->GetDealerAsync(read_ruleset_1.GetCallback()); + + dealer_handle->SetRulesetFile( + testing::TestRuleset::Open(rulesets().indexed_2())); + dealer_handle->GetDealerAsync(after_set_ruleset_2.GetCallback()); + dealer_handle->GetDealerAsync(read_ruleset_2.GetCallback()); + + dealer_handle.reset(nullptr); + task_runner()->RunUntilIdle(); + + after_set_ruleset_1.ExpectRulesetState(true); + read_ruleset_1.ExpectRulesetContents(rulesets().indexed_1().contents); + after_set_ruleset_2.ExpectRulesetState(true); + read_ruleset_2.ExpectRulesetContents(rulesets().indexed_2().contents); +} + +} // namespace subresource_filter
diff --git a/components/subresource_filter/content/common/ruleset_dealer.cc b/components/subresource_filter/content/common/ruleset_dealer.cc index e5e490b..9cdbc3f 100644 --- a/components/subresource_filter/content/common/ruleset_dealer.cc +++ b/components/subresource_filter/content/common/ruleset_dealer.cc
@@ -9,20 +9,26 @@ namespace subresource_filter { -RulesetDealer::RulesetDealer() = default; +RulesetDealer::RulesetDealer() { + DetachFromThread(); +} + RulesetDealer::~RulesetDealer() = default; void RulesetDealer::SetRulesetFile(base::File ruleset_file) { + DCHECK(CalledOnValidThread()); DCHECK(ruleset_file.IsValid()); ruleset_file_ = std::move(ruleset_file); weak_cached_ruleset_.reset(); } -bool RulesetDealer::IsRulesetAvailable() const { +bool RulesetDealer::IsRulesetFileAvailable() const { + DCHECK(CalledOnValidThread()); return ruleset_file_.IsValid(); } scoped_refptr<const MemoryMappedRuleset> RulesetDealer::GetRuleset() { + DCHECK(CalledOnValidThread()); if (!ruleset_file_.IsValid()) return nullptr;
diff --git a/components/subresource_filter/content/common/ruleset_dealer.h b/components/subresource_filter/content/common/ruleset_dealer.h index a467fcd..5934154 100644 --- a/components/subresource_filter/content/common/ruleset_dealer.h +++ b/components/subresource_filter/content/common/ruleset_dealer.h
@@ -9,6 +9,7 @@ #include "base/macros.h" #include "base/memory/ref_counted.h" #include "base/memory/weak_ptr.h" +#include "base/threading/non_thread_safe.h" namespace subresource_filter { @@ -29,29 +30,29 @@ // they will use the same, cached, MemoryMappedRuleset instance, and will // not call mmap() multiple times. // -class RulesetDealer { +class RulesetDealer : protected base::NonThreadSafe { public: RulesetDealer(); virtual ~RulesetDealer(); // Sets the |ruleset_file| to memory map and distribute from now on. - void SetRulesetFile(base::File ruleset_file); + virtual void SetRulesetFile(base::File ruleset_file); - // Returns whether a subsequent call to GetRuleset() would return a non-null - // ruleset, but without memory mapping the ruleset. - bool IsRulesetAvailable() const; + // Returns whether |this| dealer has a ruleset file to read from. Does not + // interact with the file system. + bool IsRulesetFileAvailable() const; // Returns the set |ruleset_file|. Normally, the same instance is used by all // call sites in a given process. That intance is mapped lazily and umapped // eagerly as soon as the last reference to it is dropped. - scoped_refptr<const MemoryMappedRuleset> GetRuleset(); + virtual scoped_refptr<const MemoryMappedRuleset> GetRuleset(); + + // For testing only. + bool has_cached_ruleset() const { return !!weak_cached_ruleset_.get(); } private: friend class SubresourceFilterRulesetDealerTest; - // Exposed for testing only. - bool has_cached_ruleset() const { return !!weak_cached_ruleset_.get(); } - base::File ruleset_file_; base::WeakPtr<MemoryMappedRuleset> weak_cached_ruleset_;
diff --git a/components/subresource_filter/content/common/ruleset_dealer_unittest.cc b/components/subresource_filter/content/common/ruleset_dealer_unittest.cc index 2fd8ca0..fcf7acd 100644 --- a/components/subresource_filter/content/common/ruleset_dealer_unittest.cc +++ b/components/subresource_filter/content/common/ruleset_dealer_unittest.cc
@@ -15,8 +15,8 @@ namespace { -const char kTestRulesetData1[] = "foo"; -const char kTestRulesetData2[] = "bar"; +const char kTestRulesetSuffix1[] = "foo"; +const char kTestRulesetSuffix2[] = "bar"; std::vector<uint8_t> ReadRulesetContents(const MemoryMappedRuleset* ruleset) { return std::vector<uint8_t>(ruleset->data(), @@ -34,10 +34,10 @@ ResetRulesetDealer(); ASSERT_NO_FATAL_FAILURE( test_ruleset_creator_.CreateRulesetToDisallowURLsWithPathSuffix( - kTestRulesetData1, &test_ruleset_pair_1_)); + kTestRulesetSuffix1, &test_ruleset_pair_1_)); ASSERT_NO_FATAL_FAILURE( test_ruleset_creator_.CreateRulesetToDisallowURLsWithPathSuffix( - kTestRulesetData2, &test_ruleset_pair_2_)); + kTestRulesetSuffix2, &test_ruleset_pair_2_)); } testing::TestRuleset& test_indexed_ruleset_1() { @@ -66,29 +66,29 @@ }; TEST_F(SubresourceFilterRulesetDealerTest, NoRuleset) { - EXPECT_FALSE(ruleset_dealer()->IsRulesetAvailable()); + EXPECT_FALSE(ruleset_dealer()->IsRulesetFileAvailable()); EXPECT_FALSE(!!ruleset_dealer()->GetRuleset()); } TEST_F(SubresourceFilterRulesetDealerTest, MostRecentlySetRulesetIsReturned) { ruleset_dealer()->SetRulesetFile( testing::TestRuleset::Open(test_indexed_ruleset_1())); - EXPECT_TRUE(ruleset_dealer()->IsRulesetAvailable()); + EXPECT_TRUE(ruleset_dealer()->IsRulesetFileAvailable()); scoped_refptr<const MemoryMappedRuleset> ref_to_ruleset_1 = ruleset_dealer()->GetRuleset(); - EXPECT_TRUE(ruleset_dealer()->IsRulesetAvailable()); + EXPECT_TRUE(ruleset_dealer()->IsRulesetFileAvailable()); ASSERT_TRUE(!!ref_to_ruleset_1); EXPECT_EQ(test_indexed_ruleset_1().contents, ReadRulesetContents(ref_to_ruleset_1.get())); ruleset_dealer()->SetRulesetFile( testing::TestRuleset::Open(test_indexed_ruleset_2())); - EXPECT_TRUE(ruleset_dealer()->IsRulesetAvailable()); + EXPECT_TRUE(ruleset_dealer()->IsRulesetFileAvailable()); scoped_refptr<const MemoryMappedRuleset> ref_to_ruleset_2 = ruleset_dealer()->GetRuleset(); - EXPECT_TRUE(ruleset_dealer()->IsRulesetAvailable()); + EXPECT_TRUE(ruleset_dealer()->IsRulesetFileAvailable()); ASSERT_TRUE(!!ref_to_ruleset_2); EXPECT_EQ(test_indexed_ruleset_1().contents, ReadRulesetContents(ref_to_ruleset_1.get())); @@ -106,7 +106,7 @@ scoped_refptr<const MemoryMappedRuleset> another_ref_to_ruleset = ruleset_dealer()->GetRuleset(); - EXPECT_TRUE(ruleset_dealer()->IsRulesetAvailable()); + EXPECT_TRUE(ruleset_dealer()->IsRulesetFileAvailable()); EXPECT_TRUE(!!ref_to_ruleset); EXPECT_TRUE(!!another_ref_to_ruleset); EXPECT_EQ(ref_to_ruleset.get(), another_ref_to_ruleset.get()); @@ -116,13 +116,13 @@ ruleset_dealer()->SetRulesetFile( testing::TestRuleset::Open(test_indexed_ruleset_1())); - EXPECT_TRUE(ruleset_dealer()->IsRulesetAvailable()); + EXPECT_TRUE(ruleset_dealer()->IsRulesetFileAvailable()); EXPECT_FALSE(has_cached_ruleset()); scoped_refptr<const MemoryMappedRuleset> ref_to_ruleset = ruleset_dealer()->GetRuleset(); - EXPECT_TRUE(ruleset_dealer()->IsRulesetAvailable()); + EXPECT_TRUE(ruleset_dealer()->IsRulesetFileAvailable()); EXPECT_TRUE(!!ref_to_ruleset); EXPECT_TRUE(has_cached_ruleset()); } @@ -143,14 +143,14 @@ another_ref_to_ruleset = nullptr; - EXPECT_TRUE(ruleset_dealer()->IsRulesetAvailable()); + EXPECT_TRUE(ruleset_dealer()->IsRulesetFileAvailable()); EXPECT_TRUE(has_cached_ruleset()); EXPECT_EQ(test_indexed_ruleset_1().contents, ReadRulesetContents(ref_to_ruleset.get())); ref_to_ruleset = nullptr; - EXPECT_TRUE(ruleset_dealer()->IsRulesetAvailable()); + EXPECT_TRUE(ruleset_dealer()->IsRulesetFileAvailable()); EXPECT_FALSE(has_cached_ruleset()); } @@ -170,7 +170,7 @@ ref_to_ruleset = ruleset_dealer()->GetRuleset(); - EXPECT_TRUE(ruleset_dealer()->IsRulesetAvailable()); + EXPECT_TRUE(ruleset_dealer()->IsRulesetFileAvailable()); EXPECT_TRUE(has_cached_ruleset()); ASSERT_TRUE(!!ref_to_ruleset); EXPECT_EQ(test_indexed_ruleset_1().contents, @@ -178,7 +178,7 @@ ref_to_ruleset = nullptr; - EXPECT_TRUE(ruleset_dealer()->IsRulesetAvailable()); + EXPECT_TRUE(ruleset_dealer()->IsRulesetFileAvailable()); EXPECT_FALSE(has_cached_ruleset()); } @@ -189,21 +189,21 @@ scoped_refptr<const MemoryMappedRuleset> ref_to_ruleset = ruleset_dealer()->GetRuleset(); - ASSERT_TRUE(ruleset_dealer()->IsRulesetAvailable()); + ASSERT_TRUE(ruleset_dealer()->IsRulesetFileAvailable()); ASSERT_TRUE(!!ref_to_ruleset); ASSERT_TRUE(has_cached_ruleset()); ruleset_dealer()->SetRulesetFile( testing::TestRuleset::Open(test_indexed_ruleset_2())); - EXPECT_TRUE(ruleset_dealer()->IsRulesetAvailable()); + EXPECT_TRUE(ruleset_dealer()->IsRulesetFileAvailable()); EXPECT_FALSE(has_cached_ruleset()); // Check that nothing explodes if the last reference to the old ruleset is // dropped after it is no longer cached by the RulesetDealer. ref_to_ruleset = nullptr; - EXPECT_TRUE(ruleset_dealer()->IsRulesetAvailable()); + EXPECT_TRUE(ruleset_dealer()->IsRulesetFileAvailable()); EXPECT_FALSE(has_cached_ruleset()); } @@ -223,18 +223,18 @@ ruleset_dealer()->SetRulesetFile( testing::TestRuleset::Open(test_indexed_ruleset_2())); - EXPECT_TRUE(ruleset_dealer()->IsRulesetAvailable()); + EXPECT_TRUE(ruleset_dealer()->IsRulesetFileAvailable()); scoped_refptr<const MemoryMappedRuleset> ref_to_ruleset_2 = ruleset_dealer()->GetRuleset(); - EXPECT_TRUE(ruleset_dealer()->IsRulesetAvailable()); + EXPECT_TRUE(ruleset_dealer()->IsRulesetFileAvailable()); ref_to_ruleset_1 = nullptr; - EXPECT_TRUE(ruleset_dealer()->IsRulesetAvailable()); + EXPECT_TRUE(ruleset_dealer()->IsRulesetFileAvailable()); scoped_refptr<const MemoryMappedRuleset> another_ref_to_ruleset_2 = ruleset_dealer()->GetRuleset(); - EXPECT_TRUE(ruleset_dealer()->IsRulesetAvailable()); + EXPECT_TRUE(ruleset_dealer()->IsRulesetFileAvailable()); EXPECT_TRUE(has_cached_ruleset()); ASSERT_TRUE(!!ref_to_ruleset_2); EXPECT_TRUE(!!another_ref_to_ruleset_2);
diff --git a/components/subresource_filter/content/renderer/subresource_filter_agent.cc b/components/subresource_filter/content/renderer/subresource_filter_agent.cc index 7290e0d..876cea1 100644 --- a/components/subresource_filter/content/renderer/subresource_filter_agent.cc +++ b/components/subresource_filter/content/renderer/subresource_filter_agent.cc
@@ -84,7 +84,7 @@ if (activation_state_for_provisional_load_ != ActivationState::DISABLED) { UMA_HISTOGRAM_BOOLEAN("SubresourceFilter.DocumentLoad.RulesetIsAvailable", - ruleset_dealer_->IsRulesetAvailable()); + ruleset_dealer_->IsRulesetFileAvailable()); } } @@ -163,7 +163,7 @@ ancestor_document_urls.front().SchemeIsFile()) { RecordHistogramsOnLoadCommitted(); if (activation_state_for_provisional_load_ != ActivationState::DISABLED && - ruleset_dealer_->IsRulesetAvailable()) { + ruleset_dealer_->IsRulesetFileAvailable()) { base::Closure first_disallowed_load_callback( base::Bind(&SubresourceFilterAgent:: SignalFirstSubresourceDisallowedForCommittedLoad,
diff --git a/components/subresource_filter/core/common/test_ruleset_creator.cc b/components/subresource_filter/core/common/test_ruleset_creator.cc index 08cd6271..81aabf8 100644 --- a/components/subresource_filter/core/common/test_ruleset_creator.cc +++ b/components/subresource_filter/core/common/test_ruleset_creator.cc
@@ -4,6 +4,8 @@ #include "components/subresource_filter/core/common/test_ruleset_creator.h" +#include <string> + #include "base/files/file_util.h" #include "base/logging.h" #include "base/strings/string_number_conversions.h" @@ -21,6 +23,15 @@ // The methods below assume that char and uint8_t are interchangeable. static_assert(CHAR_BIT == 8, "Assumed char was 8 bits."); +void WriteRulesetContents(const std::vector<uint8_t>& contents, + base::FilePath path) { + int ruleset_size_as_int = base::checked_cast<int>(contents.size()); + int num_bytes_written = + base::WriteFile(path, reinterpret_cast<const char*>(contents.data()), + ruleset_size_as_int); + ASSERT_EQ(ruleset_size_as_int, num_bytes_written); +} + std::vector<uint8_t> SerializeUnindexedRulesetWithMultipleRules( const std::vector<proto::UrlRule>& rules) { std::string ruleset_contents; @@ -65,6 +76,33 @@ return file; } +// static +void TestRuleset::CorruptByTruncating(const TestRuleset& ruleset, + size_t tail_size) { + ASSERT_GT(tail_size, 0u); + ASSERT_FALSE(ruleset.contents.empty()); + std::vector<uint8_t> new_contents = ruleset.contents; + + const size_t new_size = + tail_size < new_contents.size() ? new_contents.size() - tail_size : 0; + new_contents.resize(new_size); + WriteRulesetContents(new_contents, ruleset.path); +} + +// static +void TestRuleset::CorruptByFilling(const TestRuleset& ruleset, + size_t from, + size_t to, + uint8_t fill_with) { + ASSERT_LT(from, to); + ASSERT_LE(to, ruleset.contents.size()); + + std::vector<uint8_t> new_contents = ruleset.contents; + for (size_t i = from; i < to; ++i) + new_contents[i] = fill_with; + WriteRulesetContents(new_contents, ruleset.path); +} + // TestRulesetPair ------------------------------------------------------------- TestRulesetPair::TestRulesetPair() = default; @@ -93,6 +131,21 @@ test_unindexed_ruleset)); } +void TestRulesetCreator::CreateRulesetToDisallowURLsWithManySuffixes( + base::StringPiece suffix, + int num_of_suffixes, + TestRulesetPair* test_ruleset_pair) { + DCHECK(test_ruleset_pair); + + std::vector<proto::UrlRule> rules; + for (int i = 0; i < num_of_suffixes; ++i) { + std::string current_suffix = + suffix.as_string() + '_' + base::IntToString(i); + rules.push_back(CreateSuffixRule(current_suffix)); + } + CreateRulesetWithRules(rules, test_ruleset_pair); +} + void TestRulesetCreator::CreateRulesetWithRules( const std::vector<proto::UrlRule>& rules, TestRulesetPair* test_ruleset_pair) { @@ -119,11 +172,7 @@ ruleset->contents = std::move(ruleset_contents); ASSERT_NO_FATAL_FAILURE(GetUniqueTemporaryPath(&ruleset->path)); - int ruleset_size_as_int = base::checked_cast<int>(ruleset->contents.size()); - int num_bytes_written = base::WriteFile( - ruleset->path, reinterpret_cast<const char*>(ruleset->contents.data()), - ruleset_size_as_int); - ASSERT_EQ(ruleset_size_as_int, num_bytes_written); + WriteRulesetContents(ruleset->contents, ruleset->path); } } // namespace testing
diff --git a/components/subresource_filter/core/common/test_ruleset_creator.h b/components/subresource_filter/core/common/test_ruleset_creator.h index eef00b0..9ff0b53e 100644 --- a/components/subresource_filter/core/common/test_ruleset_creator.h +++ b/components/subresource_filter/core/common/test_ruleset_creator.h
@@ -30,6 +30,15 @@ // Convenience function to open a read-only file handle to |ruleset|. static base::File Open(const TestRuleset& ruleset); + // Corrupts the |ruleset| file by truncating its tail of a certain size. + static void CorruptByTruncating(const TestRuleset& ruleset, size_t tail_size); + + // Overrides all bytes in the [from..to) range by |fill_with|. + static void CorruptByFilling(const TestRuleset& ruleset, + size_t from, + size_t to, + uint8_t fill_with); + std::vector<uint8_t> contents; base::FilePath path; }; @@ -61,14 +70,22 @@ base::StringPiece suffix, TestRulesetPair* test_ruleset_pair); - void CreateRulesetWithRules(const std::vector<proto::UrlRule>& rules, - TestRulesetPair* test_ruleset_pair); - // Same as above, but only creates an unindexed ruleset. void CreateUnindexedRulesetToDisallowURLsWithPathSuffix( base::StringPiece suffix, TestRuleset* test_unindexed_ruleset); + // Similar to CreateRulesetToDisallowURLsWithPathSuffix, but the resulting + // ruleset consists of |num_of_suffixes| rules, each of them disallowing URLs + // with suffixes of the form |suffix|_k, 0 <= k < |num_of_suffixes|. + void CreateRulesetToDisallowURLsWithManySuffixes( + base::StringPiece suffix, + int num_of_suffixes, + TestRulesetPair* test_ruleset_pair); + + void CreateRulesetWithRules(const std::vector<proto::UrlRule>& rules, + TestRulesetPair* test_ruleset_pair); + // Returns a unique |path| that is valid for the lifetime of this instance. // No file at |path| will be automatically created. void GetUniqueTemporaryPath(base::FilePath* path);
diff --git a/components/sync/base/model_type.h b/components/sync/base/model_type.h index 5cca1a08..8ce553a 100644 --- a/components/sync/base/model_type.h +++ b/components/sync/base/model_type.h
@@ -16,8 +16,8 @@ namespace base { class ListValue; -class StringValue; class Value; +using StringValue = Value; } namespace sync_pb {
diff --git a/components/sync/syncable/syncable_id.h b/components/sync/syncable/syncable_id.h index 28e2b6b..df74680 100644 --- a/components/sync/syncable/syncable_id.h +++ b/components/sync/syncable/syncable_id.h
@@ -15,7 +15,8 @@ #include "base/trace_event/memory_usage_estimator.h" namespace base { -class StringValue; +class Value; +using StringValue = Value; } // namespace base namespace sql {
diff --git a/content/common/sandbox_linux/android/sandbox_bpf_base_policy_android.cc b/content/common/sandbox_linux/android/sandbox_bpf_base_policy_android.cc index be6a91c..fed5af3 100644 --- a/content/common/sandbox_linux/android/sandbox_bpf_base_policy_android.cc +++ b/content/common/sandbox_linux/android/sandbox_bpf_base_policy_android.cc
@@ -38,6 +38,8 @@ #define SOCK_NONBLOCK O_NONBLOCK #endif +#define CASES SANDBOX_BPF_DSL_CASES + namespace { #if !defined(__i386__) @@ -177,16 +179,26 @@ // documented to be a valid errno, but we will use it anyways. return Error(EPERM); } + + // https://crbug.com/682488 + if (sysno == __NR_getsockopt || sysno == __NR_setsockopt) { + // The baseline policy applies other restrictions to these syscalls. + const Arg<int> level(1); + const Arg<int> option(2); + return If(AllOf(level == SOL_SOCKET, option == SO_SNDTIMEO), Allow()) + .Else(SandboxBPFBasePolicy::EvaluateSyscall(sysno)); + } #elif defined(__i386__) if (sysno == __NR_socketcall) { + // The baseline policy allows other socketcall sub-calls. const Arg<int> socketcall(0); - const Arg<int> domain(1); - const Arg<int> type(2); - const Arg<int> protocol(3); - return If(socketcall == SYS_CONNECT, Allow()) - .ElseIf(socketcall == SYS_SOCKET, Allow()) - .ElseIf(socketcall == SYS_GETSOCKOPT, Allow()) - .Else(Error(EPERM)); + return Switch(socketcall) + .CASES((SYS_CONNECT, + SYS_SOCKET, + SYS_SETSOCKOPT, + SYS_GETSOCKOPT), + Allow()) + .Default(SandboxBPFBasePolicy::EvaluateSyscall(sysno)); } #endif
diff --git a/extensions/browser/api/idle/idle_manager.h b/extensions/browser/api/idle/idle_manager.h index d8ff5a03..2dd296b 100644 --- a/extensions/browser/api/idle/idle_manager.h +++ b/extensions/browser/api/idle/idle_manager.h
@@ -22,7 +22,8 @@ #include "ui/base/idle/idle.h" namespace base { -class StringValue; +class Value; +using StringValue = Value; } // namespace base namespace content {
diff --git a/ios/chrome/app/strings/ios_strings.grd b/ios/chrome/app/strings/ios_strings.grd index 50a4d1b9..7791700 100644 --- a/ios/chrome/app/strings/ios_strings.grd +++ b/ios/chrome/app/strings/ios_strings.grd
@@ -354,18 +354,12 @@ <message name="IDS_IOS_BOOKMARK_NAME_FIELD_HEADER" desc="Title shown above bookmark name field on Edit Bookmark screen. [Length: 20em]"> Name </message> - <message name="IDS_IOS_BOOKMARK_NEW_ALL_BOOKMARKS_LABEL" desc="A label for a button that takes the user to a screen where they can see all their bookmarks. [Length: 30em.]"> - All Bookmarks - </message> <message name="IDS_IOS_BOOKMARK_NEW_BACK_LABEL" desc="Accessibility label for a button that takes the user back to the previous screen. [Length: 30em.]"> Back </message> <message name="IDS_IOS_BOOKMARK_NEW_BOOKMARKS_BAR_TITLE" desc="Text that describes the bookmarks present in the bookmarks bar for chrome on desktop. [Length: 30em.] [iOS only]"> Bookmarks Bar </message> - <message name="IDS_IOS_BOOKMARK_NEW_BOOKMARKS_LABEL" desc="A title for the screen where the user can see all their bookmarks. [Length: 30em.]"> - Bookmarks - </message> <message name="IDS_IOS_BOOKMARK_NEW_BOOKMARK_CREATED" desc="A description presented to the user that they just created a bookmark. [Length: 30em.] [iOS only]"> Item created </message>
diff --git a/ios/chrome/browser/about_flags.mm b/ios/chrome/browser/about_flags.mm index 42b945f..ca7030b 100644 --- a/ios/chrome/browser/about_flags.mm +++ b/ios/chrome/browser/about_flags.mm
@@ -122,15 +122,6 @@ command_line->AppendSwitch(switches::kDisableLRUSnapshotCache); } - // Populate command line flag for the AllBookmarks from the - // configuration plist. - NSString* enableAllBookmarks = [defaults stringForKey:@"AllBookmarks"]; - if ([enableAllBookmarks isEqualToString:@"Enabled"]) { - command_line->AppendSwitch(switches::kEnableAllBookmarksView); - } else if ([enableAllBookmarks isEqualToString:@"Disabled"]) { - command_line->AppendSwitch(switches::kDisableAllBookmarksView); - } - // Populate command line flags from PasswordGenerationEnabled. NSString* enablePasswordGenerationValue = [defaults stringForKey:@"PasswordGenerationEnabled"];
diff --git a/ios/chrome/browser/chrome_switches.cc b/ios/chrome/browser/chrome_switches.cc index 2d4c469..2f92cc5 100644 --- a/ios/chrome/browser/chrome_switches.cc +++ b/ios/chrome/browser/chrome_switches.cc
@@ -12,9 +12,6 @@ // all work out. // ----------------------------------------------------------------------------- -// Disables all bookmarks view in bookmark manager. -const char kDisableAllBookmarksView[] = "disable-all-bookmarks-view"; - // Disables Contextual Search. const char kDisableContextualSearch[] = "disable-contextual-search"; @@ -65,9 +62,6 @@ // Disables the Suggestions UI const char kDisableSuggestionsUI[] = "disable-suggestions-ui"; -// Enables all bookmarks view in bookmark manager. -const char kEnableAllBookmarksView[] = "enable-all-bookmarks-view"; - // Enables Contextual Search. const char kEnableContextualSearch[] = "enable-contextual-search";
diff --git a/ios/chrome/browser/chrome_switches.h b/ios/chrome/browser/chrome_switches.h index 852ba2b..93f34187 100644 --- a/ios/chrome/browser/chrome_switches.h +++ b/ios/chrome/browser/chrome_switches.h
@@ -9,7 +9,6 @@ namespace switches { -extern const char kDisableAllBookmarksView[]; extern const char kDisableContextualSearch[]; extern const char kDisableIOSFastWebScrollViewInsets[]; extern const char kDisableIOSFeatures[]; @@ -27,7 +26,6 @@ extern const char kDisableDownloadImageRenaming[]; extern const char kDisableSuggestionsUI[]; -extern const char kEnableAllBookmarksView[]; extern const char kEnableContextualSearch[]; extern const char kEnableIOSFastWebScrollViewInsets[]; extern const char kEnableIOSFeatures[];
diff --git a/ios/chrome/browser/content_suggestions/BUILD.gn b/ios/chrome/browser/content_suggestions/BUILD.gn index 8a5a3d5..ce76fb1 100644 --- a/ios/chrome/browser/content_suggestions/BUILD.gn +++ b/ios/chrome/browser/content_suggestions/BUILD.gn
@@ -7,11 +7,17 @@ sources = [ "content_suggestions_coordinator.h", "content_suggestions_coordinator.mm", + "content_suggestions_mediator.h", + "content_suggestions_mediator.mm", + "content_suggestions_service_bridge_observer.h", + "content_suggestions_service_bridge_observer.mm", ] deps = [ "//base", + "//components/ntp_snippets", "//ios/chrome/app/strings", "//ios/chrome/browser", + "//ios/chrome/browser/ntp_snippets", "//ios/chrome/browser/ui/content_suggestions", "//ui/base", ]
diff --git a/ios/chrome/browser/content_suggestions/content_suggestions_coordinator.h b/ios/chrome/browser/content_suggestions/content_suggestions_coordinator.h index 8c051bb..7a1bcf0 100644 --- a/ios/chrome/browser/content_suggestions/content_suggestions_coordinator.h +++ b/ios/chrome/browser/content_suggestions/content_suggestions_coordinator.h
@@ -7,10 +7,16 @@ #import "ios/chrome/browser/chrome_coordinator.h" +namespace ios { +class ChromeBrowserState; +} + // Coordinator to manage the Suggestions UI via a // ContentSuggestionsViewController. @interface ContentSuggestionsCoordinator : ChromeCoordinator +// BrowserState used to create the ContentSuggestionFactory. +@property(nonatomic, assign) ios::ChromeBrowserState* browserState; // Whether the Suggestions UI is displayed. If this is true, start is a no-op. @property(nonatomic, readonly) BOOL visible;
diff --git a/ios/chrome/browser/content_suggestions/content_suggestions_coordinator.mm b/ios/chrome/browser/content_suggestions/content_suggestions_coordinator.mm index 0df1af28..bbc9d7a 100644 --- a/ios/chrome/browser/content_suggestions/content_suggestions_coordinator.mm +++ b/ios/chrome/browser/content_suggestions/content_suggestions_coordinator.mm
@@ -5,6 +5,8 @@ #import "ios/chrome/browser/content_suggestions/content_suggestions_coordinator.h" #include "base/mac/scoped_nsobject.h" +#import "ios/chrome/browser/content_suggestions/content_suggestions_mediator.h" +#include "ios/chrome/browser/ntp_snippets/ios_chrome_content_suggestions_service_factory.h" #import "ios/chrome/browser/ui/content_suggestions/content_suggestions_commands.h" #import "ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.h" #include "ios/chrome/grit/ios_strings.h" @@ -16,25 +18,33 @@ @interface ContentSuggestionsCoordinator ()<ContentSuggestionsCommands> { UINavigationController* _navigationController; + ContentSuggestionsMediator* _contentSuggestionsMediator; } @end @implementation ContentSuggestionsCoordinator +@synthesize browserState = _browserState; @synthesize visible = _visible; - (void)start { - if (self.visible) { - // Prevent this coordinator from being started twice in a row. + if (self.visible || !self.browserState) { + // Prevent this coordinator from being started twice in a row or without a + // browser state. return; } _visible = YES; + _contentSuggestionsMediator = [[ContentSuggestionsMediator alloc] + initWithContentService:IOSChromeContentSuggestionsServiceFactory:: + GetForBrowserState(self.browserState)]; + ContentSuggestionsViewController* suggestionsViewController = [[ContentSuggestionsViewController alloc] - initWithStyle:CollectionViewControllerStyleDefault]; + initWithStyle:CollectionViewControllerStyleDefault + dataSource:_contentSuggestionsMediator]; suggestionsViewController.suggestionCommandHandler = self; _navigationController = [[UINavigationController alloc]
diff --git a/ios/chrome/browser/content_suggestions/content_suggestions_mediator.h b/ios/chrome/browser/content_suggestions/content_suggestions_mediator.h new file mode 100644 index 0000000..f53553a --- /dev/null +++ b/ios/chrome/browser/content_suggestions/content_suggestions_mediator.h
@@ -0,0 +1,30 @@ +// 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 IOS_CHROME_BROWSER_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_MEDIATOR_H_ +#define IOS_CHROME_BROWSER_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_MEDIATOR_H_ + +#import <UIKit/UIKit.h> + +#import "ios/chrome/browser/content_suggestions/content_suggestions_mediator.h" +#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_data_source.h" + +namespace ntp_snippets { +class ContentSuggestionsService; +} + +// Mediator for ContentSuggestions. Makes the interface between a +// ntp_snippets::ContentSuggestionsService and the Objective-C services using +// its data. +@interface ContentSuggestionsMediator : NSObject<ContentSuggestionsDataSource> + +// Initialize the mediator with the |contentService| to mediate. +- (instancetype)initWithContentService: + (ntp_snippets::ContentSuggestionsService*)contentService + NS_DESIGNATED_INITIALIZER; +- (instancetype)init NS_UNAVAILABLE; + +@end + +#endif // IOS_CHROME_BROWSER_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_MEDIATOR_H_
diff --git a/ios/chrome/browser/content_suggestions/content_suggestions_mediator.mm b/ios/chrome/browser/content_suggestions/content_suggestions_mediator.mm new file mode 100644 index 0000000..c87b163 --- /dev/null +++ b/ios/chrome/browser/content_suggestions/content_suggestions_mediator.mm
@@ -0,0 +1,113 @@ +// 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. + +#import "ios/chrome/browser/content_suggestions/content_suggestions_mediator.h" + +#include "base/memory/ptr_util.h" +#include "components/ntp_snippets/category.h" +#include "components/ntp_snippets/content_suggestion.h" +#import "ios/chrome/browser/content_suggestions/content_suggestions_service_bridge_observer.h" +#import "ios/chrome/browser/ui/content_suggestions/content_suggestion.h" +#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_data_sink.h" + +#if !defined(__has_feature) || !__has_feature(objc_arc) +#error "This file requires ARC support." +#endif + +@interface ContentSuggestionsMediator ()<ContentSuggestionsServiceObserver> { + // Bridge for this class to become an observer of a ContentSuggestionsService. + std::unique_ptr<ContentSuggestionsServiceBridge> _suggestionBridge; +} + +@property(nonatomic, assign) + ntp_snippets::ContentSuggestionsService* contentService; + +// Converts the data in |category| to ContentSuggestion and adds them to the +// |contentArray|. +- (void)addContentInCategory:(ntp_snippets::Category&)category + toArray:(NSMutableArray<ContentSuggestion*>*)contentArray; + +// Converts the |contentsuggestion| to a ContentSuggestion. +- (ContentSuggestion*)convertContentSuggestion: + (const ntp_snippets::ContentSuggestion&)contentsuggestion; + +@end + +@implementation ContentSuggestionsMediator + +@synthesize contentService = _contentService; +@synthesize dataSink = _dataSink; + +- (instancetype)initWithContentService: + (ntp_snippets::ContentSuggestionsService*)contentService { + self = [super init]; + if (self) { + _suggestionBridge = + base::MakeUnique<ContentSuggestionsServiceBridge>(self, contentService); + _contentService = contentService; + } + return self; +} + +#pragma mark - ContentSuggestionsDataSource + +- (NSArray<ContentSuggestion*>*)allSuggestions { + std::vector<ntp_snippets::Category> categories = + self.contentService->GetCategories(); + NSMutableArray<ContentSuggestion*>* dataHolders = [NSMutableArray array]; + for (auto& category : categories) { + [self addContentInCategory:category toArray:dataHolders]; + } + return dataHolders; +} + +#pragma mark - ContentSuggestionsServiceObserver + +- (void)contentSuggestionsService: + (ntp_snippets::ContentSuggestionsService*)suggestionsService + newSuggestionsInCategory:(ntp_snippets::Category)category { + [self.dataSink dataAvailable]; +} + +- (void)contentSuggestionsService: + (ntp_snippets::ContentSuggestionsService*)suggestionsService + category:(ntp_snippets::Category)category + statusChangedTo:(ntp_snippets::CategoryStatus)status { + // Update dataSink. +} + +- (void)contentSuggestionsService: + (ntp_snippets::ContentSuggestionsService*)suggestionsService + SuggestionInvalidated: + (const ntp_snippets::ContentSuggestion::ID&)suggestion_id { + // Update dataSink. +} + +- (void)contentSuggestionsServiceFullRefreshRequired: + (ntp_snippets::ContentSuggestionsService*)suggestionsService { + // Update dataSink. +} + +- (void)contentSuggestionsServiceShutdown: + (ntp_snippets::ContentSuggestionsService*)suggestionsService { + // Update dataSink. +} + +#pragma mark - Private + +- (void)addContentInCategory:(ntp_snippets::Category&)category + toArray:(NSMutableArray<ContentSuggestion*>*)contentArray { + const std::vector<ntp_snippets::ContentSuggestion>& suggestions = + self.contentService->GetSuggestionsForCategory(category); + for (auto& contentSuggestion : suggestions) { + [contentArray addObject:[self convertContentSuggestion:contentSuggestion]]; + } +} + +- (ContentSuggestion*)convertContentSuggestion: + (const ntp_snippets::ContentSuggestion&)contentsuggestion { + return [[ContentSuggestion alloc] init]; +} + +@end
diff --git a/ios/chrome/browser/content_suggestions/content_suggestions_service_bridge_observer.h b/ios/chrome/browser/content_suggestions/content_suggestions_service_bridge_observer.h new file mode 100644 index 0000000..50ed8590 --- /dev/null +++ b/ios/chrome/browser/content_suggestions/content_suggestions_service_bridge_observer.h
@@ -0,0 +1,69 @@ +// 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 IOS_CHROME_BROWSER_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_SERVICE_BRIDGE_OBSERVER_H_ +#define IOS_CHROME_BROWSER_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_SERVICE_BRIDGE_OBSERVER_H_ + +#include "components/ntp_snippets/content_suggestions_service.h" + +// Observes ContentSuggestionsService events from Objective-C. To use as a +// ntp_snippets::ContentSuggestionsService::Observer, wrap in a +// ContentSuggestionsServiceBridge. +@protocol ContentSuggestionsServiceObserver + +// Invoked by ntp_snippets::ContentSuggestionsService::OnNewSuggestions. +- (void)contentSuggestionsService: + (ntp_snippets::ContentSuggestionsService*)suggestionsService + newSuggestionsInCategory:(ntp_snippets::Category)category; + +// Invoked by ntp_snippets::ContentSuggestionsService::OnCategoryStatusChanged. +- (void)contentSuggestionsService: + (ntp_snippets::ContentSuggestionsService*)suggestionsService + category:(ntp_snippets::Category)category + statusChangedTo:(ntp_snippets::CategoryStatus)status; + +// Invoked by ntp_snippets::ContentSuggestionsService::OnSuggestionInvalidated. +- (void)contentSuggestionsService: + (ntp_snippets::ContentSuggestionsService*)suggestionsService + SuggestionInvalidated: + (const ntp_snippets::ContentSuggestion::ID&)suggestion_id; + +// Invoked by ntp_snippets::ContentSuggestionsService::OnFullRefreshRequired. +- (void)contentSuggestionsServiceFullRefreshRequired: + (ntp_snippets::ContentSuggestionsService*)suggestionsService; + +// Invoked by +// ntp_snippets::ContentSuggestionsService::ContentSuggestionsServiceShutdown. +- (void)contentSuggestionsServiceShutdown: + (ntp_snippets::ContentSuggestionsService*)suggestionsService; + +@end + +// Observer for the ContentSuggestionsService that translates all the callbacks +// to Objective-C calls. +class ContentSuggestionsServiceBridge + : public ntp_snippets::ContentSuggestionsService::Observer { + public: + ContentSuggestionsServiceBridge( + id<ContentSuggestionsServiceObserver> observer, + ntp_snippets::ContentSuggestionsService* service); + ~ContentSuggestionsServiceBridge() override; + + private: + void OnNewSuggestions(ntp_snippets::Category category) override; + void OnCategoryStatusChanged( + ntp_snippets::Category category, + ntp_snippets::CategoryStatus new_status) override; + void OnSuggestionInvalidated( + const ntp_snippets::ContentSuggestion::ID& suggestion_id) override; + void OnFullRefreshRequired() override; + void ContentSuggestionsServiceShutdown() override; + + __unsafe_unretained id<ContentSuggestionsServiceObserver> observer_; + ntp_snippets::ContentSuggestionsService* service_; // weak + + DISALLOW_COPY_AND_ASSIGN(ContentSuggestionsServiceBridge); +}; + +#endif // IOS_CHROME_BROWSER_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_SERVICE_BRIDGE_OBSERVER_H_
diff --git a/ios/chrome/browser/content_suggestions/content_suggestions_service_bridge_observer.mm b/ios/chrome/browser/content_suggestions/content_suggestions_service_bridge_observer.mm new file mode 100644 index 0000000..90d3d7c --- /dev/null +++ b/ios/chrome/browser/content_suggestions/content_suggestions_service_bridge_observer.mm
@@ -0,0 +1,49 @@ +// 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. + +#import "ios/chrome/browser/content_suggestions/content_suggestions_service_bridge_observer.h" + +#if !defined(__has_feature) || !__has_feature(objc_arc) +#error "This file requires ARC support." +#endif + +ContentSuggestionsServiceBridge::ContentSuggestionsServiceBridge( + id<ContentSuggestionsServiceObserver> observer, + ntp_snippets::ContentSuggestionsService* service) { + observer_ = observer; + service_ = service; + service->AddObserver(this); +} + +ContentSuggestionsServiceBridge::~ContentSuggestionsServiceBridge() { + service_->RemoveObserver(this); +} + +void ContentSuggestionsServiceBridge::OnNewSuggestions( + ntp_snippets::Category category) { + [observer_ contentSuggestionsService:service_ + newSuggestionsInCategory:category]; +} + +void ContentSuggestionsServiceBridge::OnCategoryStatusChanged( + ntp_snippets::Category category, + ntp_snippets::CategoryStatus new_status) { + [observer_ contentSuggestionsService:service_ + category:category + statusChangedTo:new_status]; +} + +void ContentSuggestionsServiceBridge::OnSuggestionInvalidated( + const ntp_snippets::ContentSuggestion::ID& suggestion_id) { + [observer_ contentSuggestionsService:service_ + SuggestionInvalidated:suggestion_id]; +} + +void ContentSuggestionsServiceBridge::OnFullRefreshRequired() { + [observer_ contentSuggestionsServiceFullRefreshRequired:service_]; +} + +void ContentSuggestionsServiceBridge::ContentSuggestionsServiceShutdown() { + [observer_ contentSuggestionsServiceShutdown:service_]; +}
diff --git a/ios/chrome/browser/experimental_flags.h b/ios/chrome/browser/experimental_flags.h index a23319a..efdc73a 100644 --- a/ios/chrome/browser/experimental_flags.h +++ b/ios/chrome/browser/experimental_flags.h
@@ -43,9 +43,6 @@ // Whether background crash report upload should generate a local notification. bool IsAlertOnBackgroundUploadEnabled(); -// Whether the All Bookmarks view is visible in bookmarks. -bool IsAllBookmarksEnabled(); - // Whether auto-reload is enabled. bool IsAutoReloadEnabled();
diff --git a/ios/chrome/browser/experimental_flags.mm b/ios/chrome/browser/experimental_flags.mm index 8f83865..892a616 100644 --- a/ios/chrome/browser/experimental_flags.mm +++ b/ios/chrome/browser/experimental_flags.mm
@@ -85,25 +85,6 @@ boolForKey:kEnableAlertOnBackgroundUpload]; } -bool IsAllBookmarksEnabled() { - base::CommandLine* command_line = base::CommandLine::ForCurrentProcess(); - if (command_line->HasSwitch(switches::kEnableAllBookmarksView)) { - return true; - } else if (command_line->HasSwitch(switches::kDisableAllBookmarksView)) { - return false; - } - - // Check if the finch experiment exists. - std::string group_name = - base::FieldTrialList::FindFullName("RemoveAllBookmarks"); - - if (group_name.empty()) { - return false; // If no finch experiment, all bookmarks is disabled. - } - return base::StartsWith(group_name, "Enabled", - base::CompareCase::INSENSITIVE_ASCII); -} - bool IsAutoReloadEnabled() { std::string group_name = base::FieldTrialList::FindFullName("IOSAutoReload"); base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
diff --git a/ios/chrome/browser/native_app_launcher/BUILD.gn b/ios/chrome/browser/native_app_launcher/BUILD.gn index 687ac9db..99552a3 100644 --- a/ios/chrome/browser/native_app_launcher/BUILD.gn +++ b/ios/chrome/browser/native_app_launcher/BUILD.gn
@@ -51,6 +51,7 @@ } source_set("native_app_launcher_internal") { + configs += [ "//build/config/compiler:enable_arc" ] sources = [ "native_app_navigation_controller.h", "native_app_navigation_controller.mm",
diff --git a/ios/chrome/browser/native_app_launcher/DEPS b/ios/chrome/browser/native_app_launcher/DEPS index 1c40ab5f..eb9956d 100644 --- a/ios/chrome/browser/native_app_launcher/DEPS +++ b/ios/chrome/browser/native_app_launcher/DEPS
@@ -5,4 +5,7 @@ "+ios/web/navigation/navigation_manager_impl.h", "+ios/web/web_state/web_state_impl.h", ], + "^native_app_navigation_controller\.mm$": [ + "+ios/web/web_state/ui/crw_web_controller.h", + ], }
diff --git a/ios/chrome/browser/native_app_launcher/native_app_navigation_controller.mm b/ios/chrome/browser/native_app_launcher/native_app_navigation_controller.mm index bbb39766..5181252a 100644 --- a/ios/chrome/browser/native_app_launcher/native_app_navigation_controller.mm +++ b/ios/chrome/browser/native_app_launcher/native_app_navigation_controller.mm
@@ -20,9 +20,14 @@ #import "ios/public/provider/chrome/browser/native_app_launcher/native_app_types.h" #import "ios/public/provider/chrome/browser/native_app_launcher/native_app_whitelist_manager.h" #include "ios/web/public/web_state/web_state.h" +#import "ios/web/web_state/ui/crw_web_controller.h" #import "net/base/mac/url_conversions.h" #include "net/url_request/url_request_context_getter.h" +#if !defined(__has_feature) || !__has_feature(objc_arc) +#error "This file requires ARC support." +#endif + using base::UserMetricsAction; @interface NativeAppNavigationController () @@ -48,10 +53,10 @@ // DEPRECATED: Tab hosting the infobar and is also used for accessing Tab // states such as navigation manager and whether it is a pre-rendered tab. // Use |webState| whenever possible. - __unsafe_unretained Tab* _tab; // weak - base::scoped_nsprotocol<id<NativeAppMetadata>> _metadata; + __weak Tab* _tab; + id<NativeAppMetadata> _metadata; // A set of appIds encoded as NSStrings. - base::scoped_nsobject<NSMutableSet> _appsPossiblyBeingInstalled; + NSMutableSet* _appsPossiblyBeingInstalled; } // Designated initializer. Use this instead of -init. @@ -68,16 +73,16 @@ // same webState. DCHECK(!tab || [tab webState] == webState); _tab = tab; - _appsPossiblyBeingInstalled.reset([[NSMutableSet alloc] init]); + _appsPossiblyBeingInstalled = [[NSMutableSet alloc] init]; } return self; } - (void)copyStateFrom:(NativeAppNavigationController*)controller { DCHECK(controller); - _appsPossiblyBeingInstalled.reset([[NSMutableSet alloc] - initWithSet:[controller appsPossiblyBeingInstalled]]); - for (NSString* appIdString in _appsPossiblyBeingInstalled.get()) { + _appsPossiblyBeingInstalled = [[NSMutableSet alloc] + initWithSet:[controller appsPossiblyBeingInstalled]]; + for (NSString* appIdString in _appsPossiblyBeingInstalled) { DCHECK([appIdString isKindOfClass:[NSString class]]); NSURL* appURL = [ios::GetChromeBrowserProvider()->GetNativeAppWhitelistManager() @@ -92,7 +97,6 @@ - (void)dealloc { [[InstallationNotifier sharedInstance] unregisterForNotifications:self]; - [super dealloc]; } - (NSMutableSet*)appsPossiblyBeingInstalled { @@ -102,9 +106,8 @@ - (void)showInfoBarIfNecessary { // Find a potential matching native app. GURL pageURL = _webState->GetLastCommittedURL(); - _metadata.reset( - [ios::GetChromeBrowserProvider()->GetNativeAppWhitelistManager() - newNativeAppForURL:pageURL]); + _metadata = [ios::GetChromeBrowserProvider()->GetNativeAppWhitelistManager() + nativeAppForURL:pageURL]; if (!_metadata || [_metadata shouldBypassInfoBars]) return;
diff --git a/ios/chrome/browser/native_app_launcher/native_app_navigation_util.mm b/ios/chrome/browser/native_app_launcher/native_app_navigation_util.mm index 7a6c34e..072e0d8 100644 --- a/ios/chrome/browser/native_app_launcher/native_app_navigation_util.mm +++ b/ios/chrome/browser/native_app_launcher/native_app_navigation_util.mm
@@ -9,6 +9,10 @@ #include "ios/web/public/navigation_manager.h" #import "ios/web/public/web_state/web_state.h" +#if !defined(__has_feature) || !__has_feature(objc_arc) +#error "This file requires ARC support." +#endif + namespace native_app_launcher { bool IsLinkNavigation(web::WebState* web_state) {
diff --git a/ios/chrome/browser/resources/Settings.bundle/Experimental.plist b/ios/chrome/browser/resources/Settings.bundle/Experimental.plist index 67dd19d..0b517c4 100644 --- a/ios/chrome/browser/resources/Settings.bundle/Experimental.plist +++ b/ios/chrome/browser/resources/Settings.bundle/Experimental.plist
@@ -128,28 +128,6 @@ <key>Type</key> <string>PSMultiValueSpecifier</string> <key>Title</key> - <string>Enable All Bookmarks</string> - <key>Key</key> - <string>AllBookmarks</string> - <key>DefaultValue</key> - <string></string> - <key>Titles</key> - <array> - <string>Default</string> - <string>Enabled</string> - <string>Disabled</string> - </array> - <key>Values</key> - <array> - <string></string> - <string>Enabled</string> - <string>Disabled</string> - </array> - </dict> - <dict> - <key>Type</key> - <string>PSMultiValueSpecifier</string> - <key>Title</key> <string>Enable Popular Sites</string> <key>Key</key> <string>EnablePopularSites</string>
diff --git a/ios/chrome/browser/suggestions/image_fetcher_impl.mm b/ios/chrome/browser/suggestions/image_fetcher_impl.mm index ceee532..c701dfc4 100644 --- a/ios/chrome/browser/suggestions/image_fetcher_impl.mm +++ b/ios/chrome/browser/suggestions/image_fetcher_impl.mm
@@ -40,7 +40,6 @@ DataUseServiceName data_use_service_name) { // Not implemented - will be obsolete once iOS also uses // image_fetcher::ImageDataFetcher. - NOTREACHED(); } void ImageFetcherImpl::StartOrQueueNetworkRequest(
diff --git a/ios/chrome/browser/tabs/tab.mm b/ios/chrome/browser/tabs/tab.mm index 36ff77d..670c8cc5 100644 --- a/ios/chrome/browser/tabs/tab.mm +++ b/ios/chrome/browser/tabs/tab.mm
@@ -1847,8 +1847,8 @@ return NO; base::scoped_nsprotocol<id<NativeAppMetadata>> metadata( - [ios::GetChromeBrowserProvider()->GetNativeAppWhitelistManager() - newNativeAppForURL:url]); + [[ios::GetChromeBrowserProvider()->GetNativeAppWhitelistManager() + nativeAppForURL:url] retain]); if (![metadata shouldAutoOpenLinks]) return NO;
diff --git a/ios/chrome/browser/ui/authentication/signin_interaction_controller_egtest.mm b/ios/chrome/browser/ui/authentication/signin_interaction_controller_egtest.mm index d827393..cd544f5 100644 --- a/ios/chrome/browser/ui/authentication/signin_interaction_controller_egtest.mm +++ b/ios/chrome/browser/ui/authentication/signin_interaction_controller_egtest.mm
@@ -547,9 +547,7 @@ } // Selects the top level folder (Sign In promo is only shown there). - NSString* topLevelFolderTitle = experimental_flags::IsAllBookmarksEnabled() - ? @"All Bookmarks" - : @"Mobile Bookmarks"; + NSString* topLevelFolderTitle = @"Mobile Bookmarks"; id<GREYMatcher> all_bookmarks_matcher = grey_allOf(grey_kindOfClass(NSClassFromString(@"BookmarkMenuCell")), grey_descendant(grey_text(topLevelFolderTitle)), nil);
diff --git a/ios/chrome/browser/ui/bookmarks/BUILD.gn b/ios/chrome/browser/ui/bookmarks/BUILD.gn index c589e26..807c3fd 100644 --- a/ios/chrome/browser/ui/bookmarks/BUILD.gn +++ b/ios/chrome/browser/ui/bookmarks/BUILD.gn
@@ -4,8 +4,6 @@ source_set("bookmarks") { sources = [ - "bookmark_all_collection_view.h", - "bookmark_all_collection_view.mm", "bookmark_collection_cells.h", "bookmark_collection_cells.mm", "bookmark_collection_view.h", @@ -204,9 +202,6 @@ "resources/bookmark_blue_folder.png", "resources/bookmark_blue_folder@2x.png", "resources/bookmark_blue_folder@3x.png", - "resources/bookmark_blue_star.png", - "resources/bookmark_blue_star@2x.png", - "resources/bookmark_blue_star@3x.png", "resources/bookmark_gray_back.png", "resources/bookmark_gray_back@2x.png", "resources/bookmark_gray_back@3x.png", @@ -228,9 +223,6 @@ "resources/bookmark_gray_new_folder.png", "resources/bookmark_gray_new_folder@2x.png", "resources/bookmark_gray_new_folder@3x.png", - "resources/bookmark_gray_star.png", - "resources/bookmark_gray_star@2x.png", - "resources/bookmark_gray_star@3x.png", "resources/bookmark_gray_star_large.png", "resources/bookmark_gray_star_large@2x.png", "resources/bookmark_gray_star_large@3x.png",
diff --git a/ios/chrome/browser/ui/bookmarks/bookmark_folder_collection_view.mm b/ios/chrome/browser/ui/bookmarks/bookmark_folder_collection_view.mm index 7f68a85..ffcbd4f 100644 --- a/ios/chrome/browser/ui/bookmarks/bookmark_folder_collection_view.mm +++ b/ios/chrome/browser/ui/bookmarks/bookmark_folder_collection_view.mm
@@ -439,9 +439,6 @@ } - (void)promoStateChangedAnimated:(BOOL)animate { - if (experimental_flags::IsAllBookmarksEnabled()) - return; // The promo is not shown if All Bookmarks is enabled. - BOOL newPromoState = !self.editing && self.folder && self.folder->type() == BookmarkNode::MOBILE &&
diff --git a/ios/chrome/browser/ui/bookmarks/bookmark_home_handset_view_controller.mm b/ios/chrome/browser/ui/bookmarks/bookmark_home_handset_view_controller.mm index ab58a75..6dcefb3 100644 --- a/ios/chrome/browser/ui/bookmarks/bookmark_home_handset_view_controller.mm +++ b/ios/chrome/browser/ui/bookmarks/bookmark_home_handset_view_controller.mm
@@ -21,7 +21,6 @@ #import "ios/chrome/browser/ui/alert_coordinator/action_sheet_coordinator.h" #import "ios/chrome/browser/ui/bookmarks/bars/bookmark_editing_bar.h" #import "ios/chrome/browser/ui/bookmarks/bars/bookmark_navigation_bar.h" -#import "ios/chrome/browser/ui/bookmarks/bookmark_all_collection_view.h" #import "ios/chrome/browser/ui/bookmarks/bookmark_collection_cells.h" #import "ios/chrome/browser/ui/bookmarks/bookmark_edit_view_controller.h" #import "ios/chrome/browser/ui/bookmarks/bookmark_folder_collection_view.h" @@ -68,7 +67,6 @@ // time, it contains exactly one of the BookmarkCollectionView subclasses. @property(nonatomic, retain) UIView* contentView; // The possible views that can be shown from the menu. -@property(nonatomic, retain) BookmarkAllCollectionView* allItemsView; @property(nonatomic, retain) BookmarkFolderCollectionView* folderView; // This view is created and used if the model is not fully loaded yet by the // time this controller starts. @@ -121,7 +119,6 @@ // view has been loaded, and the bookmark model is loaded. - (void)loadBookmarkViews; // If the view doesn't exist, create it. -- (void)ensureAllViewExists; - (void)ensureFolderViewExists; // Updates the property 'primaryMenuItem'. // Updates the UI to reflect the new state of 'primaryMenuItem'. @@ -219,7 +216,6 @@ @end @implementation BookmarkHomeHandsetViewController -@synthesize allItemsView = _allItemsView; @synthesize contentView = _contentView; @synthesize folderView = _folderView; @synthesize waitForModelView = _waitForModelView; @@ -257,7 +253,6 @@ } - (void)dealloc { - _allItemsView.delegate = nil; _folderView.delegate = nil; _menuView.delegate = nil; @@ -380,21 +375,6 @@ } } -- (void)ensureAllViewExists { - if (self.allItemsView) - return; - - base::scoped_nsobject<BookmarkAllCollectionView> view( - [[BookmarkAllCollectionView alloc] - initWithBrowserState:self.browserState - frame:[self frameForPrimaryView]]); - self.allItemsView = view; - self.allItemsView.delegate = self; - [self.allItemsView setEditing:self.editing animated:NO]; - self.allItemsView.autoresizingMask = - UIViewAutoresizingFlexibleHeight | UIViewAutoresizingFlexibleWidth; -} - - (void)ensureFolderViewExists { if (self.folderView) return; @@ -412,30 +392,21 @@ - (void)updatePrimaryMenuItem:(BookmarkMenuItem*)menuItem animated:(BOOL)animated { + DCHECK(menuItem.type == bookmarks::MenuItemFolder); if ([self.primaryMenuItem isEqual:menuItem]) return; // Disable editing on previous primary view before dismissing it. No need to // animate because this view is immediately removed from hierarchy. - [self.primaryView setEditing:NO animated:NO]; + if ([[self primaryMenuItem] supportsEditing]) + [self.primaryView setEditing:NO animated:NO]; [[self primaryView] removeFromSuperview]; self.primaryMenuItem = menuItem; - switch (self.primaryMenuItem.type) { - case bookmarks::MenuItemAll: - [self ensureAllViewExists]; - break; - case bookmarks::MenuItemFolder: - [self ensureFolderViewExists]; - [self.folderView resetFolder:self.primaryMenuItem.folder]; - [self.folderView promoStateChangedAnimated:NO]; - break; - case bookmarks::MenuItemDivider: - case bookmarks::MenuItemSectionHeader: - NOTREACHED(); - break; - } + [self ensureFolderViewExists]; + [self.folderView resetFolder:self.primaryMenuItem.folder]; + [self.folderView promoStateChangedAnimated:NO]; UIView* primaryView = [self primaryView]; [[self primaryView] changeOrientation:GetInterfaceOrientation()]; @@ -451,16 +422,9 @@ } - (UIView<BookmarkHomePrimaryView>*)primaryView { - switch (self.primaryMenuItem.type) { - case bookmarks::MenuItemAll: - return self.allItemsView; - case bookmarks::MenuItemFolder: - return self.folderView; - case bookmarks::MenuItemDivider: - case bookmarks::MenuItemSectionHeader: - NOTREACHED(); - return nil; - } + if (self.primaryMenuItem.type == bookmarks::MenuItemFolder) + return self.folderView; + return nil; } #pragma mark - Editing bar methods. @@ -1157,15 +1121,6 @@ #pragma mark - BookmarkPromoControllerDelegate - (void)promoStateChanged:(BOOL)promoEnabled { - [self.allItemsView.collectionView reloadData]; - // This is required to workaround a crash seen once on iOS 7.1 when - // the collection gets two reloadData without getting a call to layout - // subviews, the collection view will reuse some cached data for the perfoming - // the layout which are invalid after the second call to reloadData. - // Forcing the layout invalidation after each reloadData seems to fix the - // issue. - [self.allItemsView.collectionView.collectionViewLayout invalidateLayout]; - [self.folderView promoStateChangedAnimated:self.folderView == [self primaryView]]; }
diff --git a/ios/chrome/browser/ui/bookmarks/bookmark_home_tablet_ntp_controller.mm b/ios/chrome/browser/ui/bookmarks/bookmark_home_tablet_ntp_controller.mm index e4e521b..bbb6f7e 100644 --- a/ios/chrome/browser/ui/bookmarks/bookmark_home_tablet_ntp_controller.mm +++ b/ios/chrome/browser/ui/bookmarks/bookmark_home_tablet_ntp_controller.mm
@@ -24,7 +24,6 @@ #import "ios/chrome/browser/ui/alert_coordinator/action_sheet_coordinator.h" #import "ios/chrome/browser/ui/bookmarks/bars/bookmark_editing_bar.h" #import "ios/chrome/browser/ui/bookmarks/bars/bookmark_navigation_bar.h" -#import "ios/chrome/browser/ui/bookmarks/bookmark_all_collection_view.h" #import "ios/chrome/browser/ui/bookmarks/bookmark_edit_view_controller.h" #import "ios/chrome/browser/ui/bookmarks/bookmark_folder_collection_view.h" #import "ios/chrome/browser/ui/bookmarks/bookmark_folder_editor_view_controller.h" @@ -131,7 +130,6 @@ // time, it contains exactly one of the BookmarkCollectionView subclasses. @property(nonatomic, retain) ContentView* contentView; // The possible views that can be shown from the menu. -@property(nonatomic, retain) BookmarkAllCollectionView* allItemsView; @property(nonatomic, retain) BookmarkFolderCollectionView* folderView; // This view is created and used if the model is not fully loaded yet by the // time this controller starts. @@ -185,7 +183,6 @@ // loaded, and the bookmark model is loaded. - (void)loadBookmarkViews; // If the view doesn't exist, create it. -- (void)ensureAllViewExists; - (void)ensureFolderViewExists; // Updates the property 'primaryMenuItem'. // Updates the UI to reflect the new state of 'primaryMenuItem'. @@ -282,7 +279,6 @@ @synthesize bookmarks = _bookmarks; @synthesize contentView = _contentView; -@synthesize allItemsView = _allItemsView; @synthesize folderView = _folderView; @synthesize waitForModelView = _waitForModelView; @@ -329,7 +325,6 @@ - (void)dealloc { _contentView.delegate = nil; - _allItemsView.delegate = nil; _folderView.delegate = nil; _menuView.delegate = nil; @@ -506,20 +501,6 @@ } } -- (void)ensureAllViewExists { - if (self.allItemsView) - return; - - base::scoped_nsobject<BookmarkAllCollectionView> view( - [[BookmarkAllCollectionView alloc] initWithBrowserState:self.browserState - frame:CGRectZero]); - self.allItemsView = view; - self.allItemsView.delegate = self; - [self.allItemsView setEditing:self.editing animated:NO]; - self.allItemsView.autoresizingMask = - UIViewAutoresizingFlexibleHeight | UIViewAutoresizingFlexibleWidth; -} - - (void)ensureFolderViewExists { if (self.folderView) return; @@ -537,6 +518,7 @@ - (void)updatePrimaryMenuItem:(BookmarkMenuItem*)menuItem animated:(BOOL)animated { + DCHECK(menuItem.type == bookmarks::MenuItemFolder); if ([self.primaryMenuItem isEqual:menuItem]) return; @@ -546,20 +528,9 @@ [[self primaryView] removeFromSuperview]; self.primaryMenuItem = menuItem; - switch (self.primaryMenuItem.type) { - case bookmarks::MenuItemAll: - [self ensureAllViewExists]; - break; - case bookmarks::MenuItemFolder: - [self ensureFolderViewExists]; - [self.folderView resetFolder:self.primaryMenuItem.folder]; - [self.folderView promoStateChangedAnimated:NO]; - break; - case bookmarks::MenuItemDivider: - case bookmarks::MenuItemSectionHeader: - NOTREACHED(); - break; - } + [self ensureFolderViewExists]; + [self.folderView resetFolder:self.primaryMenuItem.folder]; + [self.folderView promoStateChangedAnimated:NO]; [[self primaryView] changeOrientation:GetInterfaceOrientation()]; [[self primaryView] setScrollsToTop:self.scrollToTop]; @@ -578,20 +549,9 @@ } - (UIView<BookmarkHomePrimaryView>*)primaryView { - if (!self.primaryMenuItem) - return nil; - DCHECK([self contentView]); - - switch (self.primaryMenuItem.type) { - case bookmarks::MenuItemAll: - return self.allItemsView; - case bookmarks::MenuItemFolder: - return self.folderView; - case bookmarks::MenuItemDivider: - case bookmarks::MenuItemSectionHeader: - NOTREACHED(); - return nil; - } + if (self.primaryMenuItem.type == bookmarks::MenuItemFolder) + return self.folderView; + return nil; } - (BOOL)shouldPresentMenuInSlideInPanel { @@ -1324,14 +1284,6 @@ #pragma mark - BookmarkPromoControllerDelegate - (void)promoStateChanged:(BOOL)promoEnabled { - [self.allItemsView.collectionView reloadData]; - // This is required to workaround a crash seen once on iOS 7.1 when - // the collection gets two reloadData without getting a call to layout - // subviews, the collection view will reuse some cached data for the perfoming - // the layout which are invalid after the second call to reloadData. - // Forcing the layout invalidation after each reloadData seems to fix the - // issue. - [self.allItemsView.collectionView.collectionViewLayout invalidateLayout]; [self.folderView promoStateChangedAnimated:self.folderView == [self primaryView]]; }
diff --git a/ios/chrome/browser/ui/bookmarks/bookmark_menu_item.h b/ios/chrome/browser/ui/bookmarks/bookmark_menu_item.h index f9a4b00..d453234 100644 --- a/ios/chrome/browser/ui/bookmarks/bookmark_menu_item.h +++ b/ios/chrome/browser/ui/bookmarks/bookmark_menu_item.h
@@ -16,8 +16,6 @@ // and must not be changed. New values must be added at the end, and if // a value is removed it should not be reused. typedef enum { - // Corresponds with the "all items" view. - MenuItemAll = 0, // A very thin divider. MenuItemDivider = 1, // A BookmarkNode* that is a folder. @@ -51,7 +49,6 @@ // Returns the menuitem located at the root ancestor of this item. - (BookmarkMenuItem*)parentItem; -+ (BookmarkMenuItem*)allMenuItem; + (BookmarkMenuItem*)dividerMenuItem; + (BookmarkMenuItem*)folderMenuItemForNode:(const bookmarks::BookmarkNode*)node rootAncestor:
diff --git a/ios/chrome/browser/ui/bookmarks/bookmark_menu_item.mm b/ios/chrome/browser/ui/bookmarks/bookmark_menu_item.mm index 45c6b0e..b5920fe9 100644 --- a/ios/chrome/browser/ui/bookmarks/bookmark_menu_item.mm +++ b/ios/chrome/browser/ui/bookmarks/bookmark_menu_item.mm
@@ -18,11 +18,10 @@ namespace bookmarks { BOOL NumberIsValidMenuItemType(int number) { // Invalid and deprecated numbers. - if (number < 0 || number > MenuItemLast) + if (number < 1 || number > MenuItemLast) return NO; MenuItemType type = static_cast<MenuItemType>(number); switch (type) { - case MenuItemAll: case MenuItemFolder: return YES; @@ -61,7 +60,6 @@ - (UIAccessibilityTraits)accessibilityTraits { switch (self.type) { - case bookmarks::MenuItemAll: case bookmarks::MenuItemFolder: return super.accessibilityTraits |= UIAccessibilityTraitButton; case bookmarks::MenuItemSectionHeader: @@ -73,8 +71,6 @@ - (NSString*)title { switch (self.type) { - case bookmarks::MenuItemAll: - return l10n_util::GetNSString(IDS_IOS_BOOKMARK_NEW_ALL_BOOKMARKS_LABEL); case bookmarks::MenuItemDivider: return nil; case bookmarks::MenuItemFolder: @@ -86,8 +82,6 @@ - (NSString*)titleForMenu { switch (self.type) { - case bookmarks::MenuItemAll: - return l10n_util::GetNSString(IDS_IOS_BOOKMARK_NEW_ALL_BOOKMARKS_LABEL); case bookmarks::MenuItemDivider: case bookmarks::MenuItemFolder: case bookmarks::MenuItemSectionHeader: @@ -97,8 +91,6 @@ - (NSString*)titleForNavigationBar { switch (self.type) { - case bookmarks::MenuItemAll: - return l10n_util::GetNSString(IDS_IOS_BOOKMARK_NEW_BOOKMARKS_LABEL); case bookmarks::MenuItemDivider: case bookmarks::MenuItemFolder: case bookmarks::MenuItemSectionHeader: @@ -108,8 +100,6 @@ - (NSString*)accessibilityIdentifier { switch (self.type) { - case bookmarks::MenuItemAll: - return @"MenuItemAll"; case bookmarks::MenuItemDivider: return nil; case bookmarks::MenuItemFolder: @@ -121,11 +111,6 @@ - (UIImage*)imagePrimary:(BOOL)primary { switch (self.type) { - case bookmarks::MenuItemAll: - if (primary) - return [UIImage imageNamed:@"bookmark_blue_star"]; - else - return [UIImage imageNamed:@"bookmark_gray_star"]; case bookmarks::MenuItemFolder: if (self.folder->type() == BookmarkNode::BOOKMARK_BAR || self.folder->type() == BookmarkNode::MOBILE || @@ -152,7 +137,6 @@ case bookmarks::MenuItemDivider: case bookmarks::MenuItemSectionHeader: return NO; - case bookmarks::MenuItemAll: case bookmarks::MenuItemFolder: return YES; } @@ -160,7 +144,6 @@ - (BOOL)supportsEditing { switch (self.type) { - case bookmarks::MenuItemAll: case bookmarks::MenuItemFolder: return YES; case bookmarks::MenuItemDivider: @@ -181,7 +164,6 @@ switch (self.type) { case bookmarks::MenuItemDivider: - case bookmarks::MenuItemAll: return YES; case bookmarks::MenuItemFolder: return self.folder == otherMenuItem.folder; @@ -203,7 +185,6 @@ - (NSUInteger)hash { switch (self.type) { case bookmarks::MenuItemDivider: - case bookmarks::MenuItemAll: return self.type; case bookmarks::MenuItemFolder: return self.type + reinterpret_cast<NSUInteger>(self.folder); @@ -212,12 +193,6 @@ } } -+ (BookmarkMenuItem*)allMenuItem { - BookmarkMenuItem* item = [[[BookmarkMenuItem alloc] init] autorelease]; - item.type = bookmarks::MenuItemAll; - return item; -} - + (BookmarkMenuItem*)dividerMenuItem { BookmarkMenuItem* item = [[[BookmarkMenuItem alloc] init] autorelease]; item.type = bookmarks::MenuItemDivider;
diff --git a/ios/chrome/browser/ui/bookmarks/bookmark_menu_view.mm b/ios/chrome/browser/ui/bookmarks/bookmark_menu_view.mm index bee854ae..680ec47 100644 --- a/ios/chrome/browser/ui/bookmarks/bookmark_menu_view.mm +++ b/ios/chrome/browser/ui/bookmarks/bookmark_menu_view.mm
@@ -137,18 +137,12 @@ [[NSMutableArray alloc] init]); [self.menuItems addObject:topSection]; - if (experimental_flags::IsAllBookmarksEnabled()) { - // All Items is always visible. - [topSection addObject:[BookmarkMenuItem allMenuItem]]; - } - // Bookmarks Bar, Mobile Bookmarks and Other Bookmarks are special folders and - // are shown at the top if they contain anything. - if (!mobileBookmarks->empty() || - !experimental_flags::IsAllBookmarksEnabled()) { - [topSection - addObject:[BookmarkMenuItem folderMenuItemForNode:mobileBookmarks - rootAncestor:mobileBookmarks]]; - } + // Mobile bookmark is shown even if empty. + [topSection + addObject:[BookmarkMenuItem folderMenuItemForNode:mobileBookmarks + rootAncestor:mobileBookmarks]]; + // Bookmarks Bar and Other Bookmarks are special folders and are shown at the + // top if they contain anything. if (!bookmarkBar->empty()) { [topSection addObject:[BookmarkMenuItem folderMenuItemForNode:bookmarkBar rootAncestor:bookmarkBar]]; @@ -268,15 +262,6 @@ return; } - if (node == self.primaryMenuItem.rootAncestor) { - // The deleted node is the root node of the current selected folder. Move to - // all items. - self.primaryMenuItem = [BookmarkMenuItem allMenuItem]; - [self.delegate bookmarkMenuView:self selectedMenuItem:self.primaryMenuItem]; - [self reloadData]; - return; - } - const BookmarkNode* root = RootLevelFolderForNode(parentFolder, self.bookmarkModel); @@ -400,8 +385,7 @@ shouldProcessInkTouchesAtTouchLocation:(CGPoint)location { NSIndexPath* indexPath = [self.tableView indexPathForRowAtPoint:location]; BookmarkMenuItem* menuItem = [self menuItemAtIndexPath:indexPath]; - return menuItem.type == bookmarks::MenuItemAll || - menuItem.type == bookmarks::MenuItemFolder; + return menuItem.type == bookmarks::MenuItemFolder; } - (MDCInkView*)inkTouchController:(MDCInkTouchController*)inkTouchController
diff --git a/ios/chrome/browser/ui/bookmarks/bookmark_position_cache.h b/ios/chrome/browser/ui/bookmarks/bookmark_position_cache.h index c999f970..1eaf1f6 100644 --- a/ios/chrome/browser/ui/bookmarks/bookmark_position_cache.h +++ b/ios/chrome/browser/ui/bookmarks/bookmark_position_cache.h
@@ -20,7 +20,6 @@ @property(nonatomic, assign, readonly) CGFloat position; @property(nonatomic, assign, readonly) bookmarks::MenuItemType type; -+ (BookmarkPositionCache*)cacheForMenuItemAllWithPosition:(CGFloat)position; + (BookmarkPositionCache*)cacheForMenuItemFolderWithPosition:(CGFloat)position folderId:(int64_t)folderId;
diff --git a/ios/chrome/browser/ui/bookmarks/bookmark_position_cache.mm b/ios/chrome/browser/ui/bookmarks/bookmark_position_cache.mm index 7d5e7f1..51fdd98 100644 --- a/ios/chrome/browser/ui/bookmarks/bookmark_position_cache.mm +++ b/ios/chrome/browser/ui/bookmarks/bookmark_position_cache.mm
@@ -43,13 +43,6 @@ #pragma mark - Public Constructors -+ (BookmarkPositionCache*)cacheForMenuItemAllWithPosition:(CGFloat)position { - return [[[BookmarkPositionCache alloc] - initWithFolderId:0 - position:position - type:bookmarks::MenuItemAll] autorelease]; -} - + (BookmarkPositionCache*)cacheForMenuItemFolderWithPosition:(CGFloat)position folderId:(int64_t)folderId { return [[[BookmarkPositionCache alloc] @@ -91,7 +84,6 @@ return NO; switch (self.type) { case bookmarks::MenuItemDivider: - case bookmarks::MenuItemAll: case bookmarks::MenuItemSectionHeader: return YES; case bookmarks::MenuItemFolder:
diff --git a/ios/chrome/browser/ui/bookmarks/bookmark_position_cache_unittest.mm b/ios/chrome/browser/ui/bookmarks/bookmark_position_cache_unittest.mm index 3085820..5c956e2 100644 --- a/ios/chrome/browser/ui/bookmarks/bookmark_position_cache_unittest.mm +++ b/ios/chrome/browser/ui/bookmarks/bookmark_position_cache_unittest.mm
@@ -8,16 +8,6 @@ namespace { -TEST(BookmarkPositionCacheTest, TestMenuItemAllCoding) { - BookmarkPositionCache* cache = - [BookmarkPositionCache cacheForMenuItemAllWithPosition:23.2]; - - NSData* data = [NSKeyedArchiver archivedDataWithRootObject:cache]; - BookmarkPositionCache* cache2 = - [NSKeyedUnarchiver unarchiveObjectWithData:data]; - EXPECT_NSEQ(cache, cache2); -} - TEST(BookmarkPositionCacheTest, TestMenuItemFolderCoding) { BookmarkPositionCache* cache = [BookmarkPositionCache cacheForMenuItemFolderWithPosition:1010101
diff --git a/ios/chrome/browser/ui/bookmarks/bookmark_utils_ios.mm b/ios/chrome/browser/ui/bookmarks/bookmark_utils_ios.mm index 4c6a27b..80b9d308 100644 --- a/ios/chrome/browser/ui/bookmarks/bookmark_utils_ios.mm +++ b/ios/chrome/browser/ui/bookmarks/bookmark_utils_ios.mm
@@ -626,9 +626,6 @@ cacheForMenuItemFolderWithPosition:position folderId:item.folder->id()]; break; - case bookmarks::MenuItemAll: - cache = [BookmarkPositionCache cacheForMenuItemAllWithPosition:position]; - break; case bookmarks::MenuItemDivider: case bookmarks::MenuItemSectionHeader: NOTREACHED(); @@ -659,11 +656,6 @@ return NO; switch (cache.type) { - case bookmarks::MenuItemAll: - if (!experimental_flags::IsAllBookmarksEnabled()) - return NO; - *item = [BookmarkMenuItem allMenuItem]; - break; case bookmarks::MenuItemFolder: { const BookmarkNode* bookmark = FindFolderById(model, cache.folderId); if (!bookmark)
diff --git a/ios/chrome/browser/ui/bookmarks/bookmark_utils_ios_unittest.mm b/ios/chrome/browser/ui/bookmarks/bookmark_utils_ios_unittest.mm index 841affa..d9ec4ef 100644 --- a/ios/chrome/browser/ui/bookmarks/bookmark_utils_ios_unittest.mm +++ b/ios/chrome/browser/ui/bookmarks/bookmark_utils_ios_unittest.mm
@@ -178,29 +178,17 @@ } TEST_F(BookmarkIOSUtilsUnitTest, TestPositionCache) { - // Try to store and retrieve a cache for the allMenuItem. - BookmarkMenuItem* item = [BookmarkMenuItem allMenuItem]; - CGFloat position = 23; - bookmark_utils_ios::CachePosition(position, item); - CGFloat outPosition; - BookmarkMenuItem* outItem; - BOOL result = bookmark_utils_ios::GetPositionCache(_bookmarkModel, &outItem, - &outPosition); - if (experimental_flags::IsAllBookmarksEnabled()) { - ASSERT_TRUE(result); - EXPECT_NSEQ(item, outItem); - EXPECT_NEAR(position, outPosition, 0.01); - } else { - ASSERT_FALSE(result); - } - // Try to store and retrieve a cache for the folderMenuItem. const BookmarkNode* mobileNode = _bookmarkModel->mobile_node(); const BookmarkNode* f1 = AddFolder(mobileNode, @"f1"); - item = [BookmarkMenuItem folderMenuItemForNode:f1 rootAncestor:NULL]; + BookmarkMenuItem* item = + [BookmarkMenuItem folderMenuItemForNode:f1 rootAncestor:NULL]; + CGFloat position = 23; bookmark_utils_ios::CachePosition(position, item); - result = bookmark_utils_ios::GetPositionCache(_bookmarkModel, &outItem, - &outPosition); + BookmarkMenuItem* outItem = nil; + CGFloat outPosition; + BOOL result = bookmark_utils_ios::GetPositionCache(_bookmarkModel, &outItem, + &outPosition); ASSERT_TRUE(result); EXPECT_NSEQ(item, outItem); EXPECT_NEAR(position, outPosition, 0.01);
diff --git a/ios/chrome/browser/ui/bookmarks/bookmarks_egtest.mm b/ios/chrome/browser/ui/bookmarks/bookmarks_egtest.mm index f39f7ad..64d09643 100644 --- a/ios/chrome/browser/ui/bookmarks/bookmarks_egtest.mm +++ b/ios/chrome/browser/ui/bookmarks/bookmarks_egtest.mm
@@ -1005,11 +1005,7 @@ [[self class] removeBookmarkWithTitle:@"Folder 1"]; NSString* rootFolderTitle = nil; - if (experimental_flags::IsAllBookmarksEnabled()) { - rootFolderTitle = @"Bookmarks"; - } else { - rootFolderTitle = @"Mobile Bookmarks"; - } + rootFolderTitle = @"Mobile Bookmarks"; // Folder 2 and 3 are now deleted, UI should have moved to top level folder. [[EarlGrey @@ -1028,10 +1024,6 @@ // Test that the root folder is selected in the menu. This is only the case // on iPhone. - if (experimental_flags::IsAllBookmarksEnabled()) { - rootFolderTitle = @"All Bookmarks"; - } - GREYElementMatcherBlock* selectedMatcher = [GREYElementMatcherBlock matcherWithMatchesBlock:^BOOL(id element) { UITableViewCell* cell = (UITableViewCell*)element; @@ -1301,11 +1293,7 @@ // Navigates to the bookmark manager UI, and selects the top level folder. + (void)openTopLevelBookmarksFolder { - if (experimental_flags::IsAllBookmarksEnabled()) { - [BookmarksTestCase openBookmarkFolder:@"All Bookmarks"]; - } else { - [BookmarksTestCase openMobileBookmarks]; - } + [BookmarksTestCase openMobileBookmarks]; } // Navigates to the bookmark manager UI, and selects MobileBookmarks.
diff --git a/ios/chrome/browser/ui/bookmarks/resources/bookmark_blue_star.png b/ios/chrome/browser/ui/bookmarks/resources/bookmark_blue_star.png deleted file mode 100644 index c50e4bf..0000000 --- a/ios/chrome/browser/ui/bookmarks/resources/bookmark_blue_star.png +++ /dev/null Binary files differ
diff --git a/ios/chrome/browser/ui/bookmarks/resources/bookmark_blue_star@2x.png b/ios/chrome/browser/ui/bookmarks/resources/bookmark_blue_star@2x.png deleted file mode 100644 index f64258e..0000000 --- a/ios/chrome/browser/ui/bookmarks/resources/bookmark_blue_star@2x.png +++ /dev/null Binary files differ
diff --git a/ios/chrome/browser/ui/bookmarks/resources/bookmark_blue_star@3x.png b/ios/chrome/browser/ui/bookmarks/resources/bookmark_blue_star@3x.png deleted file mode 100644 index aaee784..0000000 --- a/ios/chrome/browser/ui/bookmarks/resources/bookmark_blue_star@3x.png +++ /dev/null Binary files differ
diff --git a/ios/chrome/browser/ui/bookmarks/resources/bookmark_gray_star.png b/ios/chrome/browser/ui/bookmarks/resources/bookmark_gray_star.png deleted file mode 100644 index b623b08d1..0000000 --- a/ios/chrome/browser/ui/bookmarks/resources/bookmark_gray_star.png +++ /dev/null Binary files differ
diff --git a/ios/chrome/browser/ui/bookmarks/resources/bookmark_gray_star@2x.png b/ios/chrome/browser/ui/bookmarks/resources/bookmark_gray_star@2x.png deleted file mode 100644 index e295f56..0000000 --- a/ios/chrome/browser/ui/bookmarks/resources/bookmark_gray_star@2x.png +++ /dev/null Binary files differ
diff --git a/ios/chrome/browser/ui/bookmarks/resources/bookmark_gray_star@3x.png b/ios/chrome/browser/ui/bookmarks/resources/bookmark_gray_star@3x.png deleted file mode 100644 index 993dfd2..0000000 --- a/ios/chrome/browser/ui/bookmarks/resources/bookmark_gray_star@3x.png +++ /dev/null Binary files differ
diff --git a/ios/chrome/browser/ui/browser_view_controller.mm b/ios/chrome/browser/ui/browser_view_controller.mm index d00a200..f662304 100644 --- a/ios/chrome/browser/ui/browser_view_controller.mm +++ b/ios/chrome/browser/ui/browser_view_controller.mm
@@ -4400,6 +4400,7 @@ _contentSuggestionsCoordinator.reset([[ContentSuggestionsCoordinator alloc] initWithBaseViewController:self]); } + [_contentSuggestionsCoordinator setBrowserState:_browserState]; [_contentSuggestionsCoordinator start]; }
diff --git a/ios/chrome/browser/ui/content_suggestions/BUILD.gn b/ios/chrome/browser/ui/content_suggestions/BUILD.gn index c35d965..0479d04 100644 --- a/ios/chrome/browser/ui/content_suggestions/BUILD.gn +++ b/ios/chrome/browser/ui/content_suggestions/BUILD.gn
@@ -5,11 +5,15 @@ source_set("content_suggestions") { configs += [ "//build/config/compiler:enable_arc" ] sources = [ + "content_suggestion.h", + "content_suggestion.mm", "content_suggestions_article_item.h", "content_suggestions_article_item.mm", "content_suggestions_collection_updater.h", "content_suggestions_collection_updater.mm", "content_suggestions_commands.h", + "content_suggestions_data_sink.h", + "content_suggestions_data_source.h", "content_suggestions_expandable_item.h", "content_suggestions_expandable_item.mm", "content_suggestions_favicon_internal_cell.h",
diff --git a/ios/chrome/browser/ui/content_suggestions/content_suggestion.h b/ios/chrome/browser/ui/content_suggestions/content_suggestion.h new file mode 100644 index 0000000..68bcf1b --- /dev/null +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestion.h
@@ -0,0 +1,18 @@ +// 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 IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTION_H_ +#define IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTION_H_ + +#import <UIKit/UIKit.h> + +// Data for a suggestions item, compatible with Objective-C. +@interface ContentSuggestion : NSObject + +@property(nonatomic, copy) NSString* title; +@property(nonatomic, strong) UIImage* image; + +@end + +#endif // IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTION_H_
diff --git a/ios/chrome/browser/ui/content_suggestions/content_suggestion.mm b/ios/chrome/browser/ui/content_suggestions/content_suggestion.mm new file mode 100644 index 0000000..0d25178 --- /dev/null +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestion.mm
@@ -0,0 +1,16 @@ +// 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. + +#import "ios/chrome/browser/ui/content_suggestions/content_suggestion.h" + +#if !defined(__has_feature) || !__has_feature(objc_arc) +#error "This file requires ARC support." +#endif + +@implementation ContentSuggestion + +@synthesize title = _title; +@synthesize image = _image; + +@end
diff --git a/ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.h b/ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.h index b453f17..29615fd 100644 --- a/ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.h +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.h
@@ -9,6 +9,7 @@ #import "ios/chrome/browser/ui/collection_view/collection_view_model.h" +@protocol ContentSuggestionsDataSource; @class ContentSuggestionsViewController; // Enum defining the ItemType of this ContentSuggestionsCollectionUpdater. @@ -24,6 +25,12 @@ // handling the items addition. @interface ContentSuggestionsCollectionUpdater : NSObject +// Initialize with the |dataSource| used to get the data. +- (instancetype)initWithDataSource:(id<ContentSuggestionsDataSource>)dataSource + NS_DESIGNATED_INITIALIZER; + +- (instancetype)init NS_UNAVAILABLE; + // |collectionViewController| this Updater will update. Needs to be set before // adding items. @property(nonatomic, assign)
diff --git a/ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.mm b/ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.mm index 6ba3bf0d..68c32ab 100644 --- a/ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.mm +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestions_collection_updater.mm
@@ -9,6 +9,8 @@ #import "ios/chrome/browser/ui/collection_view/collection_view_controller.h" #import "ios/chrome/browser/ui/collection_view/collection_view_model.h" #import "ios/chrome/browser/ui/content_suggestions/content_suggestions_article_item.h" +#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_data_sink.h" +#import "ios/chrome/browser/ui/content_suggestions/content_suggestions_data_source.h" #import "ios/chrome/browser/ui/content_suggestions/content_suggestions_expandable_item.h" #import "ios/chrome/browser/ui/content_suggestions/content_suggestions_favicon_item.h" #import "ios/chrome/browser/ui/content_suggestions/content_suggestions_item.h" @@ -19,9 +21,26 @@ #error "This file requires ARC support." #endif +@interface ContentSuggestionsCollectionUpdater ()<ContentSuggestionsDataSink> + +@property(nonatomic, weak) id<ContentSuggestionsDataSource> dataSource; + +@end + @implementation ContentSuggestionsCollectionUpdater @synthesize collectionViewController = _collectionViewController; +@synthesize dataSource = _dataSource; + +- (instancetype)initWithDataSource: + (id<ContentSuggestionsDataSource>)dataSource { + self = [super init]; + if (self) { + _dataSource = dataSource; + _dataSource.dataSink = self; + } + return self; +} #pragma mark - Properties @@ -30,6 +49,10 @@ _collectionViewController = collectionViewController; [collectionViewController loadModel]; CollectionViewModel* model = collectionViewController.collectionViewModel; + + // TODO(crbug.com/686728): Load the data with the dataSource instead of hard + // coded value. + NSInteger sectionIdentifier = kSectionIdentifierEnumZero; // Stack Item. @@ -88,6 +111,12 @@ } } +#pragma mark - ContentSuggestionsDataSink + +- (void)dataAvailable { + // TODO(crbug.com/686728): Get the new data from the DataSource. +} + #pragma mark - Public methods - (void)addTextItem:(NSString*)title
diff --git a/ios/chrome/browser/ui/content_suggestions/content_suggestions_data_sink.h b/ios/chrome/browser/ui/content_suggestions/content_suggestions_data_sink.h new file mode 100644 index 0000000..ebf8bfa --- /dev/null +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestions_data_sink.h
@@ -0,0 +1,17 @@ +// 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 IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_DATA_SINK_H_ +#define IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_DATA_SINK_H_ + +// Data sink for the ContentSuggestions. It will be notified when new data needs +// to be pulled. +@protocol ContentSuggestionsDataSink + +// Notifies the Data Sink that new data is available. +- (void)dataAvailable; + +@end + +#endif // IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_DATA_SINK_H_
diff --git a/ios/chrome/browser/ui/content_suggestions/content_suggestions_data_source.h b/ios/chrome/browser/ui/content_suggestions/content_suggestions_data_source.h new file mode 100644 index 0000000..39eff1cc --- /dev/null +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestions_data_source.h
@@ -0,0 +1,24 @@ +// 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 IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_DATA_SOURCE_H_ +#define IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_DATA_SOURCE_H_ + +@class ContentSuggestion; +@protocol ContentSuggestionsDataSink; + +// DataSource for the content suggestions. Provides the suggestions data in a +// format compatible with Objective-C. +@protocol ContentSuggestionsDataSource + +// The data sink that will be notified when the data change. +@property(nonatomic, weak) id<ContentSuggestionsDataSink> dataSink; + +// Returns all the data currently available. Returns an empty array if nothing +// is available. +- (NSArray<ContentSuggestion*>*)allSuggestions; + +@end + +#endif // IOS_CHROME_BROWSER_UI_CONTENT_SUGGESTIONS_CONTENT_SUGGESTIONS_DATA_SOURCE_H_
diff --git a/ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.h b/ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.h index df6a46d..b078851e 100644 --- a/ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.h +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.h
@@ -12,12 +12,20 @@ #import "ios/chrome/browser/ui/content_suggestions/content_suggestions_favicon_item.h" @protocol ContentSuggestionsCommands; +@protocol ContentSuggestionsDataSource; // CollectionViewController to display the suggestions items. @interface ContentSuggestionsViewController : CollectionViewController<ContentSuggestionsExpandableCellDelegate, ContentSuggestionsFaviconCellDelegate> +- (instancetype)initWithStyle:(CollectionViewControllerStyle)style + dataSource:(id<ContentSuggestionsDataSource>)dataSource + NS_DESIGNATED_INITIALIZER; + +- (instancetype)initWithStyle:(CollectionViewControllerStyle)style + NS_UNAVAILABLE; + // Handler for the commands sent by the ContentSuggestionsViewController. @property(nonatomic, weak) id<ContentSuggestionsCommands> suggestionCommandHandler;
diff --git a/ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.mm b/ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.mm index 3b82cd53..f7d7440 100644 --- a/ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.mm +++ b/ios/chrome/browser/ui/content_suggestions/content_suggestions_view_controller.mm
@@ -40,12 +40,21 @@ @synthesize suggestionCommandHandler = _suggestionCommandHandler; @synthesize collectionUpdater = _collectionUpdater; +- (instancetype)initWithStyle:(CollectionViewControllerStyle)style + dataSource:(id<ContentSuggestionsDataSource>)dataSource { + self = [super initWithStyle:style]; + if (self) { + _collectionUpdater = [[ContentSuggestionsCollectionUpdater alloc] + initWithDataSource:dataSource]; + } + return self; +} + #pragma mark - UIViewController - (void)viewDidLoad { [super viewDidLoad]; - _collectionUpdater = [[ContentSuggestionsCollectionUpdater alloc] init]; _collectionUpdater.collectionViewController = self; self.collectionView.delegate = self;
diff --git a/ios/public/provider/chrome/browser/native_app_launcher/fake_native_app_whitelist_manager.h b/ios/public/provider/chrome/browser/native_app_launcher/fake_native_app_whitelist_manager.h index 9ea2dcb..61fc53ef 100644 --- a/ios/public/provider/chrome/browser/native_app_launcher/fake_native_app_whitelist_manager.h +++ b/ios/public/provider/chrome/browser/native_app_launcher/fake_native_app_whitelist_manager.h
@@ -12,7 +12,7 @@ // Fake NativeAppWhitelistManager used for testing purposes. @interface FakeNativeAppWhitelistManager : NSObject<NativeAppWhitelistManager> -// The metadata returned by calls to |newNativeAppForURL:|. +// The metadata returned by calls to |nativeAppForURL:|. @property(nonatomic, strong, readwrite) id<NativeAppMetadata> metadata; // The Apps array returned by calls to |filteredAppsUsingBlock:|.
diff --git a/ios/public/provider/chrome/browser/native_app_launcher/fake_native_app_whitelist_manager.mm b/ios/public/provider/chrome/browser/native_app_launcher/fake_native_app_whitelist_manager.mm index 5f552b7..eaabe3d 100644 --- a/ios/public/provider/chrome/browser/native_app_launcher/fake_native_app_whitelist_manager.mm +++ b/ios/public/provider/chrome/browser/native_app_launcher/fake_native_app_whitelist_manager.mm
@@ -23,7 +23,7 @@ _appWhitelist = appList; } -- (id<NativeAppMetadata>)newNativeAppForURL:(const GURL&)url { +- (id<NativeAppMetadata>)nativeAppForURL:(const GURL&)url { return _metadata; }
diff --git a/ios/public/provider/chrome/browser/native_app_launcher/native_app_whitelist_manager.h b/ios/public/provider/chrome/browser/native_app_launcher/native_app_whitelist_manager.h index 071ffb9..e846741 100644 --- a/ios/public/provider/chrome/browser/native_app_launcher/native_app_whitelist_manager.h +++ b/ios/public/provider/chrome/browser/native_app_launcher/native_app_whitelist_manager.h
@@ -23,10 +23,9 @@ tldList:(NSArray*)tldList acceptStoreIDs:(NSArray*)storeIDs; -// Returns a new object with the metadata about the iOS native application that -// can handle |url|. Returns nil otherwise. Caller owns the returned object -// and needs to release it when done. -- (id<NativeAppMetadata>)newNativeAppForURL:(const GURL&)url; +// Returns an object with the metadata about the iOS native application that +// can handle |url|. Returns nil otherwise. +- (id<NativeAppMetadata>)nativeAppForURL:(const GURL&)url; // Returns an autoreleased NSArray of NativeAppMetadata objects which // |condition| block returns YES.
diff --git a/ios/showcase/suggestions/sc_suggestions_coordinator.mm b/ios/showcase/suggestions/sc_suggestions_coordinator.mm index e014083..332795c 100644 --- a/ios/showcase/suggestions/sc_suggestions_coordinator.mm +++ b/ios/showcase/suggestions/sc_suggestions_coordinator.mm
@@ -34,7 +34,8 @@ self.alerter.baseViewController = self.baseViewController; _suggestionViewController = [[ContentSuggestionsViewController alloc] - initWithStyle:CollectionViewControllerStyleDefault]; + initWithStyle:CollectionViewControllerStyleDefault + dataSource:nil]; _suggestionViewController.suggestionCommandHandler = reinterpret_cast<id<ContentSuggestionsCommands>>(self.alerter);
diff --git a/ios/web/net/request_tracker_impl.mm b/ios/web/net/request_tracker_impl.mm index b6ac91c..7ffccb9 100644 --- a/ios/web/net/request_tracker_impl.mm +++ b/ios/web/net/request_tracker_impl.mm
@@ -416,6 +416,16 @@ // static void RequestTrackerImpl::BlockUntilTrackersShutdown() { DCHECK_CURRENTLY_ON(web::WebThread::UI); + // Initialize the globals as part of the shutdown to prevent a crash when + // trying to acquire the lock if it was never initialised. It can happen + // if the application is terminated when no RequestTracker has been created + // (which can happen if no UIWebView has been created). See crbug.com/684611 + // for information on such a crash. + // + // As RequestTracker are deprecated and are only used in Sign-In workflow + // it is simpler to just do the initialisation here than tracking whether + // the method should be called or not by client code. + pthread_once(&g_once_control, &InitializeGlobals); { base::AutoLock scoped_lock(*g_waiting_on_io_thread_lock); g_waiting_on_io_thread = true;
diff --git a/ios/web/shell/test/context_menu_egtest.mm b/ios/web/shell/test/context_menu_egtest.mm index bb8fbfbf..ddd7895 100644 --- a/ios/web/shell/test/context_menu_egtest.mm +++ b/ios/web/shell/test/context_menu_egtest.mm
@@ -9,6 +9,7 @@ #import "base/ios/block_types.h" #include "base/strings/sys_string_conversions.h" +#import "ios/testing/earl_grey/disabled_test_macros.h" #import "ios/testing/earl_grey/matchers.h" #import "ios/web/public/test/http_server.h" #include "ios/web/public/test/http_server_util.h" @@ -35,6 +36,11 @@ // Tests context menu appears on a regular link. - (void)testContextMenu { +// TODO(crbug.com/687546): Tests disabled on devices. +#if !TARGET_IPHONE_SIMULATOR + EARL_GREY_TEST_DISABLED(@"Disabled for devices because it is very flaky."); +#endif + // Create map of canned responses and set up the test HTML server. std::map<GURL, std::string> responses; GURL initialURL = web::test::HttpServer::MakeUrl("http://contextMenuOpen");
diff --git a/sandbox/linux/seccomp-bpf-helpers/syscall_sets.cc b/sandbox/linux/seccomp-bpf-helpers/syscall_sets.cc index b30b3e6ac..1d9f95cd 100644 --- a/sandbox/linux/seccomp-bpf-helpers/syscall_sets.cc +++ b/sandbox/linux/seccomp-bpf-helpers/syscall_sets.cc
@@ -120,9 +120,7 @@ #if defined(__i386__) || defined(__arm__) || defined(__mips__) case __NR_lstat64: #endif -#if defined(__i386__) || defined(__arm__) || defined(__x86_64__) case __NR_memfd_create: -#endif case __NR_mkdirat: case __NR_mknodat: #if defined(__i386__)
diff --git a/sandbox/linux/system_headers/arm64_linux_syscalls.h b/sandbox/linux/system_headers/arm64_linux_syscalls.h index 8acb2d1..59d0eab8 100644 --- a/sandbox/linux/system_headers/arm64_linux_syscalls.h +++ b/sandbox/linux/system_headers/arm64_linux_syscalls.h
@@ -1059,4 +1059,8 @@ #define __NR_getrandom 278 #endif +#if !defined(__NR_memfd_create) +#define __NR_memfd_create 279 +#endif + #endif // SANDBOX_LINUX_SYSTEM_HEADERS_ARM64_LINUX_SYSCALLS_H_
diff --git a/sandbox/linux/system_headers/mips64_linux_syscalls.h b/sandbox/linux/system_headers/mips64_linux_syscalls.h index 5a179b0..90f3d1be 100644 --- a/sandbox/linux/system_headers/mips64_linux_syscalls.h +++ b/sandbox/linux/system_headers/mips64_linux_syscalls.h
@@ -1267,4 +1267,8 @@ #define __NR_getrandom (__NR_Linux + 313) #endif +#if !defined(__NR_memfd_create) +#define __NR_memfd_create (__NR_Linux + 314) +#endif + #endif // SANDBOX_LINUX_SYSTEM_HEADERS_MIPS64_LINUX_SYSCALLS_H_
diff --git a/sandbox/linux/system_headers/mips_linux_syscalls.h b/sandbox/linux/system_headers/mips_linux_syscalls.h index 819f9eb..784d6b8 100644 --- a/sandbox/linux/system_headers/mips_linux_syscalls.h +++ b/sandbox/linux/system_headers/mips_linux_syscalls.h
@@ -1429,4 +1429,8 @@ #define __NR_getrandom (__NR_Linux + 353) #endif +#if !defined(__NR_memfd_create) +#define __NR_memfd_create (__NR_Linux + 354) +#endif + #endif // SANDBOX_LINUX_SYSTEM_HEADERS_MIPS_LINUX_SYSCALLS_H_
diff --git a/skia/ext/benchmarking_canvas.cc b/skia/ext/benchmarking_canvas.cc index d66614a..3e3602d 100644 --- a/skia/ext/benchmarking_canvas.cc +++ b/skia/ext/benchmarking_canvas.cc
@@ -121,7 +121,7 @@ std::unique_ptr<base::StringValue> val( new base::StringValue(SkBlendMode_Name(mode))); - return std::move(val); + return val; } std::unique_ptr<base::Value> AsValue(SkCanvas::PointMode mode) { @@ -131,7 +131,7 @@ std::unique_ptr<base::StringValue> val( new base::StringValue(gModeStrings[mode])); - return std::move(val); + return val; } std::unique_ptr<base::Value> AsValue(const SkColorFilter& filter) { @@ -261,7 +261,7 @@ std::unique_ptr<base::StringValue> val(new base::StringValue(builder.str())); - return std::move(val); + return val; } std::unique_ptr<base::Value> AsValue(SkClipOp op) { @@ -276,7 +276,7 @@ DCHECK_LT(index, SK_ARRAY_COUNT(gOpStrings)); std::unique_ptr<base::StringValue> val( new base::StringValue(gOpStrings[index])); - return std::move(val); + return val; } std::unique_ptr<base::Value> AsValue(const SkRegion& region) {
diff --git a/third_party/WebKit/LayoutTests/TestExpectations b/third_party/WebKit/LayoutTests/TestExpectations index de9a97cd2..f2659351 100644 --- a/third_party/WebKit/LayoutTests/TestExpectations +++ b/third_party/WebKit/LayoutTests/TestExpectations
@@ -2378,6 +2378,8 @@ crbug.com/678346 [ Win7 Debug ] storage/indexeddb/mozilla/test_objectStore_openKeyCursor.html [ Pass Timeout ] crbug.com/678346 [ Win7 Debug ] storage/indexeddb/structured-clone.html [ Pass Timeout ] +crbug.com/685456 [ Win Mac ] fast/media/mq-color-gamut-picture.html [ Failure Pass ] + crbug.com/678487 http/tests/inspector/resource-tree/resource-tree-reload.html [ Failure Timeout Pass ] crbug.com/678487 virtual/mojo-loading/http/tests/inspector/resource-tree/resource-tree-reload.html [ Failure Timeout Pass ] crbug.com/678488 http/tests/inspector/search/source-frame-replace-2.html [ Timeout Pass ]
diff --git a/third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc b/third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc index 0c9669cc..f7e50974 100644 --- a/third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc +++ b/third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc
@@ -282,12 +282,12 @@ // TODO: handle floats & orthogonal children. for (NGBlockNode* node = first_child_; node; node = node->NextSibling()) { Optional<MinAndMaxContentSizes> child_minmax; - if (NeedMinAndMaxContentSizesForContentContribution(*node->Style())) { + if (NeedMinAndMaxContentSizesForContentContribution(node->Style())) { child_minmax = node->ComputeMinAndMaxContentSizes(); } MinAndMaxContentSizes child_sizes = - ComputeMinAndMaxContentContribution(*node->Style(), child_minmax); + ComputeMinAndMaxContentContribution(node->Style(), child_minmax); sizes->min_content = std::max(sizes->min_content, child_sizes.min_content); sizes->max_content = std::max(sizes->max_content, child_sizes.max_content); @@ -380,7 +380,7 @@ curr_bfc_offset_.block_offset += content_size_; while (current_child_) { - EPosition position = current_child_->Style()->position(); + EPosition position = current_child_->Style().position(); if (position == AbsolutePosition || position == FixedPosition) { builder_->AddOutOfFlowChildCandidate(current_child_, GetChildSpaceOffset());
diff --git a/third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.h b/third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.h index 932a888..daf2b69 100644 --- a/third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.h +++ b/third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.h
@@ -108,7 +108,7 @@ // Read-only Getters. const ComputedStyle& CurrentChildStyle() const { DCHECK(current_child_); - return *current_child_->Style(); + return current_child_->Style(); } const NGConstraintSpace& ConstraintSpace() const {
diff --git a/third_party/WebKit/Source/core/layout/ng/ng_block_node.cc b/third_party/WebKit/Source/core/layout/ng/ng_block_node.cc index 6bc68e3..e531bfd 100644 --- a/third_party/WebKit/Source/core/layout/ng/ng_block_node.cc +++ b/third_party/WebKit/Source/core/layout/ng/ng_block_node.cc
@@ -116,8 +116,8 @@ NGConstraintSpace* constraint_space = NGConstraintSpaceBuilder( - FromPlatformWritingMode(Style()->getWritingMode())) - .SetTextDirection(Style()->direction()) + FromPlatformWritingMode(Style().getWritingMode())) + .SetTextDirection(Style().direction()) .ToConstraintSpace(); // TODO(cbiesinger): For orthogonal children, we need to always synthesize. @@ -126,7 +126,7 @@ NGLayoutInputNode* first_child = FirstChild(); if (!first_child || first_child->Type() == kLegacyBlock) { NGBlockLayoutAlgorithm minmax_algorithm( - layout_box_, Style(), toNGBlockNode(FirstChild()), constraint_space); + layout_box_, &Style(), toNGBlockNode(FirstChild()), constraint_space); if (minmax_algorithm.ComputeMinAndMaxContentSizes(&sizes)) return sizes; } @@ -134,7 +134,7 @@ // Have to synthesize this value. NGPhysicalFragment* physical_fragment = Layout(constraint_space); NGBoxFragment* fragment = new NGBoxFragment( - FromPlatformWritingMode(Style()->getWritingMode()), Style()->direction(), + FromPlatformWritingMode(Style().getWritingMode()), Style().direction(), toNGPhysicalBoxFragment(physical_fragment)); sizes.min_content = fragment->InlineOverflow(); @@ -142,32 +142,25 @@ // Now, redo with infinite space for max_content constraint_space = NGConstraintSpaceBuilder( - FromPlatformWritingMode(Style()->getWritingMode())) - .SetTextDirection(Style()->direction()) + FromPlatformWritingMode(Style().getWritingMode())) + .SetTextDirection(Style().direction()) .SetAvailableSize({LayoutUnit::max(), LayoutUnit()}) .SetPercentageResolutionSize({LayoutUnit(), LayoutUnit()}) .ToConstraintSpace(); physical_fragment = Layout(constraint_space); fragment = new NGBoxFragment( - FromPlatformWritingMode(Style()->getWritingMode()), Style()->direction(), + FromPlatformWritingMode(Style().getWritingMode()), Style().direction(), toNGPhysicalBoxFragment(physical_fragment)); sizes.max_content = fragment->InlineOverflow(); return sizes; } -ComputedStyle* NGBlockNode::MutableStyle() { +const ComputedStyle& NGBlockNode::Style() const { if (style_) - return style_.get(); + return *style_.get(); DCHECK(layout_box_); - return layout_box_->mutableStyle(); -} - -const ComputedStyle* NGBlockNode::Style() const { - if (style_) - return style_.get(); - DCHECK(layout_box_); - return layout_box_->style(); + return layout_box_->styleRef(); } NGBlockNode* NGBlockNode::NextSibling() { @@ -189,7 +182,7 @@ LayoutObject* child = layout_box_ ? layout_box_->slowFirstChild() : nullptr; if (child) { if (child->isInline()) { - SetFirstChild(new NGInlineNode(child, MutableStyle())); + SetFirstChild(new NGInlineNode(child, &Style())); } else { SetFirstChild(new NGBlockNode(child)); } @@ -252,7 +245,7 @@ layout_box_->setWidth(fragment_->Width()); layout_box_->setHeight(fragment_->Height()); NGBoxStrut border_and_padding = - ComputeBorders(*Style()) + ComputePadding(constraint_space, *Style()); + ComputeBorders(Style()) + ComputePadding(constraint_space, Style()); LayoutUnit intrinsic_logical_height = layout_box_->style()->isHorizontalWritingMode() ? fragment_->HeightOverflow()
diff --git a/third_party/WebKit/Source/core/layout/ng/ng_block_node.h b/third_party/WebKit/Source/core/layout/ng/ng_block_node.h index 28d3574..ce67407 100644 --- a/third_party/WebKit/Source/core/layout/ng/ng_block_node.h +++ b/third_party/WebKit/Source/core/layout/ng/ng_block_node.h
@@ -43,8 +43,7 @@ // using Layout with special constraint spaces. MinAndMaxContentSizes ComputeMinAndMaxContentSizes(); - const ComputedStyle* Style() const; - ComputedStyle* MutableStyle(); + const ComputedStyle& Style() const; NGLayoutInputNode* FirstChild();
diff --git a/third_party/WebKit/Source/core/layout/ng/ng_layout_input_node.cc b/third_party/WebKit/Source/core/layout/ng/ng_layout_input_node.cc index 89996db..dcb0e27 100644 --- a/third_party/WebKit/Source/core/layout/ng/ng_layout_input_node.cc +++ b/third_party/WebKit/Source/core/layout/ng/ng_layout_input_node.cc
@@ -25,18 +25,18 @@ NGBlockNode* block = toNGBlockNode(input_node); if (!block->CanUseNewLayout()) return new NGLegacyBlockLayoutAlgorithm(block, constraint_space); - const ComputedStyle* style = block->Style(); + const ComputedStyle& style = block->Style(); LayoutObject* layout_object = input_node->GetLayoutObject(); if (block->HasInlineChildren()) { NGInlineNode* child = toNGInlineNode(block->FirstChild()); - return new NGInlineLayoutAlgorithm(layout_object, style, child, + return new NGInlineLayoutAlgorithm(layout_object, &style, child, constraint_space); } NGBlockNode* child = toNGBlockNode(block->FirstChild()); // TODO(layout-ng): The break token should be passed as an argument to this // method instead of getting it from the NGBlockNode NGBreakToken* token = block->CurrentBreakToken(); - return new NGBlockLayoutAlgorithm(layout_object, style, child, + return new NGBlockLayoutAlgorithm(layout_object, &style, child, constraint_space, token); } }
diff --git a/third_party/WebKit/Source/core/layout/ng/ng_out_of_flow_layout_part.cc b/third_party/WebKit/Source/core/layout/ng/ng_out_of_flow_layout_part.cc index cdbda4f8..06d90c47 100644 --- a/third_party/WebKit/Source/core/layout/ng/ng_out_of_flow_layout_part.cc +++ b/third_party/WebKit/Source/core/layout/ng/ng_out_of_flow_layout_part.cc
@@ -75,7 +75,7 @@ out_of_flow_candidate_positions[position_index++]; if (IsContainingBlockForAbsoluteDescendant(container_style_, - *descendant->Style())) { + descendant->Style())) { NGLogicalOffset offset; NGFragment* fragment = LayoutDescendant(*descendant, static_position, &offset); @@ -100,21 +100,21 @@ Optional<MinAndMaxContentSizes> inline_estimate; Optional<LayoutUnit> block_estimate; - if (AbsoluteNeedsChildInlineSize(*descendant.Style())) { + if (AbsoluteNeedsChildInlineSize(descendant.Style())) { inline_estimate = descendant.ComputeMinAndMaxContentSizes(); } NGAbsolutePhysicalPosition node_position = ComputePartialAbsoluteWithChildInlineSize( - *container_space_, *descendant.Style(), static_position, + *container_space_, descendant.Style(), static_position, inline_estimate); - if (AbsoluteNeedsChildBlockSize(*descendant.Style())) { + if (AbsoluteNeedsChildBlockSize(descendant.Style())) { fragment = GenerateFragment(descendant, block_estimate, node_position); block_estimate = fragment->BlockSize(); } - ComputeFullAbsoluteWithChildBlockSize(*container_space_, *descendant.Style(), + ComputeFullAbsoluteWithChildBlockSize(*container_space_, descendant.Style(), static_position, block_estimate, &node_position);
diff --git a/third_party/WebKit/Source/web/tests/ScrollingCoordinatorTest.cpp b/third_party/WebKit/Source/web/tests/ScrollingCoordinatorTest.cpp index 43055af..85f01cb 100644 --- a/third_party/WebKit/Source/web/tests/ScrollingCoordinatorTest.cpp +++ b/third_party/WebKit/Source/web/tests/ScrollingCoordinatorTest.cpp
@@ -38,7 +38,6 @@ #include "platform/geometry/IntPoint.h" #include "platform/geometry/IntRect.h" #include "platform/graphics/GraphicsLayer.h" -#include "platform/testing/RuntimeEnabledFeaturesTestHelpers.h" #include "platform/testing/URLTestHelpers.h" #include "public/platform/Platform.h" #include "public/platform/WebLayer.h" @@ -55,13 +54,9 @@ namespace blink { -class ScrollingCoordinatorTest : public testing::Test, - public testing::WithParamInterface<bool>, - private ScopedRootLayerScrollingForTest { +class ScrollingCoordinatorTest : public testing::Test { public: - ScrollingCoordinatorTest() - : ScopedRootLayerScrollingForTest(GetParam()), - m_baseURL("http://www.test.com/") { + ScrollingCoordinatorTest() : m_baseURL("http://www.test.com/") { m_helper.initialize(true, nullptr, &m_mockWebViewClient, nullptr, &configureSettings); webViewImpl()->resize(IntSize(320, 240)); @@ -89,11 +84,6 @@ FrameTestHelpers::loadFrame(webViewImpl()->mainFrame(), url); } - void loadHTML(const std::string& html) { - FrameTestHelpers::loadHTMLString(webViewImpl()->mainFrame(), html, - URLTestHelpers::toKURL("about:blank")); - } - void forceFullCompositingUpdate() { webViewImpl()->updateAllLifecyclePhases(); } @@ -105,9 +95,13 @@ } WebLayer* getRootScrollLayer() { - GraphicsLayer* layer = - frame()->view()->layoutViewportScrollableArea()->layerForScrolling(); - return layer ? layer->platformLayer() : nullptr; + PaintLayerCompositor* compositor = + frame()->contentLayoutItem().compositor(); + DCHECK(compositor); + DCHECK(compositor->scrollLayer()); + + WebLayer* webScrollLayer = compositor->scrollLayer()->platformLayer(); + return webScrollLayer; } WebViewImpl* webViewImpl() const { return m_helper.webView(); } @@ -119,6 +113,9 @@ return webViewImpl()->layerTreeView(); } + void styleRelatedMainThreadScrollingReasonTest(const std::string&, + const uint32_t); + protected: std::string m_baseURL; FrameTestHelpers::TestWebViewClient m_mockWebViewClient; @@ -133,11 +130,8 @@ FrameTestHelpers::WebViewHelper m_helper; }; -INSTANTIATE_TEST_CASE_P(All, ScrollingCoordinatorTest, ::testing::Bool()); - -TEST_P(ScrollingCoordinatorTest, fastScrollingByDefault) { - webViewImpl()->resize(WebSize(800, 600)); - loadHTML("<div id='spacer' style='height: 1000px'></div>"); +TEST_F(ScrollingCoordinatorTest, fastScrollingByDefault) { + navigateTo("about:blank"); forceFullCompositingUpdate(); // Make sure the scrolling coordinator is active. @@ -149,7 +143,6 @@ // Fast scrolling should be enabled by default. WebLayer* rootScrollLayer = getRootScrollLayer(); - ASSERT_TRUE(rootScrollLayer); ASSERT_TRUE(rootScrollLayer->scrollable()); ASSERT_FALSE(rootScrollLayer->shouldScrollOnMainThread()); ASSERT_EQ(WebEventListenerProperties::Nothing, @@ -165,9 +158,8 @@ ASSERT_FALSE(innerViewportScrollLayer->shouldScrollOnMainThread()); } -TEST_P(ScrollingCoordinatorTest, fastScrollingCanBeDisabledWithSetting) { - webViewImpl()->resize(WebSize(800, 600)); - loadHTML("<div id='spacer' style='height: 1000px'></div>"); +TEST_F(ScrollingCoordinatorTest, fastScrollingCanBeDisabledWithSetting) { + navigateTo("about:blank"); webViewImpl()->settings()->setThreadedScrollingEnabled(false); forceFullCompositingUpdate(); @@ -180,7 +172,6 @@ // Main scrolling should be enabled with the setting override. WebLayer* rootScrollLayer = getRootScrollLayer(); - ASSERT_TRUE(rootScrollLayer); ASSERT_TRUE(rootScrollLayer->scrollable()); ASSERT_TRUE(rootScrollLayer->shouldScrollOnMainThread()); @@ -191,7 +182,7 @@ ASSERT_TRUE(innerViewportScrollLayer->shouldScrollOnMainThread()); } -TEST_P(ScrollingCoordinatorTest, fastFractionalScrollingDiv) { +TEST_F(ScrollingCoordinatorTest, fastFractionalScrollingDiv) { bool origFractionalOffsetsEnabled = RuntimeEnabledFeatures::fractionalScrollOffsetsEnabled(); RuntimeEnabledFeatures::setFractionalScrollOffsetsEnabled(true); @@ -251,14 +242,13 @@ return graphicsLayer->platformLayer(); } -TEST_P(ScrollingCoordinatorTest, fastScrollingForFixedPosition) { +TEST_F(ScrollingCoordinatorTest, fastScrollingForFixedPosition) { registerMockedHttpURLLoad("fixed-position.html"); navigateTo(m_baseURL + "fixed-position.html"); forceFullCompositingUpdate(); // Fixed position should not fall back to main thread scrolling. WebLayer* rootScrollLayer = getRootScrollLayer(); - ASSERT_TRUE(rootScrollLayer); ASSERT_FALSE(rootScrollLayer->shouldScrollOnMainThread()); Document* document = frame()->document(); @@ -344,14 +334,13 @@ } } -TEST_P(ScrollingCoordinatorTest, fastScrollingForStickyPosition) { +TEST_F(ScrollingCoordinatorTest, fastScrollingForStickyPosition) { registerMockedHttpURLLoad("sticky-position.html"); navigateTo(m_baseURL + "sticky-position.html"); forceFullCompositingUpdate(); // Sticky position should not fall back to main thread scrolling. WebLayer* rootScrollLayer = getRootScrollLayer(); - ASSERT_TRUE(rootScrollLayer); EXPECT_FALSE(rootScrollLayer->shouldScrollOnMainThread()); Document* document = frame()->document(); @@ -451,7 +440,7 @@ } } -TEST_P(ScrollingCoordinatorTest, touchEventHandler) { +TEST_F(ScrollingCoordinatorTest, touchEventHandler) { registerMockedHttpURLLoad("touch-event-handler.html"); navigateTo(m_baseURL + "touch-event-handler.html"); forceFullCompositingUpdate(); @@ -461,7 +450,7 @@ WebEventListenerClass::TouchStartOrMove)); } -TEST_P(ScrollingCoordinatorTest, touchEventHandlerPassive) { +TEST_F(ScrollingCoordinatorTest, touchEventHandlerPassive) { registerMockedHttpURLLoad("touch-event-handler-passive.html"); navigateTo(m_baseURL + "touch-event-handler-passive.html"); forceFullCompositingUpdate(); @@ -471,7 +460,7 @@ WebEventListenerClass::TouchStartOrMove)); } -TEST_P(ScrollingCoordinatorTest, touchEventHandlerBoth) { +TEST_F(ScrollingCoordinatorTest, touchEventHandlerBoth) { registerMockedHttpURLLoad("touch-event-handler-both.html"); navigateTo(m_baseURL + "touch-event-handler-both.html"); forceFullCompositingUpdate(); @@ -481,7 +470,7 @@ WebEventListenerClass::TouchStartOrMove)); } -TEST_P(ScrollingCoordinatorTest, wheelEventHandler) { +TEST_F(ScrollingCoordinatorTest, wheelEventHandler) { registerMockedHttpURLLoad("wheel-event-handler.html"); navigateTo(m_baseURL + "wheel-event-handler.html"); forceFullCompositingUpdate(); @@ -491,7 +480,7 @@ WebEventListenerClass::MouseWheel)); } -TEST_P(ScrollingCoordinatorTest, wheelEventHandlerPassive) { +TEST_F(ScrollingCoordinatorTest, wheelEventHandlerPassive) { registerMockedHttpURLLoad("wheel-event-handler-passive.html"); navigateTo(m_baseURL + "wheel-event-handler-passive.html"); forceFullCompositingUpdate(); @@ -501,7 +490,7 @@ WebEventListenerClass::MouseWheel)); } -TEST_P(ScrollingCoordinatorTest, wheelEventHandlerBoth) { +TEST_F(ScrollingCoordinatorTest, wheelEventHandlerBoth) { registerMockedHttpURLLoad("wheel-event-handler-both.html"); navigateTo(m_baseURL + "wheel-event-handler-both.html"); forceFullCompositingUpdate(); @@ -511,7 +500,7 @@ WebEventListenerClass::MouseWheel)); } -TEST_P(ScrollingCoordinatorTest, scrollEventHandler) { +TEST_F(ScrollingCoordinatorTest, scrollEventHandler) { registerMockedHttpURLLoad("scroll-event-handler.html"); navigateTo(m_baseURL + "scroll-event-handler.html"); forceFullCompositingUpdate(); @@ -519,7 +508,7 @@ ASSERT_TRUE(webLayerTreeView()->haveScrollEventHandlers()); } -TEST_P(ScrollingCoordinatorTest, updateEventHandlersDuringTeardown) { +TEST_F(ScrollingCoordinatorTest, updateEventHandlersDuringTeardown) { registerMockedHttpURLLoad("scroll-event-handler-window.html"); navigateTo(m_baseURL + "scroll-event-handler-window.html"); forceFullCompositingUpdate(); @@ -529,17 +518,16 @@ frame()->document()->shutdown(); } -TEST_P(ScrollingCoordinatorTest, clippedBodyTest) { +TEST_F(ScrollingCoordinatorTest, clippedBodyTest) { registerMockedHttpURLLoad("clipped-body.html"); navigateTo(m_baseURL + "clipped-body.html"); forceFullCompositingUpdate(); WebLayer* rootScrollLayer = getRootScrollLayer(); - ASSERT_TRUE(rootScrollLayer); ASSERT_EQ(0u, rootScrollLayer->nonFastScrollableRegion().size()); } -TEST_P(ScrollingCoordinatorTest, overflowScrolling) { +TEST_F(ScrollingCoordinatorTest, overflowScrolling) { registerMockedHttpURLLoad("overflow-scrolling.html"); navigateTo(m_baseURL + "overflow-scrolling.html"); forceFullCompositingUpdate(); @@ -585,7 +573,7 @@ #endif } -TEST_P(ScrollingCoordinatorTest, overflowHidden) { +TEST_F(ScrollingCoordinatorTest, overflowHidden) { registerMockedHttpURLLoad("overflow-hidden.html"); navigateTo(m_baseURL + "overflow-hidden.html"); forceFullCompositingUpdate(); @@ -646,7 +634,7 @@ ASSERT_TRUE(webScrollLayer->userScrollableVertical()); } -TEST_P(ScrollingCoordinatorTest, iframeScrolling) { +TEST_F(ScrollingCoordinatorTest, iframeScrolling) { registerMockedHttpURLLoad("iframe-scrolling.html"); registerMockedHttpURLLoad("iframe-scrolling-inner.html"); navigateTo(m_baseURL + "iframe-scrolling.html"); @@ -672,12 +660,10 @@ PaintLayerCompositor* innerCompositor = innerLayoutViewItem.compositor(); ASSERT_TRUE(innerCompositor->inCompositingMode()); + ASSERT_TRUE(innerCompositor->scrollLayer()); - GraphicsLayer* scrollLayer = - innerFrameView->layoutViewportScrollableArea()->layerForScrolling(); - ASSERT_TRUE(scrollLayer); - ASSERT_EQ(innerFrameView->layoutViewportScrollableArea(), - scrollLayer->getScrollableArea()); + GraphicsLayer* scrollLayer = innerCompositor->scrollLayer(); + ASSERT_EQ(innerFrameView, scrollLayer->getScrollableArea()); WebLayer* webScrollLayer = scrollLayer->platformLayer(); ASSERT_TRUE(webScrollLayer->scrollable()); @@ -692,7 +678,7 @@ #endif } -TEST_P(ScrollingCoordinatorTest, rtlIframe) { +TEST_F(ScrollingCoordinatorTest, rtlIframe) { registerMockedHttpURLLoad("rtl-iframe.html"); registerMockedHttpURLLoad("rtl-iframe-inner.html"); navigateTo(m_baseURL + "rtl-iframe.html"); @@ -718,26 +704,21 @@ PaintLayerCompositor* innerCompositor = innerLayoutViewItem.compositor(); ASSERT_TRUE(innerCompositor->inCompositingMode()); + ASSERT_TRUE(innerCompositor->scrollLayer()); - GraphicsLayer* scrollLayer = - innerFrameView->layoutViewportScrollableArea()->layerForScrolling(); - ASSERT_TRUE(scrollLayer); - ASSERT_EQ(innerFrameView->layoutViewportScrollableArea(), - scrollLayer->getScrollableArea()); + GraphicsLayer* scrollLayer = innerCompositor->scrollLayer(); + ASSERT_EQ(innerFrameView, scrollLayer->getScrollableArea()); WebLayer* webScrollLayer = scrollLayer->platformLayer(); ASSERT_TRUE(webScrollLayer->scrollable()); int expectedScrollPosition = - 958 + (innerFrameView->layoutViewportScrollableArea() - ->verticalScrollbar() - ->isOverlayScrollbar() - ? 0 - : 15); + 958 + + (innerFrameView->verticalScrollbar()->isOverlayScrollbar() ? 0 : 15); ASSERT_EQ(expectedScrollPosition, webScrollLayer->scrollPositionDouble().x); } -TEST_P(ScrollingCoordinatorTest, setupScrollbarLayerShouldNotCrash) { +TEST_F(ScrollingCoordinatorTest, setupScrollbarLayerShouldNotCrash) { registerMockedHttpURLLoad("setup_scrollbar_layer_crash.html"); navigateTo(m_baseURL + "setup_scrollbar_layer_crash.html"); forceFullCompositingUpdate(); @@ -745,7 +726,7 @@ // an empty document by javascript. } -TEST_P(ScrollingCoordinatorTest, +TEST_F(ScrollingCoordinatorTest, scrollbarsForceMainThreadOrHaveWebScrollbarLayer) { registerMockedHttpURLLoad("trivial-scroller.html"); navigateTo(m_baseURL + "trivial-scroller.html"); @@ -772,10 +753,10 @@ } #if OS(MACOSX) || OS(ANDROID) -TEST_P(ScrollingCoordinatorTest, +TEST_F(ScrollingCoordinatorTest, DISABLED_setupScrollbarLayerShouldSetScrollLayerOpaque) #else -TEST_P(ScrollingCoordinatorTest, setupScrollbarLayerShouldSetScrollLayerOpaque) +TEST_F(ScrollingCoordinatorTest, setupScrollbarLayerShouldSetScrollLayerOpaque) #endif { registerMockedHttpURLLoad("wide_document.html"); @@ -786,7 +767,7 @@ ASSERT_TRUE(frameView); GraphicsLayer* scrollbarGraphicsLayer = - frameView->layoutViewportScrollableArea()->layerForHorizontalScrollbar(); + frameView->layerForHorizontalScrollbar(); ASSERT_TRUE(scrollbarGraphicsLayer); WebLayer* platformLayer = scrollbarGraphicsLayer->platformLayer(); @@ -801,16 +782,19 @@ ASSERT_EQ(platformLayer->opaque(), contentsLayer->opaque()); } -TEST_P(ScrollingCoordinatorTest, +TEST_F(ScrollingCoordinatorTest, FixedPositionLosingBackingShouldTriggerMainThreadScroll) { webViewImpl()->settings()->setPreferCompositingToLCDTextEnabled(false); registerMockedHttpURLLoad("fixed-position-losing-backing.html"); navigateTo(m_baseURL + "fixed-position-losing-backing.html"); forceFullCompositingUpdate(); - WebLayer* scrollLayer = getRootScrollLayer(); - ASSERT_TRUE(scrollLayer); - + WebLayer* scrollLayer = frame() + ->page() + ->deprecatedLocalMainFrame() + ->view() + ->layerForScrolling() + ->platformLayer(); Document* document = frame()->document(); Element* fixedPos = document->getElementById("fixed"); @@ -828,7 +812,7 @@ EXPECT_TRUE(scrollLayer->shouldScrollOnMainThread()); } -TEST_P(ScrollingCoordinatorTest, CustomScrollbarShouldTriggerMainThreadScroll) { +TEST_F(ScrollingCoordinatorTest, CustomScrollbarShouldTriggerMainThreadScroll) { webViewImpl()->settings()->setPreferCompositingToLCDTextEnabled(true); webViewImpl()->setDeviceScaleFactor(2.f); registerMockedHttpURLLoad("custom_scrollbar.html"); @@ -869,7 +853,7 @@ MainThreadScrollingReason::kCustomScrollbarScrolling); } -TEST_P(ScrollingCoordinatorTest, +TEST_F(ScrollingCoordinatorTest, BackgroundAttachmentFixedShouldTriggerMainThreadScroll) { registerMockedHttpURLLoad("iframe-background-attachment-fixed.html"); registerMockedHttpURLLoad("iframe-background-attachment-fixed-inner.html"); @@ -895,12 +879,10 @@ PaintLayerCompositor* innerCompositor = innerLayoutViewItem.compositor(); ASSERT_TRUE(innerCompositor->inCompositingMode()); + ASSERT_TRUE(innerCompositor->scrollLayer()); - GraphicsLayer* scrollLayer = - innerFrameView->layoutViewportScrollableArea()->layerForScrolling(); - ASSERT_TRUE(scrollLayer); - ASSERT_EQ(innerFrameView->layoutViewportScrollableArea(), - scrollLayer->getScrollableArea()); + GraphicsLayer* scrollLayer = innerCompositor->scrollLayer(); + ASSERT_EQ(innerFrameView, scrollLayer->getScrollableArea()); WebLayer* webScrollLayer = scrollLayer->platformLayer(); ASSERT_TRUE(webScrollLayer->scrollable()); @@ -919,9 +901,7 @@ layoutObject = iframe->layoutObject(); ASSERT_TRUE(layoutObject); - scrollLayer = layoutObject->frameView() - ->layoutViewportScrollableArea() - ->layerForScrolling(); + scrollLayer = layoutObject->frameView()->layerForScrolling(); ASSERT_TRUE(scrollLayer); webScrollLayer = scrollLayer->platformLayer(); @@ -942,9 +922,7 @@ layoutObject = iframe->layoutObject(); ASSERT_TRUE(layoutObject); - scrollLayer = layoutObject->frameView() - ->layoutViewportScrollableArea() - ->layerForScrolling(); + scrollLayer = layoutObject->frameView()->layerForScrolling(); ASSERT_TRUE(scrollLayer); webScrollLayer = scrollLayer->platformLayer(); @@ -955,7 +933,7 @@ // Upon resizing the content size, the main thread scrolling reason // kHasNonLayerViewportConstrainedObject should be updated on all frames -TEST_P(ScrollingCoordinatorTest, +TEST_F(ScrollingCoordinatorTest, RecalculateMainThreadScrollingReasonsUponResize) { webViewImpl()->settings()->setPreferCompositingToLCDTextEnabled(false); registerMockedHttpURLLoad("has-non-layer-viewport-constrained-objects.html"); @@ -969,23 +947,29 @@ LayoutObject* layoutObject = element->layoutObject(); ASSERT_TRUE(layoutObject); - GraphicsLayer* scrollLayer = layoutObject->frameView() - ->layoutViewportScrollableArea() - ->layerForScrolling(); - WebLayer* webScrollLayer; + GraphicsLayer* scrollLayer = layoutObject->frameView()->layerForScrolling(); + ASSERT_TRUE(scrollLayer); - if (RuntimeEnabledFeatures::rootLayerScrollingEnabled()) { - // When RLS is enabled, the LayoutView won't have a scrolling contents - // because it does not overflow. - ASSERT_FALSE(scrollLayer); - } else { - ASSERT_TRUE(scrollLayer); - webScrollLayer = scrollLayer->platformLayer(); - ASSERT_TRUE(webScrollLayer->scrollable()); - ASSERT_FALSE( - webScrollLayer->mainThreadScrollingReasons() & - MainThreadScrollingReason::kHasNonLayerViewportConstrainedObjects); - } + WebLayer* webScrollLayer = scrollLayer->platformLayer(); + ASSERT_TRUE(webScrollLayer->scrollable()); + ASSERT_FALSE( + webScrollLayer->mainThreadScrollingReasons() & + MainThreadScrollingReason::kHasNonLayerViewportConstrainedObjects); + + Element* iframe = frame()->document()->getElementById("iframe"); + ASSERT_TRUE(iframe); + + layoutObject = iframe->layoutObject(); + ASSERT_TRUE(layoutObject); + + scrollLayer = layoutObject->frameView()->layerForScrolling(); + ASSERT_TRUE(scrollLayer); + + webScrollLayer = scrollLayer->platformLayer(); + ASSERT_TRUE(webScrollLayer->scrollable()); + ASSERT_FALSE( + webScrollLayer->mainThreadScrollingReasons() & + MainThreadScrollingReason::kHasNonLayerViewportConstrainedObjects); // When the div becomes to scrollable it should scroll on main thread element->setAttribute("style", @@ -996,9 +980,19 @@ layoutObject = element->layoutObject(); ASSERT_TRUE(layoutObject); - scrollLayer = layoutObject->frameView() - ->layoutViewportScrollableArea() - ->layerForScrolling(); + scrollLayer = layoutObject->frameView()->layerForScrolling(); + ASSERT_TRUE(scrollLayer); + + webScrollLayer = scrollLayer->platformLayer(); + ASSERT_TRUE(webScrollLayer->scrollable()); + ASSERT_TRUE( + webScrollLayer->mainThreadScrollingReasons() & + MainThreadScrollingReason::kHasNonLayerViewportConstrainedObjects); + + layoutObject = iframe->layoutObject(); + ASSERT_TRUE(layoutObject); + + scrollLayer = layoutObject->frameView()->layerForScrolling(); ASSERT_TRUE(scrollLayer); webScrollLayer = scrollLayer->platformLayer(); @@ -1016,21 +1010,26 @@ layoutObject = element->layoutObject(); ASSERT_TRUE(layoutObject); - scrollLayer = layoutObject->frameView() - ->layoutViewportScrollableArea() - ->layerForScrolling(); - if (RuntimeEnabledFeatures::rootLayerScrollingEnabled()) { - // When RLS is enabled, the LayoutView won't have a scrolling contents - // because it does not overflow. - ASSERT_FALSE(scrollLayer); - } else { - ASSERT_TRUE(scrollLayer); - webScrollLayer = scrollLayer->platformLayer(); - ASSERT_TRUE(webScrollLayer->scrollable()); - ASSERT_FALSE( - webScrollLayer->mainThreadScrollingReasons() & - MainThreadScrollingReason::kHasNonLayerViewportConstrainedObjects); - } + scrollLayer = layoutObject->frameView()->layerForScrolling(); + ASSERT_TRUE(scrollLayer); + + webScrollLayer = scrollLayer->platformLayer(); + ASSERT_TRUE(webScrollLayer->scrollable()); + ASSERT_FALSE( + webScrollLayer->mainThreadScrollingReasons() & + MainThreadScrollingReason::kHasNonLayerViewportConstrainedObjects); + + layoutObject = iframe->layoutObject(); + ASSERT_TRUE(layoutObject); + + scrollLayer = layoutObject->frameView()->layerForScrolling(); + ASSERT_TRUE(scrollLayer); + + webScrollLayer = scrollLayer->platformLayer(); + ASSERT_TRUE(webScrollLayer->scrollable()); + ASSERT_FALSE( + webScrollLayer->mainThreadScrollingReasons() & + MainThreadScrollingReason::kHasNonLayerViewportConstrainedObjects); } class StyleRelatedMainThreadScrollingReasonTest @@ -1093,32 +1092,28 @@ } }; -INSTANTIATE_TEST_CASE_P(All, - StyleRelatedMainThreadScrollingReasonTest, - ::testing::Bool()); - -TEST_P(StyleRelatedMainThreadScrollingReasonTest, TransparentTest) { +TEST_F(StyleRelatedMainThreadScrollingReasonTest, TransparentTest) { testStyle("transparent", MainThreadScrollingReason::kHasOpacityAndLCDText); } -TEST_P(StyleRelatedMainThreadScrollingReasonTest, TransformTest) { +TEST_F(StyleRelatedMainThreadScrollingReasonTest, TransformTest) { testStyle("transform", MainThreadScrollingReason::kHasTransformAndLCDText); } -TEST_P(StyleRelatedMainThreadScrollingReasonTest, BackgroundNotOpaqueTest) { +TEST_F(StyleRelatedMainThreadScrollingReasonTest, BackgroundNotOpaqueTest) { testStyle("background-not-opaque", MainThreadScrollingReason::kBackgroundNotOpaqueInRectAndLCDText); } -TEST_P(StyleRelatedMainThreadScrollingReasonTest, BorderRadiusTest) { +TEST_F(StyleRelatedMainThreadScrollingReasonTest, BorderRadiusTest) { testStyle("border-radius", MainThreadScrollingReason::kHasBorderRadius); } -TEST_P(StyleRelatedMainThreadScrollingReasonTest, ClipTest) { +TEST_F(StyleRelatedMainThreadScrollingReasonTest, ClipTest) { testStyle("clip", MainThreadScrollingReason::kHasClipRelatedProperty); } -TEST_P(StyleRelatedMainThreadScrollingReasonTest, ClipPathTest) { +TEST_F(StyleRelatedMainThreadScrollingReasonTest, ClipPathTest) { uint32_t reason = MainThreadScrollingReason::kHasClipRelatedProperty; webViewImpl()->settings()->setPreferCompositingToLCDTextEnabled(false); Document* document = frame()->document(); @@ -1153,13 +1148,13 @@ ASSERT_FALSE(frameView->mainThreadScrollingReasons() & reason); } -TEST_P(StyleRelatedMainThreadScrollingReasonTest, LCDTextEnabledTest) { +TEST_F(StyleRelatedMainThreadScrollingReasonTest, LCDTextEnabledTest) { testStyle("transparent border-radius", MainThreadScrollingReason::kHasOpacityAndLCDText | MainThreadScrollingReason::kHasBorderRadius); } -TEST_P(StyleRelatedMainThreadScrollingReasonTest, BoxShadowTest) { +TEST_F(StyleRelatedMainThreadScrollingReasonTest, BoxShadowTest) { testStyle("box-shadow", MainThreadScrollingReason::kHasBoxShadowFromNonRootLayer); }
diff --git a/third_party/WebKit/Source/web/tests/WebFrameTest.cpp b/third_party/WebKit/Source/web/tests/WebFrameTest.cpp index bc04711ad..d123a03 100644 --- a/third_party/WebKit/Source/web/tests/WebFrameTest.cpp +++ b/third_party/WebKit/Source/web/tests/WebFrameTest.cpp
@@ -7761,16 +7761,6 @@ webViewHelper.resize(WebSize(viewportWidth, viewportHeight)); webViewImpl->updateAllLifecyclePhases(); - WebLayer* webScrollLayer = webViewImpl->mainFrameImpl() - ->frame() - ->view() - ->layoutViewportScrollableArea() - ->layerForScrolling() - ->platformLayer(); - ASSERT_TRUE(webScrollLayer->scrollable()); - ASSERT_TRUE(webScrollLayer->userScrollableHorizontal()); - ASSERT_TRUE(webScrollLayer->userScrollableVertical()); - Document* document = webViewImpl->mainFrameImpl()->frame()->document(); UserGestureIndicator gesture(DocumentUserGestureToken::create(document)); Fullscreen::requestFullscreen(*document->documentElement()); @@ -7790,12 +7780,8 @@ Fullscreen::fullscreenElementFrom(*document)); // Verify that the main frame is still scrollable. - webScrollLayer = webViewImpl->mainFrameImpl() - ->frame() - ->view() - ->layoutViewportScrollableArea() - ->layerForScrolling() - ->platformLayer(); + WebLayer* webScrollLayer = + webViewImpl->compositor()->scrollLayer()->platformLayer(); ASSERT_TRUE(webScrollLayer->scrollable()); ASSERT_TRUE(webScrollLayer->userScrollableHorizontal()); ASSERT_TRUE(webScrollLayer->userScrollableVertical());
diff --git a/third_party/WebKit/Source/web/tests/data/has-non-layer-viewport-constrained-objects.html b/third_party/WebKit/Source/web/tests/data/has-non-layer-viewport-constrained-objects.html index f2919be..50796f6 100644 --- a/third_party/WebKit/Source/web/tests/data/has-non-layer-viewport-constrained-objects.html +++ b/third_party/WebKit/Source/web/tests/data/has-non-layer-viewport-constrained-objects.html
@@ -1,2 +1,4 @@ <div style="position:fixed;">Fixed obj</div> -<div id="scrollable" style="overflow: scroll; height:200px; will-change:transform;"></div> +<div id="scrollable" style="overflow: scroll; height:200px; will-change:transform;"> + <iframe id="iframe"></iframe> +</div>
diff --git a/third_party/WebKit/Source/web/tests/data/iframe-background-attachment-fixed-inner.html b/third_party/WebKit/Source/web/tests/data/iframe-background-attachment-fixed-inner.html index 5dc4b2d..38843967 100644 --- a/third_party/WebKit/Source/web/tests/data/iframe-background-attachment-fixed-inner.html +++ b/third_party/WebKit/Source/web/tests/data/iframe-background-attachment-fixed-inner.html
@@ -3,7 +3,8 @@ .background-attachment-fixed { background-image: url("white-1x1.png"); background-attachment: fixed; + height: 2000px; } </style> -<div id="scrollable" style="will-change:transform; height: 2000px;" class="background-attachment-fixed" /> +<div id="scrollable" style="will-change:transform;" class="background-attachment-fixed" />
diff --git a/third_party/libaddressinput/chromium/chrome_storage_impl.cc b/third_party/libaddressinput/chromium/chrome_storage_impl.cc index e901c2f..4daab33 100644 --- a/third_party/libaddressinput/chromium/chrome_storage_impl.cc +++ b/third_party/libaddressinput/chromium/chrome_storage_impl.cc
@@ -7,6 +7,7 @@ #include <memory> #include <utility> +#include "base/memory/ptr_util.h" #include "base/values.h" #include "components/prefs/writeable_pref_store.h" #include "third_party/libaddressinput/chromium/fallback_data_store.h" @@ -24,11 +25,9 @@ void ChromeStorageImpl::Put(const std::string& key, std::string* data) { DCHECK(data); std::unique_ptr<std::string> owned_data(data); - std::unique_ptr<base::StringValue> string_value( - new base::StringValue(std::string())); - string_value->GetString()->swap(*owned_data); - backing_store_->SetValue(key, std::move(string_value), - WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); + backing_store_->SetValue( + key, base::MakeUnique<base::StringValue>(std::move(*owned_data)), + WriteablePrefStore::DEFAULT_PREF_WRITE_FLAGS); } void ChromeStorageImpl::Get(const std::string& key,
