Reland 201932: Add API function chrome.notifications.getAll

Reland 201932: Add API function chrome.notifications.getAll

This function returns an object whose keys are the notification
IDs of all notifications created by that extension.

BUG=240924
TBR=miket@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=203001

[dewittj, 2013-05-24 21:43:42]

jianli, would you mind reviewing notification_ui_manager_mac.*

The code that landed in 201932 is patchset 1, so you can just use the
differences to see what's new.

[jianli, 2013-05-24 21:51:20]

https://codereview.chromium.org/15715008/diff/13001/chrome/browser/notificati...
File chrome/browser/notifications/notification_ui_manager_mac.mm (right):

https://codereview.chromium.org/15715008/diff/13001/chrome/browser/notificati...
chrome/browser/notifications/notification_ui_manager_mac.mm:167: Profile*
profile,
nit: 4-space indentation

https://codereview.chromium.org/15715008/diff/13001/chrome/browser/notificati...
chrome/browser/notifications/notification_ui_manager_mac.mm:174: it !=
notification_map_.end(); it++) {
nit: ++it

https://codereview.chromium.org/15715008/diff/13001/chrome/browser/notificati...
chrome/browser/notifications/notification_ui_manager_mac.mm:175: if
(it->second->model->origin_url() == source_origin &&
It might be better to use a temp variable to hold it->second since it is used in
multiple places.

https://codereview.chromium.org/15715008/diff/13001/chrome/browser/notificati...
chrome/browser/notifications/notification_ui_manager_mac.mm:177: //
RemoveNotification will erase from the map, invalidating iterator
Not sure I understand this comment. RemoveNotification is not called in this
method.

https://codereview.chromium.org/15715008/diff/13001/chrome/browser/notificati...
chrome/browser/notifications/notification_ui_manager_mac.mm:184: }
nit: empty line

[dewittj, 2013-05-24 23:03:22]

thanks, ptal.

https://codereview.chromium.org/15715008/diff/13001/chrome/browser/notificati...
File chrome/browser/notifications/notification_ui_manager_mac.mm (right):

https://codereview.chromium.org/15715008/diff/13001/chrome/browser/notificati...
chrome/browser/notifications/notification_ui_manager_mac.mm:167: Profile*
profile,
On 2013/05/24 21:51:20, jianli wrote:
> nit: 4-space indentation

wow, clang-format-diff has made me lazy.  Done.

https://codereview.chromium.org/15715008/diff/13001/chrome/browser/notificati...
chrome/browser/notifications/notification_ui_manager_mac.mm:174: it !=
notification_map_.end(); it++) {
On 2013/05/24 21:51:20, jianli wrote:
> nit: ++it

Done.

https://codereview.chromium.org/15715008/diff/13001/chrome/browser/notificati...
chrome/browser/notifications/notification_ui_manager_mac.mm:175: if
(it->second->model->origin_url() == source_origin &&
On 2013/05/24 21:51:20, jianli wrote:
> It might be better to use a temp variable to hold it->second since it is used
in
> multiple places.

Done.

https://codereview.chromium.org/15715008/diff/13001/chrome/browser/notificati...
chrome/browser/notifications/notification_ui_manager_mac.mm:177: //
RemoveNotification will erase from the map, invalidating iterator
On 2013/05/24 21:51:20, jianli wrote:
> Not sure I understand this comment. RemoveNotification is not called in this
> method.

Done.

https://codereview.chromium.org/15715008/diff/13001/chrome/browser/notificati...
chrome/browser/notifications/notification_ui_manager_mac.mm:184: }
On 2013/05/24 21:51:20, jianli wrote:
> nit: empty line

Done.

[jianli, 2013-05-24 23:09:00]

lgtm

[commit-bot: I haz the power, 2013-05-24 23:20:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dewittj@chromium.org/15715008/24017

[commit-bot: I haz the power, 2013-05-25 01:24:06]

Retried try job too often on linux_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2013-05-28 18:57:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dewittj@chromium.org/15715008/51001

[commit-bot: I haz the power, 2013-05-28 18:58:02]

Failed to apply patch for
chrome/browser/extensions/extension_function_histogram_value.h:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/extensions/extension_function_histogram_value.h
  Hunk #1 FAILED at 530.
  1 out of 1 hunk FAILED -- saving rejects to file
chrome/browser/extensions/extension_function_histogram_value.h.rej

Patch:       chrome/browser/extensions/extension_function_histogram_value.h
Index: chrome/browser/extensions/extension_function_histogram_value.h
diff --git a/chrome/browser/extensions/extension_function_histogram_value.h
b/chrome/browser/extensions/extension_function_histogram_value.h
index
44b709f0766fa633a1ec5c4c1a8675a6d7b9d982..b1c7f055e1962623ae1af6baffee84d14e2db344
100644
--- a/chrome/browser/extensions/extension_function_histogram_value.h
+++ b/chrome/browser/extensions/extension_function_histogram_value.h
@@ -530,6 +530,7 @@ enum HistogramValue {
   WEBVIEW_INSERTCSS,
   METRICSPRIVATE_GETISCRASHRECORDINGENABLED,
   IDENTITYPRIVATE_GETRESOURCES,
+  NOTIFICATIONS_GET_ALL,
   ENUM_BOUNDARY // Last entry: Add new entries above.
 };

[commit-bot: I haz the power, 2013-05-28 19:53:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dewittj@chromium.org/15715008/55002

[commit-bot: I haz the power, 2013-05-28 20:39:26]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2013-05-28 20:41:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dewittj@chromium.org/15715008/55002

[commit-bot: I haz the power, 2013-05-28 21:09:24]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2013-05-28 23:12:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dewittj@chromium.org/15715008/55002

[commit-bot: I haz the power, 2013-05-29 00:38:06]

Change committed as 202716

[Dan Beam, 2013-05-29 03:58:41]

sorry for the revert[1]. you can try your change ahead of time to see if it'll
break on the bots with

  $ git try -b mac_asan

next time around, btw (hope that helps!).

[1] http://src.chromium.org/viewvc/chrome?revision=202764&view=revision

[Dan Beam, 2013-05-29 04:33:54]

also, this may be a fix for the issue -
https://codereview.chromium.org/16094012/

[dewittj, 2013-05-29 20:13:10]

Looks like some files got left off between patchset 4 and 5, causing the test
failure to happen again.  Got a green for that test on mac_asan, so committing
again.

[commit-bot: I haz the power, 2013-05-29 20:13:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dewittj@chromium.org/15715008/88001

[commit-bot: I haz the power, 2013-05-29 23:13:41]

Change committed as 203001