[Extensions] Fix extension toolbar crash

chrome/browser/extensions/extension_toolbar_model.cc

 // Copyright (c) 2012 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 "chrome/browser/extensions/extension_toolbar_model.h"
 
 #include <algorithm>
 #include <string>
 
 #include "base/message_loop/message_loop.h"
@@ skipping 310 matching lines @@
     // For the last-known position, we use the index of the extension that is
     // just before this extension, plus one. (Note that this isn't the same
     // as new_index + 1, because last_known_positions_ can include disabled
     // extensions.)
     int new_last_known_index =
         new_index == 0 ? 0 :
         std::find(last_known_positions_.begin(),
                   last_known_positions_.end(),
                   toolbar_items_[new_index - 1]->id()) -
             last_known_positions_.begin() + 1;
+    // In theory, the extension before this one should always
+    // be in last known positions, but if something funny happened with prefs,
+    // make sure we handle it.
+    // TODO(devlin): Track down these cases so we can CHECK this.
+    new_last_known_index =
+        std::min(new_last_known_index,
+        static_cast<int>(last_known_positions_.size()));

[Marijn Kruisselbrink, 2015/02/20 18:26:41]

I personally prefer std::min<int>(a, b) over std::min(a, static_cast<int>(b))
here, but both work equally well of course.

[Devlin, 2015/02/20 21:50:00]

Done.

     last_known_positions_.insert(
         last_known_positions_.begin() + new_last_known_index, extension->id());
     UpdatePrefs();
   } else {
     new_index = FindNewPositionFromLastKnownGood(extension);
   }
 
   toolbar_items_.insert(toolbar_items_.begin() + new_index, extension);
 
   // If we're currently highlighting, then even though we add a browser action
@@ skipping 75 matching lines @@
 // have holes.
 // 2. Create a vector of extensions that did not have a pref value.
 // 3. Remove holes from the sorted vector and append the unsorted vector.
 void ExtensionToolbarModel::InitializeExtensionList() {
   DCHECK(toolbar_items_.empty());  // We shouldn't have any items yet.
 
   last_known_positions_ = extension_prefs_->GetToolbarOrder();
   if (profile_->IsOffTheRecord())
     IncognitoPopulate();
   else
-    Populate(last_known_positions_);
+    Populate(&last_known_positions_);
 
   extensions_initialized_ = true;
   MaybeUpdateVisibilityPrefs();
   FOR_EACH_OBSERVER(Observer, observers_, OnToolbarModelInitialized());
 }
 
-void ExtensionToolbarModel::Populate(const ExtensionIdList& positions) {
+void ExtensionToolbarModel::Populate(ExtensionIdList* positions) {
   DCHECK(!profile_->IsOffTheRecord());
   const ExtensionSet& extensions =
       ExtensionRegistry::Get(profile_)->enabled_extensions();
   // Items that have explicit positions.
-  ExtensionList sorted(positions.size(), NULL);
+  ExtensionList sorted(positions->size(), NULL);
   // The items that don't have explicit positions.
   ExtensionList unsorted;
 
   // Create the lists.
   int hidden = 0;
   for (const scoped_refptr<const Extension>& extension : extensions) {
     if (!ShouldAddExtension(extension.get())) {
       if (!ExtensionActionAPI::GetBrowserActionVisibility(extension_prefs_,
                                                           extension->id()))
         ++hidden;
       continue;
     }
 
     ExtensionIdList::const_iterator pos =
-        std::find(positions.begin(), positions.end(), extension->id());
-    if (pos != positions.end())
-      sorted[pos - positions.begin()] = extension;
-    else
+        std::find(positions->begin(), positions->end(), extension->id());
+    if (pos != positions->end()) {
+      sorted[pos - positions->begin()] = extension;
+    } else {
+      // Unknown extension - push it to the back of unsorted, and add it to the
+      // of ids at the end.

[Marijn Kruisselbrink, 2015/02/20 18:26:41]

"and add it to the of ids at the end"? 
I think you forgot a word there.

[Devlin, 2015/02/20 21:50:00]

Done.

       unsorted.push_back(extension);
+      positions->push_back(extension->id());
+    }
   }
 
   // Merge the lists.
   sorted.insert(sorted.end(), unsorted.begin(), unsorted.end());
   toolbar_items_.reserve(sorted.size());
 
   for (const scoped_refptr<const Extension>& extension : sorted) {
     // It's possible for the extension order to contain items that aren't
     // actually loaded on this machine.  For example, when extension sync is on,
     // we sync the extension order as-is but double-check with the user before
@@ skipping 115 matching lines @@
   // Clear out the old...
   while (!toolbar_items_.empty()) {
     scoped_refptr<const Extension> extension = toolbar_items_.back();
     toolbar_items_.pop_back();
     FOR_EACH_OBSERVER(
         Observer, observers_, ToolbarExtensionRemoved(extension.get()));
   }
   DCHECK(toolbar_items_.empty());
 
   // ...Add the new...
-  Populate(last_known_positions_);
+  Populate(&last_known_positions_);
 
   // ...And notify.
   for (size_t i = 0; i < toolbar_items().size(); ++i) {
     FOR_EACH_OBSERVER(Observer,
                       observers_,
                       ToolbarExtensionAdded(toolbar_items()[i].get(), i));
   }
 
   if (last_known_positions_.size() > pref_position_size) {
     // Need to update pref because we have extra icons. But can't call
@@ skipping 95 matching lines @@
   if (is_highlighting_) {
     highlighted_items_.clear();
     is_highlighting_ = false;
     if (old_visible_icon_count_ != visible_icon_count_)
       SetVisibleIconCount(old_visible_icon_count_);
     FOR_EACH_OBSERVER(Observer, observers_, ToolbarHighlightModeChanged(false));
   }
 }
 
 }  // namespace extensions