Don't crash when trying to create an ExtensionSyncData from invalid SyncData.

Don't crash when trying to create an ExtensionSyncData from invalid SyncData.

Instead, ignore such invalid inputs from sync.

BUG=359210
Committed: https://crrev.com/8704445335ddc83ec6099f83b6bc5c9e83416845
Cr-Commit-Position: refs/heads/master@{#320330}

[Yoyo Zhou, 2015-03-11 23:05:59]

yoz@chromium.org changed reviewers:
+ kalman@chromium.org

[Yoyo Zhou, 2015-03-11 23:08:40]

Patchset #1 (id:1) has been deleted

[not at google - send to devlin, 2015-03-11 23:21:35]

Only significant comment is about the UMA.

https://chromiumcodereview.appspot.com/996213005/diff/20001/chrome/browser/ap...
File chrome/browser/apps/ephemeral_app_browsertest.cc (right):

https://chromiumcodereview.appspot.com/996213005/diff/20001/chrome/browser/ap...
chrome/browser/apps/ephemeral_app_browsertest.cc:537: if (data->id() == id)
It could be null, though?

https://chromiumcodereview.appspot.com/996213005/diff/20001/chrome/browser/ex...
File chrome/browser/extensions/app_sync_bundle.cc (right):

https://chromiumcodereview.appspot.com/996213005/diff/20001/chrome/browser/ex...
chrome/browser/extensions/app_sync_bundle.cc:37: if (app_sync_data.get()) {
FYI .get() not needed, but leave it this way or don't, doesn't bother me.

https://chromiumcodereview.appspot.com/996213005/diff/20001/chrome/browser/ex...
File chrome/browser/extensions/app_sync_data.h (right):

https://chromiumcodereview.appspot.com/996213005/diff/20001/chrome/browser/ex...
chrome/browser/extensions/app_sync_data.h:42: // For constructing an AppSyncData
from received sync data.
Mention these may be null (because...)

https://chromiumcodereview.appspot.com/996213005/diff/20001/chrome/browser/ex...
File chrome/browser/extensions/extension_sync_data.cc (right):

https://chromiumcodereview.appspot.com/996213005/diff/20001/chrome/browser/ex...
chrome/browser/extensions/extension_sync_data.cc:113: LOG(ERROR) << "Attempt to
sync bad ExtensionSpecifics (bad ID):\n"
Can we track this in UMA, with an enum for why (bad ID, bad version, etc)?

https://chromiumcodereview.appspot.com/996213005/diff/20001/chrome/browser/ex...
chrome/browser/extensions/extension_sync_data.cc:162: } else {
no else after return

https://chromiumcodereview.appspot.com/996213005/diff/20001/chrome/browser/ex...
File chrome/browser/extensions/extension_sync_data.h (right):

https://chromiumcodereview.appspot.com/996213005/diff/20001/chrome/browser/ex...
chrome/browser/extensions/extension_sync_data.h:44: // For constructing an
ExtensionSyncData from received sync data.
Same comment as before.

[Yoyo Zhou, 2015-03-12 00:25:48]

yoz@chromium.org changed reviewers:
+ isherman@chromium.org

[Yoyo Zhou, 2015-03-12 00:25:49]

isherman: can you review histograms.xml?

https://chromiumcodereview.appspot.com/996213005/diff/20001/chrome/browser/ap...
File chrome/browser/apps/ephemeral_app_browsertest.cc (right):

https://chromiumcodereview.appspot.com/996213005/diff/20001/chrome/browser/ap...
chrome/browser/apps/ephemeral_app_browsertest.cc:537: if (data->id() == id)
On 2015/03/11 23:21:34, kalman wrote:
> It could be null, though?

Done.

https://chromiumcodereview.appspot.com/996213005/diff/20001/chrome/browser/ex...
File chrome/browser/extensions/app_sync_bundle.cc (right):

https://chromiumcodereview.appspot.com/996213005/diff/20001/chrome/browser/ex...
chrome/browser/extensions/app_sync_bundle.cc:37: if (app_sync_data.get()) {
On 2015/03/11 23:21:34, kalman wrote:
> FYI .get() not needed, but leave it this way or don't, doesn't bother me.

I'll leave it, I think.

https://chromiumcodereview.appspot.com/996213005/diff/20001/chrome/browser/ex...
File chrome/browser/extensions/app_sync_data.h (right):

https://chromiumcodereview.appspot.com/996213005/diff/20001/chrome/browser/ex...
chrome/browser/extensions/app_sync_data.h:42: // For constructing an AppSyncData
from received sync data.
On 2015/03/11 23:21:34, kalman wrote:
> Mention these may be null (because...)

Ok. Also changed to not use the implicit constructor from null.

https://chromiumcodereview.appspot.com/996213005/diff/20001/chrome/browser/ex...
File chrome/browser/extensions/extension_sync_data.cc (right):

https://chromiumcodereview.appspot.com/996213005/diff/20001/chrome/browser/ex...
chrome/browser/extensions/extension_sync_data.cc:113: LOG(ERROR) << "Attempt to
sync bad ExtensionSpecifics (bad ID):\n"
On 2015/03/11 23:21:34, kalman wrote:
> Can we track this in UMA, with an enum for why (bad ID, bad version, etc)?

Done.

https://chromiumcodereview.appspot.com/996213005/diff/20001/chrome/browser/ex...
chrome/browser/extensions/extension_sync_data.cc:162: } else {
On 2015/03/11 23:21:34, kalman wrote:
> no else after return

Done.

https://chromiumcodereview.appspot.com/996213005/diff/20001/chrome/browser/ex...
File chrome/browser/extensions/extension_sync_data.h (right):

https://chromiumcodereview.appspot.com/996213005/diff/20001/chrome/browser/ex...
chrome/browser/extensions/extension_sync_data.h:44: // For constructing an
ExtensionSyncData from received sync data.
On 2015/03/11 23:21:34, kalman wrote:
> Same comment as before.

Done.

[Ilya Sherman, 2015-03-12 02:14:21]

histograms.xml lgtm

[not at google - send to devlin, 2015-03-12 16:38:42]

lgtm

[Yoyo Zhou, 2015-03-12 16:53:43]

The CQ bit was checked by yoz@chromium.org

[commit-bot: I haz the power, 2015-03-12 16:54:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/996213005/40001

[commit-bot: I haz the power, 2015-03-12 18:43:00]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2015-03-12 18:43:42]

Patchset 2 (id:??) landed as
https://crrev.com/8704445335ddc83ec6099f83b6bc5c9e83416845
Cr-Commit-Position: refs/heads/master@{#320330}