Make customization documents singletons.

Make customization documents singletons.

BUG=chromium-os:13019
TEST=existing browser tests

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

[Nikita (slow), 2011-04-27 13:57:06]

http://codereview.chromium.org/6893052/diff/1020/chrome/browser/chromeos/cust...
File chrome/browser/chromeos/customization_document.cc (right):

http://codereview.chromium.org/6893052/diff/1020/chrome/browser/chromeos/cust...
chrome/browser/chromeos/customization_document.cc:122: //
StartupCustomizationDocument implementation.
Please use this style here and trough the file:

// StartupCustomizationDocument implementation. -------------

http://codereview.chromium.org/6893052/diff/1020/chrome/browser/chromeos/cust...
chrome/browser/chromeos/customization_document.cc:135:
LoadManifestFromString(manifest);
Are thread restrictions applied during testing?

http://codereview.chromium.org/6893052/diff/1020/chrome/browser/chromeos/cust...
chrome/browser/chromeos/customization_document.cc:207:
LoadManifestFromString(manifest);
Instance enters into state when call on StartFetching() will start fetching
empty URL.
Could you at least check for empty URL in StartFetching()?

http://codereview.chromium.org/6893052/diff/1020/chrome/browser/chromeos/cust...
File chrome/browser/chromeos/customization_document.h (right):

http://codereview.chromium.org/6893052/diff/1020/chrome/browser/chromeos/cust...
chrome/browser/chromeos/customization_document.h:11: #include "base/file_path.h"
Could FilePath be forward declared here?

http://codereview.chromium.org/6893052/diff/1020/chrome/browser/chromeos/cust...
chrome/browser/chromeos/customization_document.h:33: // Return true if the
document was successfully fetched and ready for apply.
nit: was successfully fetched and parsed.

