Commit 33fc1dcc authored by Denis Yaroshevskiy's avatar Denis Yaroshevskiy Committed by Commit Bot

Better DCHECKing for match validity in components/omnibox.

- Replacing NDEBUG => DCHECK_IS_ON
- Validating match before adding it as a shortcut.

Change-Id: Iaa29faa7cc37c8cf75d833ce8c166c2122d6da55
Reviewed-on: https://chromium-review.googlesource.com/986054
Commit-Queue: Denis Yaroshevskiy <dyaroshev@yandex-team.ru>
Reviewed-by: default avatarKevin Bailey <krb@chromium.org>
Reviewed-by: default avatarJustin Donnelly <jdonnelly@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548019}
parent 1474295c
...@@ -509,9 +509,9 @@ void AutocompleteController::UpdateResult( ...@@ -509,9 +509,9 @@ void AutocompleteController::UpdateResult(
// Need to validate before invoking CopyOldMatches as the old matches are not // Need to validate before invoking CopyOldMatches as the old matches are not
// valid against the current input. // valid against the current input.
#ifndef NDEBUG #if DCHECK_IS_ON()
result_.Validate(); result_.Validate();
#endif #endif // DCHECK_IS_ON()
if (!done_) { if (!done_) {
// This conditional needs to match the conditional in Start that invokes // This conditional needs to match the conditional in Start that invokes
......
...@@ -749,7 +749,7 @@ size_t AutocompleteMatch::EstimateMemoryUsage() const { ...@@ -749,7 +749,7 @@ size_t AutocompleteMatch::EstimateMemoryUsage() const {
return res; return res;
} }
#ifndef NDEBUG #if DCHECK_IS_ON()
void AutocompleteMatch::Validate() const { void AutocompleteMatch::Validate() const {
ValidateClassifications(contents, contents_class); ValidateClassifications(contents, contents_class);
ValidateClassifications(description, description_class); ValidateClassifications(description, description_class);
...@@ -786,4 +786,4 @@ void AutocompleteMatch::ValidateClassifications( ...@@ -786,4 +786,4 @@ void AutocompleteMatch::ValidateClassifications(
last_offset = i->offset; last_offset = i->offset;
} }
} }
#endif #endif // DCHECK_IS_ON()
...@@ -465,7 +465,7 @@ struct AutocompleteMatch { ...@@ -465,7 +465,7 @@ struct AutocompleteMatch {
// ensure if a match is deleted, the duplicates are deleted as well. // ensure if a match is deleted, the duplicates are deleted as well.
std::vector<AutocompleteMatch> duplicate_matches; std::vector<AutocompleteMatch> duplicate_matches;
#ifndef NDEBUG #if DCHECK_IS_ON()
// Does a data integrity check on this match. // Does a data integrity check on this match.
void Validate() const; void Validate() const;
...@@ -473,7 +473,7 @@ struct AutocompleteMatch { ...@@ -473,7 +473,7 @@ struct AutocompleteMatch {
void ValidateClassifications( void ValidateClassifications(
const base::string16& text, const base::string16& text,
const ACMatchClassifications& classifications) const; const ACMatchClassifications& classifications) const;
#endif #endif // DCHECK_IS_ON()
}; };
typedef AutocompleteMatch::ACMatchClassification ACMatchClassification; typedef AutocompleteMatch::ACMatchClassification ACMatchClassification;
......
...@@ -99,11 +99,9 @@ void AutocompleteResult::CopyOldMatches( ...@@ -99,11 +99,9 @@ void AutocompleteResult::CopyOldMatches(
void AutocompleteResult::AppendMatches(const AutocompleteInput& input, void AutocompleteResult::AppendMatches(const AutocompleteInput& input,
const ACMatches& matches) { const ACMatches& matches) {
for (const auto& i : matches) { for (const auto& i : matches) {
#ifndef NDEBUG
DCHECK_EQ(AutocompleteMatch::SanitizeString(i.contents), i.contents); DCHECK_EQ(AutocompleteMatch::SanitizeString(i.contents), i.contents);
DCHECK_EQ(AutocompleteMatch::SanitizeString(i.description), DCHECK_EQ(AutocompleteMatch::SanitizeString(i.description),
i.description); i.description);
#endif
matches_.push_back(i); matches_.push_back(i);
if (!AutocompleteMatch::IsSearchType(i.type)) { if (!AutocompleteMatch::IsSearchType(i.type)) {
const OmniboxFieldTrial::EmphasizeTitlesCondition condition( const OmniboxFieldTrial::EmphasizeTitlesCondition condition(
...@@ -314,12 +312,12 @@ void AutocompleteResult::CopyFrom(const AutocompleteResult& rhs) { ...@@ -314,12 +312,12 @@ void AutocompleteResult::CopyFrom(const AutocompleteResult& rhs) {
alternate_nav_url_ = rhs.alternate_nav_url_; alternate_nav_url_ = rhs.alternate_nav_url_;
} }
#ifndef NDEBUG #if DCHECK_IS_ON()
void AutocompleteResult::Validate() const { void AutocompleteResult::Validate() const {
for (const_iterator i(begin()); i != end(); ++i) for (const_iterator i(begin()); i != end(); ++i)
i->Validate(); i->Validate();
} }
#endif #endif // DCHECK_IS_ON()
// static // static
GURL AutocompleteResult::ComputeAlternateNavUrl( GURL AutocompleteResult::ComputeAlternateNavUrl(
......
...@@ -92,10 +92,10 @@ class AutocompleteResult { ...@@ -92,10 +92,10 @@ class AutocompleteResult {
// operator=() by another name. // operator=() by another name.
void CopyFrom(const AutocompleteResult& rhs); void CopyFrom(const AutocompleteResult& rhs);
#ifndef NDEBUG #if DCHECK_IS_ON()
// Does a data integrity check on this result. // Does a data integrity check on this result.
void Validate() const; void Validate() const;
#endif #endif // DCHECK_IS_ON()
// Compute the "alternate navigation URL" for a given match. This is obtained // Compute the "alternate navigation URL" for a given match. This is obtained
// by interpreting the user input directly as a URL. See comments on // by interpreting the user input directly as a URL. See comments on
......
...@@ -482,9 +482,9 @@ TEST_F(HistoryQuickProviderTest, ContentsClass) { ...@@ -482,9 +482,9 @@ TEST_F(HistoryQuickProviderTest, ContentsClass) {
"82.A7.E3.83.AB.E3.82.B5.E3.82.A4.E3.83.A6.E4.BD." "82.A7.E3.83.AB.E3.82.B5.E3.82.A4.E3.83.A6.E4.BD."
"93.E5.88.B6"), "93.E5.88.B6"),
base::string16()); base::string16());
#ifndef NDEBUG #if DCHECK_IS_ON()
ac_matches()[0].Validate(); ac_matches()[0].Validate();
#endif #endif // DCHECK_IS_ON();
// Verify that contents_class divides the string in the right places. // Verify that contents_class divides the string in the right places.
// [22, 24) is the "第二". All the other pairs are the "e3". // [22, 24) is the "第二". All the other pairs are the "e3".
ACMatchClassifications contents_class(ac_matches()[0].contents_class); ACMatchClassifications contents_class(ac_matches()[0].contents_class);
......
...@@ -121,6 +121,9 @@ void ShortcutsBackend::RemoveObserver(ShortcutsBackendObserver* obs) { ...@@ -121,6 +121,9 @@ void ShortcutsBackend::RemoveObserver(ShortcutsBackendObserver* obs) {
void ShortcutsBackend::AddOrUpdateShortcut(const base::string16& text, void ShortcutsBackend::AddOrUpdateShortcut(const base::string16& text,
const AutocompleteMatch& match) { const AutocompleteMatch& match) {
#if DCHECK_IS_ON()
match.Validate();
#endif // DCHECK_IS_ON()
// TODO(crbug.com/46623): Let's think twice about saving these. // TODO(crbug.com/46623): Let's think twice about saving these.
if (match.type == AutocompleteMatchType::TAB_SEARCH) if (match.type == AutocompleteMatchType::TAB_SEARCH)
return; return;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment