Issue 21729: Add new event chrome.tabs.onStatusChange .

Issue 21729: Add new event chrome.tabs.onStatusChange .

BUG=21729
TEST= Manually tested that onUpdated sends all tab info.  Updated unit tests.

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

[skerner, 2009-10-16 20:11:55]

This change needs a unit test.  The unit tests for tabs are windows only at the
moment, and I am working on getting a windows box to build chromium.  While I do
that, please take a look at the change so far.

Thanks,
   Sam

[Aaron Boodman, 2009-10-17 00:56:47]

One major thing that looks off here is that the change shows the onStatusChange
event being _deleted_ not added. I guess you uploaded a reverse diff?

[Aaron Boodman, 2009-10-17 00:58:22]

Another thing, in the code review description, the convention is something like:

Add new event chrome.tabs.onStatusChange

BUG=21729
TEST=<any manual testing that could be used to verify this change>

The bug line is especially important as it is used by our automated tools to
update the bug.

[Aaron Boodman, 2009-10-17 01:02:31]

Other than that, the change looks pretty good to me.

[rafaelw, 2009-10-20 20:02:39]

per irc discussion, remove this new event and change tabs.onUpdated to have a
third parameter which is the |tab| object.

[Sam Kerner (Chrome), 2009-10-26 21:56:47]

Per IRC discussion, this change now adds a new tab argument to the
chrome.tabs.onUpdated event callback.  Ready for another look.

Sam

[rafaelw, 2009-10-27 18:48:01]

lgtm. thanks for doing this.

http://codereview.chromium.org/289006/diff/9001/10007
File chrome/test/data/extensions/api_test/tabs/test.js (right):

http://codereview.chromium.org/289006/diff/9001/10007#newcode298
Line 298: pass());

looks like this will fit on previous line

http://codereview.chromium.org/289006/diff/9001/10007#newcode303
Line 303: chrome.tabs.onUpdated,
fix indent level here.

http://codereview.chromium.org/289006/diff/9001/10007#newcode322
Line 322: pass());
looks like this will fit on previous line

[Sam Kerner (Chrome), 2009-10-27 21:25:54]

http://codereview.chromium.org/289006/diff/9001/10007
File chrome/test/data/extensions/api_test/tabs/test.js (right):

http://codereview.chromium.org/289006/diff/9001/10007#newcode298
Line 298: pass());
On 2009/10/27 18:48:01, rafaelw wrote:
> 
> looks like this will fit on previous line

Done.

http://codereview.chromium.org/289006/diff/9001/10007#newcode303
Line 303: chrome.tabs.onUpdated,
On 2009/10/27 18:48:01, rafaelw wrote:
> fix indent level here.

Done.

http://codereview.chromium.org/289006/diff/9001/10007#newcode322
Line 322: pass());
On 2009/10/27 18:48:01, rafaelw wrote:
> looks like this will fit on previous line

Done.

[Aaron Boodman, 2009-10-28 06:09:27]

fwiw, Sam, the convention is that when you get an 'lgtm' plus some comments it
means 'Please make these changes, then submit.' You don't need to get a
re-review.

Not sure if Raf mentioned that to you.

[Sam Kerner (Chrome), 2009-10-28 15:09:40]

On 2009/10/28 06:09:27, Aaron Boodman wrote:
> fwiw, Sam, the convention is that when you get an 'lgtm' plus some comments it
> means 'Please make these changes, then submit.' You don't need to get a
> re-review.
> 
> Not sure if Raf mentioned that to you.

Aaron,
   Thanks for the clarification.  Submitted as revision 30341.

Sam