[Suggestions category] Warn more about removing categories.

components/ntp_snippets/category.cc

 // Copyright 2016 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/ntp_snippets/category.h"
 
 #include "base/logging.h"
 
 namespace ntp_snippets {
 
 // static
 Category Category::FromKnownCategory(KnownCategories known_category) {
   return FromIDValue(static_cast<int>(known_category));
 }
 
 // static
 Category Category::FromRemoteCategory(int remote_category) {
   DCHECK_GT(remote_category, 0);
   return Category(static_cast<int>(KnownCategories::REMOTE_CATEGORIES_OFFSET) +
                   remote_category);
 }
 
 // static
 Category Category::FromIDValue(int id) {
-  DCHECK(IsValidIDValue(id)) << "Not a valid ID: " << id;
+  DCHECK(IsValidIDValue(id))
+      << "Not a valid ID: " << id
+      << " Note that local categories must not be removed. This can happen if "
+         "you switch out of a branch with a new local category.";

[vitaliii, 2017/02/08 08:40:35]

I have multiple concerns regarding this message:
1) It does not look like a sentence:
"Not a valid ID: 13 Note that local categories..."

2) Too much attention to category removal. There may be other reasons for this
DCHECK to fail, also shuffling categories is bad as well.

3) The last sentence seems very difficult to understand. I think it is enough to
point the reader into right direction (KnownCategories) and have more elaborate
message there.

Taking all of these into account, I would go for:

"id << is not a valid category ID. This may have been caused by removal of a
local KnownCategory."

[jkrcal, 2017/02/08 10:12:05]

Done.

   return Category(id);
 }
 
 // static
 bool Category::IsValidIDValue(int id) {
   return (id >= 0) &&
          ((id < static_cast<int>(KnownCategories::LOCAL_CATEGORIES_COUNT) ||
            id > static_cast<int>(KnownCategories::REMOTE_CATEGORIES_OFFSET)));
 }
 
@@ skipping 17 matching lines @@
                                        const Category& right) const {
   return left.id() < right.id();
 }
 
 std::ostream& operator<<(std::ostream& os, const Category& obj) {
   os << obj.id();
   return os;
 }
 
 }  // namespace ntp_snippets