Refactor Chrome Now Card Dismissal Stack to use Promises

Refactor Chrome Now Card Dismissal Stack to use Promises

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

[skare_, 2014-03-07 01:57:51]

https://codereview.chromium.org/183883027/diff/1/chrome/browser/resources/goo...
File chrome/browser/resources/google_now/background.js (right):

https://codereview.chromium.org/183883027/diff/1/chrome/browser/resources/goo...
chrome/browser/resources/google_now/background.js:653:
processPendingDismissals().then(function() {
does
.then(requestNotificationCards)
work?

https://codereview.chromium.org/183883027/diff/1/chrome/browser/resources/goo...
chrome/browser/resources/google_now/background.js:717: return
fillFromChromeLocalStorage({
general comment on this pattern: this does make sense but a return statement
covering almost 50 lines was a little confusing on first read. Maybe cache in a
local?

I'll defer to a second opinion though.

[robliao, 2014-03-07 18:14:46]

https://codereview.chromium.org/183883027/diff/1/chrome/browser/resources/goo...
File chrome/browser/resources/google_now/background.js (right):

https://codereview.chromium.org/183883027/diff/1/chrome/browser/resources/goo...
chrome/browser/resources/google_now/background.js:653:
processPendingDismissals().then(function() {
Yup! That should work.
On 2014/03/07 01:57:51, Travis Skare wrote:
> does
> .then(requestNotificationCards)
> work?

https://codereview.chromium.org/183883027/diff/1/chrome/browser/resources/goo...
chrome/browser/resources/google_now/background.js:717: return
fillFromChromeLocalStorage({
Would we do the same if we were returning a very long function?
On 2014/03/07 01:57:51, Travis Skare wrote:
> general comment on this pattern: this does make sense but a return statement
> covering almost 50 lines was a little confusing on first read. Maybe cache in
a
> local?
> 
> I'll defer to a second opinion though.

[rgustafson, 2014-03-10 17:43:32]

lgtm

https://codereview.chromium.org/183883027/diff/1/chrome/browser/resources/goo...
File chrome/browser/resources/google_now/background.js (right):

https://codereview.chromium.org/183883027/diff/1/chrome/browser/resources/goo...
chrome/browser/resources/google_now/background.js:670: * @return {Promise} A
promise to request the card dimissal, rejects on error.
dismissal

https://codereview.chromium.org/183883027/diff/1/chrome/browser/resources/goo...
chrome/browser/resources/google_now/background.js:755:
dismissal.dismissalData.notificationId] =
move this onto previous line

[robliao, 2014-03-10 19:46:14]

https://codereview.chromium.org/183883027/diff/1/chrome/browser/resources/goo...
File chrome/browser/resources/google_now/background.js (right):

https://codereview.chromium.org/183883027/diff/1/chrome/browser/resources/goo...
chrome/browser/resources/google_now/background.js:670: * @return {Promise} A
promise to request the card dimissal, rejects on error.
On 2014/03/10 17:43:33, rgustafson wrote:
> dismissal

Done.

https://codereview.chromium.org/183883027/diff/1/chrome/browser/resources/goo...
chrome/browser/resources/google_now/background.js:755:
dismissal.dismissalData.notificationId] =
On 2014/03/10 17:43:33, rgustafson wrote:
> move this onto previous line

Done.

[skare_, 2014-03-11 00:07:29]

lgtm

[robliao, 2014-03-11 00:09:14]

vadimt: Please provide owner approval of this CL.

[vadimt, 2014-03-11 00:13:51]

lgtm

[robliao, 2014-03-11 00:24:54]

This CL will be submitted once https://codereview.chromium.org/187263002/ is
committed.

[robliao, 2014-03-13 21:00:31]

The CQ bit was checked by robliao@chromium.org

[robliao, 2014-03-13 21:00:34]

The CQ bit was unchecked by robliao@chromium.org

[robliao, 2014-03-13 21:34:59]

The CQ bit was checked by robliao@chromium.org

[commit-bot: I haz the power, 2014-03-13 21:36:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/robliao@chromium.org/183883027/40001

[commit-bot: I haz the power, 2014-03-13 21:48:07]

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

[commit-bot: I haz the power, 2014-03-13 21:48:08]

Try jobs failed on following builders:
  tryserver.chromium on win_chromium_compile_dbg

[robliao, 2014-03-13 21:51:36]

The CQ bit was checked by robliao@chromium.org

[commit-bot: I haz the power, 2014-03-13 21:52:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/robliao@chromium.org/183883027/40001

[commit-bot: I haz the power, 2014-03-14 00:21:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/robliao@chromium.org/183883027/40001

[commit-bot: I haz the power, 2014-03-14 06:07:46]

Change committed as 257025