cros: Allow default app order to be loaded from an external file.

cros: Allow default app order to be loaded from an external file.

BUG=157361
TEST=Verify default app order could be changed from external file.

R=tbarzic@chromium.org,csharp@chromium.org,dpolukhin@chromium.org

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

[csharp, 2012-10-26 19:21:43]

extension_sorting changes LGTM

[tbarzic, 2012-10-26 21:56:28]

http://codereview.chromium.org/11318008/diff/1/chrome/browser/chromeos/chrome...
File chrome/browser/chromeos/chrome_browser_main_chromeos.cc (right):

http://codereview.chromium.org/11318008/diff/1/chrome/browser/chromeos/chrome...
chrome/browser/chromeos/chrome_browser_main_chromeos.cc:409:
app_order_loader_->WaitForLoad();
Is there any way to get rid of this?
looks so ugly :(

http://codereview.chromium.org/11318008/diff/1/chrome/browser/chromeos/extens...
File chrome/browser/chromeos/extensions/default_app_order.cc (right):

http://codereview.chromium.org/11318008/diff/1/chrome/browser/chromeos/extens...
chrome/browser/chromeos/extensions/default_app_order.cc:89:
ExternalLoader::ExternalLoader() : loaded_(true, false) {
nit: a comment whet true and false mean.

http://codereview.chromium.org/11318008/diff/1/chrome/browser/chromeos/extens...
chrome/browser/chromeos/extensions/default_app_order.cc:95:
base::Unretained(this)));
can blocking pool still be running while this is destructed? (my guess is no,
but I'm not completely sure)

[xiyuan, 2012-10-26 22:39:25]

http://codereview.chromium.org/11318008/diff/1/chrome/browser/chromeos/chrome...
File chrome/browser/chromeos/chrome_browser_main_chromeos.cc (right):

http://codereview.chromium.org/11318008/diff/1/chrome/browser/chromeos/chrome...
chrome/browser/chromeos/chrome_browser_main_chromeos.cc:409:
app_order_loader_->WaitForLoad();
On 2012/10/26 21:56:28, tbarzic wrote:
> Is there any way to get rid of this?
> looks so ugly :(

I removed the wait call but we still have to do a synchronous load here.

http://codereview.chromium.org/11318008/diff/1/chrome/browser/chromeos/extens...
File chrome/browser/chromeos/extensions/default_app_order.cc (right):

http://codereview.chromium.org/11318008/diff/1/chrome/browser/chromeos/extens...
chrome/browser/chromeos/extensions/default_app_order.cc:89:
ExternalLoader::ExternalLoader() : loaded_(true, false) {
On 2012/10/26 21:56:28, tbarzic wrote:
> nit: a comment whet true and false mean.

Done.

http://codereview.chromium.org/11318008/diff/1/chrome/browser/chromeos/extens...
chrome/browser/chromeos/extensions/default_app_order.cc:95:
base::Unretained(this)));
On 2012/10/26 21:56:28, tbarzic wrote:
> can blocking pool still be running while this is destructed? (my guess is no,
> but I'm not completely sure)

Right now, the only ExternalLoader instance is owned by
ChromeBrowserMainPartsChromeos and get released after blocking pool shuts down.

Added a DCHECK in dtor to make sure that the posted task has run when
destructing this class. This should avoid future regressions when we instantiate
it elsewhere.

[tbarzic, 2012-10-26 22:59:14]

ok, LGTM

[xiyuan, 2012-10-26 23:14:05]

+ben, could you do owner's review?

[Ben Goodger (Google), 2012-10-29 17:15:23]

lgtm

[commit-bot: I haz the power, 2012-10-29 17:18:29]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/xiyuan@chromium.org/11318008/7002

[commit-bot: I haz the power, 2012-10-29 19:22:39]

Change committed as 164705