Query the current window with chrome.tabs.query using -1.

Query the current window with chrome.tabs.query using -1.

This makes querying for tabs in the current window easier by requiring less callbacks.

BUG=108942
TEST=ExtensionApiTest.TabQuery


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

[asargent_no_longer_on_chrome, 2012-01-09 22:59:26]

C++ code seems fine.

One nit on the js API side - I think most of our other APIs have used null
instead of a constant like -1. Maybe you should check with Aaron about what his
preference is for the style here.

[jstritar, 2012-01-09 23:23:53]

+aa

I went with -1 because we can't distinguish between null and undefined on the
C++ side (we match any window IDs when it's not present).

[mihaip_google.com, 2012-01-09 23:31:20]

We should probably have a constant the way there
is chrome.windows.WINDOW_ID_NONE (
http://code.google.com/chrome/extensions/dev/windows.html#property-WINDOW_ID_...)
for indicating the current window.

Mihai

On Mon, Jan 9, 2012 at 3:23 PM, <jstritar@chromium.org> wrote:

> +aa
>
> I went with -1 because we can't distinguish between null and undefined on
> the
> C++ side (we match any window IDs when it's not present).
>
>
http://codereview.chromium.**org/9162002/<http://codereview.chromium.org/9162...
>

[jstritar, 2012-01-10 00:32:04]

On 2012/01/09 23:31:20, mihaip wrote:
> We should probably have a constant the way there
> is chrome.windows.WINDOW_ID_NONE (
>
http://code.google.com/chrome/extensions/dev/windows.html#property-WINDOW_ID_...)
> for indicating the current window.
> 
> Mihai

Nice, didn't know about that. I'll probably need to make sure the other methods
support that constant too then.

[jstritar, 2012-01-10 19:02:43]

I updated the query method to use chrome.windows.WINDOW_ID_CURRENT.

Many of the existing methods don't support WINDOW_ID_NONE, so I didn't do those
yet. If it's okay with you, I can follow up with another CL that adds support
for WINDOW_ID_NONE and WINDOW_ID_CURRENT to the other methods.

[asargent_no_longer_on_chrome, 2012-01-10 20:06:46]

LGTM w/ one nit

http://codereview.chromium.org/9162002/diff/7001/chrome/browser/extensions/ex...
File chrome/browser/extensions/extension_tabs_module.cc (right):

http://codereview.chromium.org/9162002/diff/7001/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_tabs_module.cc:785: int window_id =
extension_misc::kCurrentWindowId - 1;
nit: can you use whatever c++ constant corresponds to WINDOW_ID_NONE here?

[jstritar, 2012-01-10 20:29:22]

FYI: created http://crbug.com/109776 for making the APIs use those window
constants consistently

http://codereview.chromium.org/9162002/diff/7001/chrome/browser/extensions/ex...
File chrome/browser/extensions/extension_tabs_module.cc (right):

http://codereview.chromium.org/9162002/diff/7001/chrome/browser/extensions/ex...
chrome/browser/extensions/extension_tabs_module.cc:785: int window_id =
extension_misc::kCurrentWindowId - 1;
On 2012/01/10 20:06:46, Antony Sargent wrote:
> nit: can you use whatever c++ constant corresponds to WINDOW_ID_NONE here?
> 

Ah, much better... done.

[commit-bot: I haz the power, 2012-01-10 21:05:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jstritar@chromium.org/9162002/14001

[commit-bot: I haz the power, 2012-01-10 21:05:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jstritar@chromium.org/9162002/14001

[commit-bot: I haz the power, 2012-01-10 23:21:27]

Change committed as 117107