http://codereview.chromium.org/6893052/diff/1020/chrome/browser/chromeos/cust...
chrome/browser/chromeos/customization_document.h:53: class
StartupCustomizationDocument : public CustomizationDocument {
Please add short comment on life-cycle.
This document receives definite ready/not ready state right after instance is
created.

http://codereview.chromium.org/6893052/diff/1020/chrome/browser/chromeos/cust...
chrome/browser/chromeos/customization_document.h:92: class
ServicesCustomizationDocument : public CustomizationDocument,
Please add short comment on life-cycle.
This document receives needs to be fetched after created and only when it's
fetched it gets proper ready/not ready state.

http://codereview.chromium.org/6893052/diff/1020/chrome/browser/chromeos/cust...
chrome/browser/chromeos/customization_document.h:102: static bool WasApplied();
Also expose WasFetched?

http://codereview.chromium.org/6893052/diff/1020/chrome/browser/chromeos/cust...
chrome/browser/chromeos/customization_document.h:109: bool ApplyCustmization();
nit: Customization.

http://codereview.chromium.org/6893052/diff/1020/chrome/browser/chromeos/logi...
File chrome/browser/chromeos/login/base_login_display_host.cc (right):

http://codereview.chromium.org/6893052/diff/1020/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/base_login_display_host.cc:149: // Initiate
services customization.
Nit: should say "Initiate service customization manifest fetching."

[Nikita (slow), 2011-04-27 14:50:02]

http://codereview.chromium.org/6893052/diff/1020/chrome/browser/chromeos/cust...
File chrome/browser/chromeos/customization_document.cc (right):

http://codereview.chromium.org/6893052/diff/1020/chrome/browser/chromeos/cust...
chrome/browser/chromeos/customization_document.cc:218:
local_state->RegisterBooleanPref(kServicesCustomizationAppliedPref, false);
FYI http://codereview.chromium.org/6905044/

[Dmitry Polukhin, 2011-04-27 14:56:15]

http://codereview.chromium.org/6893052/diff/1020/chrome/browser/chromeos/cust...
File chrome/browser/chromeos/customization_document.cc (right):

http://codereview.chromium.org/6893052/diff/1020/chrome/browser/chromeos/cust...
chrome/browser/chromeos/customization_document.cc:122: //
StartupCustomizationDocument implementation.
On 2011/04/27 13:57:07, Nikita Kostylev wrote:
> Please use this style here and trough the file:
> 
> // StartupCustomizationDocument implementation. -------------

Done.

http://codereview.chromium.org/6893052/diff/1020/chrome/browser/chromeos/cust...
chrome/browser/chromeos/customization_document.cc:135:
LoadManifestFromString(manifest);
On 2011/04/27 13:57:07, Nikita Kostylev wrote:
> Are thread restrictions applied during testing?

I think so. But in test we don't ready manifest from file, only from string it
is more convenient.

http://codereview.chromium.org/6893052/diff/1020/chrome/browser/chromeos/cust...
chrome/browser/chromeos/customization_document.cc:207:
LoadManifestFromString(manifest);
On 2011/04/27 13:57:07, Nikita Kostylev wrote:
> Instance enters into state when call on StartFetching() will start fetching
> empty URL.
> Could you at least check for empty URL in StartFetching()?

Added DCHECK(url_.is_valid());

http://codereview.chromium.org/6893052/diff/1020/chrome/browser/chromeos/cust...
File chrome/browser/chromeos/customization_document.h (right):

http://codereview.chromium.org/6893052/diff/1020/chrome/browser/chromeos/cust...
chrome/browser/chromeos/customization_document.h:11: #include "base/file_path.h"
On 2011/04/27 13:57:07, Nikita Kostylev wrote:
> Could FilePath be forward declared here?

Done.

http://codereview.chromium.org/6893052/diff/1020/chrome/browser/chromeos/cust...
chrome/browser/chromeos/customization_document.h:33: // Return true if the
document was successfully fetched and ready for apply.
On 2011/04/27 13:57:07, Nikita Kostylev wrote:
> nit: was successfully fetched and parsed.

Done.

http://codereview.chromium.org/6893052/diff/1020/chrome/browser/chromeos/cust...
chrome/browser/chromeos/customization_document.h:53: class
StartupCustomizationDocument : public CustomizationDocument {
On 2011/04/27 13:57:07, Nikita Kostylev wrote:
> Please add short comment on life-cycle.
> This document receives definite ready/not ready state right after instance is
> created.

Done.

http://codereview.chromium.org/6893052/diff/1020/chrome/browser/chromeos/cust...
chrome/browser/chromeos/customization_document.h:92: class
ServicesCustomizationDocument : public CustomizationDocument,
On 2011/04/27 13:57:07, Nikita Kostylev wrote:
> Please add short comment on life-cycle.
> This document receives needs to be fetched after created and only when it's
> fetched it gets proper ready/not ready state.

Done.

http://codereview.chromium.org/6893052/diff/1020/chrome/browser/chromeos/cust...
chrome/browser/chromeos/customization_document.h:102: static bool WasApplied();
On 2011/04/27 13:57:07, Nikita Kostylev wrote:
> Also expose WasFetched?

Was fetched itself is so interesting. The only interesting state IsReady.

http://codereview.chromium.org/6893052/diff/1020/chrome/browser/chromeos/cust...
chrome/browser/chromeos/customization_document.h:109: bool ApplyCustmization();
On 2011/04/27 13:57:07, Nikita Kostylev wrote:
> nit: Customization.

Done.

http://codereview.chromium.org/6893052/diff/1020/chrome/browser/chromeos/logi...
File chrome/browser/chromeos/login/base_login_display_host.cc (right):

http://codereview.chromium.org/6893052/diff/1020/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/base_login_display_host.cc:149: // Initiate
services customization.
On 2011/04/27 13:57:07, Nikita Kostylev wrote:
> Nit: should say "Initiate service customization manifest fetching."

Done.

[Nikita (slow), 2011-04-27 15:56:59]

LGTM
Please run linux_chromeos trybot.