Persist the order of extensions in the browser action toolbar across sessions...

Persist the order of extensions in the browser action toolbar across sessions.

Very similar to what we did with the extension shelf.

BUG=26990
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=34490

[Evan Stade, 2009-12-11 02:17:15]

http://codereview.chromium.org/487021/diff/1007/1012
File chrome/browser/gtk/browser_actions_toolbar_gtk.cc (left):

http://codereview.chromium.org/487021/diff/1007/1012#oldcode333
chrome/browser/gtk/browser_actions_toolbar_gtk.cc:333: CreateAllButtons();
having investigated this more, I realized that this call was not necessary

[Evan Stade, 2009-12-11 03:52:46]

note that this won't actually appear to work until bug 30057 is fixed (I have a
CL out for it)

[Daniel Erat, 2009-12-11 05:45:37]

LGTM, but you might want to have someone who's more familiar with the prefs code
also take a look.

http://codereview.chromium.org/487021/diff/1007/1011
File chrome/browser/extensions/extension_toolbar_model.cc (right):

http://codereview.chromium.org/487021/diff/1007/1011#newcode121
chrome/browser/extensions/extension_toolbar_model.cc:121: // 3. Remove holes and
merge the two vectors.
"merge" confused me here.  Maybe use something like "Copy the sorted vector
(skipping holes) and then the unsorted vector to |toolitems_|."?

[Evan Stade, 2009-12-11 19:43:19]

+tony

http://codereview.chromium.org/487021/diff/1007/1011
File chrome/browser/extensions/extension_toolbar_model.cc (right):

http://codereview.chromium.org/487021/diff/1007/1011#newcode121
chrome/browser/extensions/extension_toolbar_model.cc:121: // 3. Remove holes and
merge the two vectors.
On 2009/12/11 05:45:38, Daniel Erat wrote:
> "merge" confused me here.  Maybe use something like "Copy the sorted vector
> (skipping holes) and then the unsorted vector to |toolitems_|."?

Done.

[tony, 2009-12-11 19:59:38]

LGTM

http://codereview.chromium.org/487021/diff/8/1017
File chrome/browser/extensions/extension_toolbar_model.cc (right):

http://codereview.chromium.org/487021/diff/8/1017#newcode136
chrome/browser/extensions/extension_toolbar_model.cc:136:
service_->extensions()->at(i)->id(), false);
Nit: Doesn't at(i) return the extension you want?

[Evan Stade, 2009-12-11 20:04:20]

http://codereview.chromium.org/487021/diff/8/1017
File chrome/browser/extensions/extension_toolbar_model.cc (right):

http://codereview.chromium.org/487021/diff/8/1017#newcode136
chrome/browser/extensions/extension_toolbar_model.cc:136:
service_->extensions()->at(i)->id(), false);
On 2009/12/11 19:59:38, tony wrote:
> Nit: Doesn't at(i) return the extension you want?

hmm, so it does. Don't I feel stupid.