CPM Extension Uninstall Watching

CPM Extension Uninstall Watching

Add in watching for extension uninstall events. This also changes
NOTIFICATION_EXTENSION_UNINSTALL to have the Extension as details.

BUG=130212
TEST=Included browsertest; existing tests to test for regression on the notification change.


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

[Devlin, 2012-07-03 20:17:31]

Yoyo,

When you get a chance, would appreciate it if you could look this over.

[Yoyo Zhou, 2012-07-09 17:06:36]

LGTM, just a few minor comments.

http://codereview.chromium.org/10694056/diff/1/chrome/browser/extensions/app_...
File chrome/browser/extensions/app_notification_manager_unittest.cc (right):

http://codereview.chromium.org/10694056/diff/1/chrome/browser/extensions/app_...
chrome/browser/extensions/app_notification_manager_unittest.cc:111:
"pdedgpfalcphpbgbdknbecnlmobnceia");
nit: I would expect "id1" to still work here. But if it doesn't, then this is
fine.

http://codereview.chromium.org/10694056/diff/1/chrome/browser/extensions/stat...
File chrome/browser/extensions/state_store.cc (right):

http://codereview.chromium.org/10694056/diff/1/chrome/browser/extensions/stat...
chrome/browser/extensions/state_store.cc:68: case
chrome::NOTIFICATION_EXTENSION_UNINSTALLED:
Should collapse this with the previous case.

http://codereview.chromium.org/10694056/diff/1/chrome/common/chrome_notificat...
File chrome/common/chrome_notification_types.h (right):

http://codereview.chromium.org/10694056/diff/1/chrome/common/chrome_notificat...
chrome/common/chrome_notification_types.h:496: // Sent when an extension has
been uninstalled. The details are the extension
Might as well use the "details are an Extension, and the source is a Profile"
language from the adjacent ones.

http://codereview.chromium.org/10694056/diff/1/chrome/common/extensions/exten...
File chrome/common/extensions/extension_test_util.cc (right):

http://codereview.chromium.org/10694056/diff/1/chrome/common/extensions/exten...
chrome/common/extensions/extension_test_util.cc:30:
Extension::Create(FilePath(), Extension::INTERNAL, values,
This is not needed.

http://codereview.chromium.org/10694056/diff/1/chrome/common/extensions/exten...
File chrome/common/extensions/extension_test_util.h (right):

http://codereview.chromium.org/10694056/diff/1/chrome/common/extensions/exten...
chrome/common/extensions/extension_test_util.h:22: // Return a very simple
extension; |id| can be empty if the id is not important.
I can't see why the empty id case would be necessary. Especially since you're
not actually using that here, I would just not do it, and call this function
CreateExtensionWithId. It'd be better just to say that it will have the id |id|.

[Devlin, 2012-07-09 20:48:26]

https://chromiumcodereview.appspot.com/10694056/diff/1/chrome/browser/extensi...
File chrome/browser/extensions/app_notification_manager_unittest.cc (right):

https://chromiumcodereview.appspot.com/10694056/diff/1/chrome/browser/extensi...
chrome/browser/extensions/app_notification_manager_unittest.cc:111:
"pdedgpfalcphpbgbdknbecnlmobnceia");
On 2012/07/09 17:06:37, Yoyo Zhou wrote:
> nit: I would expect "id1" to still work here. But if it doesn't, then this is
> fine.

Done.

https://chromiumcodereview.appspot.com/10694056/diff/1/chrome/browser/extensi...
File chrome/browser/extensions/state_store.cc (right):

https://chromiumcodereview.appspot.com/10694056/diff/1/chrome/browser/extensi...
chrome/browser/extensions/state_store.cc:68: case
chrome::NOTIFICATION_EXTENSION_UNINSTALLED:
On 2012/07/09 17:06:37, Yoyo Zhou wrote:
> Should collapse this with the previous case.

Done.

https://chromiumcodereview.appspot.com/10694056/diff/1/chrome/common/chrome_n...
File chrome/common/chrome_notification_types.h (right):

https://chromiumcodereview.appspot.com/10694056/diff/1/chrome/common/chrome_n...
chrome/common/chrome_notification_types.h:496: // Sent when an extension has
been uninstalled. The details are the extension
On 2012/07/09 17:06:37, Yoyo Zhou wrote:
> Might as well use the "details are an Extension, and the source is a Profile"
> language from the adjacent ones.

Done.

https://chromiumcodereview.appspot.com/10694056/diff/1/chrome/common/extensio...
File chrome/common/extensions/extension_test_util.cc (right):

https://chromiumcodereview.appspot.com/10694056/diff/1/chrome/common/extensio...
chrome/common/extensions/extension_test_util.cc:30:
Extension::Create(FilePath(), Extension::INTERNAL, values,
On 2012/07/09 17:06:37, Yoyo Zhou wrote:
> This is not needed.

Done.

https://chromiumcodereview.appspot.com/10694056/diff/1/chrome/common/extensio...
File chrome/common/extensions/extension_test_util.h (right):

https://chromiumcodereview.appspot.com/10694056/diff/1/chrome/common/extensio...
chrome/common/extensions/extension_test_util.h:22: // Return a very simple
extension; |id| can be empty if the id is not important.
On 2012/07/09 17:06:37, Yoyo Zhou wrote:
> I can't see why the empty id case would be necessary. Especially since you're
> not actually using that here, I would just not do it, and call this function
> CreateExtensionWithId. It'd be better just to say that it will have the id
|id|.

Done.

[Yoyo Zhou, 2012-07-10 20:44:41]

still lgtm

https://chromiumcodereview.appspot.com/10694056/diff/6002/chrome/common/chrom...
File chrome/common/chrome_notification_types.h (right):

https://chromiumcodereview.appspot.com/10694056/diff/6002/chrome/common/chrom...
chrome/common/chrome_notification_types.h:496: // Sent when an extension has
been uninstalled. The details are an extension,
nit: please match capitalization (Extension/Profile)

[commit-bot: I haz the power, 2012-07-17 16:04:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rdevlin.cronin@chromium.org/10694056/2...

[commit-bot: I haz the power, 2012-07-17 17:19:44]

Change committed as 147025