[Sync] Move DataTypeStatusTable ownership into DataTypeManager.

[Sync] Move DataTypeStatusTable ownership into DataTypeManager.

The DataTypeManager now maintains its own status table, which it posts a copy
of on each configuration completion. This makes testing configuration results
easier as we can now just check the type status table, and makes the DTM
more self contained.

To make this work a HistoryDeleteDirectives datatype controller was added to
encapsulate the encryption dependency the type has. Additionally, the PSS
is now able to tell the DTM to reset type errors (e.g. when the user is attempting
to re-configure with or without certain types).

BUG=368834
Committed: https://crrev.com/fe2ae5fbf23243039bdc94f8f8672bc371f5339f
Cr-Commit-Position: refs/heads/master@{#292962}

[Nicolas Zea, 2014-08-29 23:20:42]

zea@chromium.org changed reviewers:
+ maniscalco@chromium.org

[Nicolas Zea, 2014-08-29 23:20:42]

PTAL

[maniscalco, 2014-08-29 23:50:36]

Just a few questions...

https://codereview.chromium.org/523043005/diff/60001/chrome/browser/sync/glue...
File chrome/browser/sync/glue/history_delete_directives_data_type_controller.cc
(right):

https://codereview.chromium.org/523043005/diff/60001/chrome/browser/sync/glue...
chrome/browser/sync/glue/history_delete_directives_data_type_controller.cc:30:
if (sync_service_->HasObserver(this))
Just curious, are you being cautious here by calling Has before Remove?  Or is
it strictly necessary?

https://codereview.chromium.org/523043005/diff/60001/chrome/browser/sync/glue...
chrome/browser/sync/glue/history_delete_directives_data_type_controller.cc:38:
void HistoryDeleteDirectivesDataTypeController::OnStateChanged() {
WDYT about adding DCHECKs to ensure this class is called on the right thread? 
For my own edification, is this always called on the UI thread?

https://codereview.chromium.org/523043005/diff/60001/components/sync_driver/d...
File components/sync_driver/data_type_manager_impl.h (right):

https://codereview.chromium.org/523043005/diff/60001/components/sync_driver/d...
components/sync_driver/data_type_manager_impl.h:81: DataTypeStatusTable
data_type_status_table() const {
I looked, but didn't see where this was called.  Did I miss it?

[Nicolas Zea, 2014-08-29 23:56:22]

PTAL

https://codereview.chromium.org/523043005/diff/60001/chrome/browser/sync/glue...
File chrome/browser/sync/glue/history_delete_directives_data_type_controller.cc
(right):

https://codereview.chromium.org/523043005/diff/60001/chrome/browser/sync/glue...
chrome/browser/sync/glue/history_delete_directives_data_type_controller.cc:30:
if (sync_service_->HasObserver(this))
On 2014/08/29 23:50:35, maniscalco wrote:
> Just curious, are you being cautious here by calling Has before Remove?  Or is
> it strictly necessary?

Ah, this is a leftover of when I was doing another remove earlier. This is
unnecessary at this point. Removed.

https://codereview.chromium.org/523043005/diff/60001/chrome/browser/sync/glue...
chrome/browser/sync/glue/history_delete_directives_data_type_controller.cc:38:
void HistoryDeleteDirectivesDataTypeController::OnStateChanged() {
On 2014/08/29 23:50:35, maniscalco wrote:
> WDYT about adding DCHECKs to ensure this class is called on the right thread? 
> For my own edification, is this always called on the UI thread?

Correct. It inherits from UIDataTypeController and the ProfileSyncService only
lives on the UI thread, so all work should happen there. Given that it's an
internal class though, and neither of those two classes are involve multiple
threads, it seems unnecessary to add thread safety checks here.

https://codereview.chromium.org/523043005/diff/60001/components/sync_driver/d...
File components/sync_driver/data_type_manager_impl.h (right):

https://codereview.chromium.org/523043005/diff/60001/components/sync_driver/d...
components/sync_driver/data_type_manager_impl.h:81: DataTypeStatusTable
data_type_status_table() const {
On 2014/08/29 23:50:36, maniscalco wrote:
> I looked, but didn't see where this was called.  Did I miss it?

Woops, I had removed this in a different commit, but that change fell through
some rebasing. Removed.

[maniscalco, 2014-09-02 16:15:50]

lgtm

[Nicolas Zea, 2014-09-02 16:25:13]

The CQ bit was checked by zea@chromium.org

[commit-bot: I haz the power, 2014-09-02 16:25:57]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/zea@chromium.org/523043005/100001

[commit-bot: I haz the power, 2014-09-02 17:23:32]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[commit-bot: I haz the power, 2014-09-02 17:52:19]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-09-02 17:52:20]

Try jobs failed on following builders:
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Nicolas Zea, 2014-09-02 17:54:18]

The CQ bit was checked by zea@chromium.org

[commit-bot: I haz the power, 2014-09-02 17:56:04]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/zea@chromium.org/523043005/100001

[commit-bot: I haz the power, 2014-09-02 18:31:33]

Committed patchset #4 (id:100001) as 44aead400e988dacb6f7146a378d764309b6367a

[commit-bot: I haz the power, 2014-09-10 03:19:35]

Patchset 4 (id:??) landed as
https://crrev.com/fe2ae5fbf23243039bdc94f8f8672bc371f5339f
Cr-Commit-Position: refs/heads/master@{#292962}