BookmarksFunction sending response after execution in all cases

BookmarksFunction sending response after execution in all cases

On some occasions (most likely on big bookmarks models), BookmarksFunction
might be executed while bookmarks model is not yet ready. In this case we
must schedule the function for later execution (when model is ready) and
once function has executed we must SendResponse().

Adding SendResponse() in the above described scenario.

BUG=None

Committed: https://crrev.com/5e25a3281098f5ba5f73659c6732a7ac01a3068a
Cr-Commit-Position: refs/heads/master@{#300744}

[atanasb, 2014-10-15 14:16:12]

atanasb@opera.com changed reviewers:
+ jyasskin@chromium.org, tfarina@chromium.org

[atanasb, 2014-10-15 14:19:03]

Guys, does this patch looks good to you?

[tfarina, 2014-10-16 19:08:25]

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

[tfarina, 2014-10-16 19:08:26]

Redirecting this to Benjamin.

At first, it looks fine.

https://codereview.chromium.org/660513002/diff/1/chrome/browser/extensions/ap...
File chrome/browser/extensions/api/bookmarks/bookmarks_api.cc (right):

https://codereview.chromium.org/660513002/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/bookmarks/bookmarks_api.cc:226:
RunAndSendResponse();
I don't know if it is fine to send
extensions::NOTIFICATION_EXTENSION_BOOKMARKS_API_INVOKED here.

[not at google - send to devlin, 2014-10-16 19:40:21]

lgtm

https://codereview.chromium.org/660513002/diff/1/chrome/browser/extensions/ap...
File chrome/browser/extensions/api/bookmarks/bookmarks_api.cc (right):

https://codereview.chromium.org/660513002/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/bookmarks/bookmarks_api.cc:226:
RunAndSendResponse();
On 2014/10/16 19:08:25, tfarina wrote:
> I don't know if it is fine to send
> extensions::NOTIFICATION_EXTENSION_BOOKMARKS_API_INVOKED here.

Good point.

It looks like this notification is used by sync to do some accounting on
extension usage of bookmarks APIs. That implies that calling it here is the
right thing to do.

[atanasb, 2014-10-17 07:55:32]

atanasb@opera.com changed reviewers:
+ tim@chromium.org

[atanasb, 2014-10-21 21:21:25]

Thanks for the review kalman. Can you please suggest another reviewer for this
patch.

[not at google - send to devlin, 2014-10-21 21:23:43]

You don't need any other reviewers?

[atanasb, 2014-10-22 12:39:28]

The CQ bit was checked by atanasb@opera.com

[commit-bot: I haz the power, 2014-10-22 12:40:39]

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

[atanasb, 2014-10-22 12:40:58]

The CQ bit was unchecked by atanasb@opera.com

[atanasb, 2014-10-22 12:40:59]

The CQ bit was checked by atanasb@opera.com

[commit-bot: I haz the power, 2014-10-22 12:41:16]

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

[commit-bot: I haz the power, 2014-10-22 12:45:51]

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

[commit-bot: I haz the power, 2014-10-22 12:45:52]

Try jobs failed on following builders:
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/72001)
  android_aosp on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)
  android_chromium_gn_compile_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  linux_chromium_compile_dbg_32 on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_rel_device on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device/bu...)
  mac_chromium_compile_dbg on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[atanasb, 2014-10-22 14:36:13]

The CQ bit was checked by atanasb@opera.com

[commit-bot: I haz the power, 2014-10-22 14:37:14]

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

[commit-bot: I haz the power, 2014-10-22 17:06:02]

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

[commit-bot: I haz the power, 2014-10-22 17:06:04]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[atanasb, 2014-10-22 18:06:00]

The CQ bit was checked by atanasb@opera.com

[commit-bot: I haz the power, 2014-10-22 18:07:33]

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

[commit-bot: I haz the power, 2014-10-22 19:28:25]

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

[commit-bot: I haz the power, 2014-10-22 19:28:26]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[atanasb, 2014-10-22 19:45:53]

The CQ bit was checked by atanasb@opera.com

[commit-bot: I haz the power, 2014-10-22 19:47:19]

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

[commit-bot: I haz the power, 2014-10-22 20:25:58]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2014-10-22 20:28:32]

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