Several changes to the benchmark extension:...

Several changes to the benchmark extension:

* Cleaned up the UI
   - can auto-start a benchmark from a given url
* added bandwidth measurements for testing bandwidth efficiency
* added time to docload, time to first paint, in addition to overall PLT.

BUG=none
TEST=none


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

[Erik does not do reviews, 2009-07-17 17:15:52]

LGTM - just some code you can remove

http://codereview.chromium.org/155675/diff/9/1002
File chrome/test/data/extensions/samples/benchmark/toolstrip.html (right):

http://codereview.chromium.org/155675/diff/9/1002#newcode42
Line 42: // This functionality is a bit racey, probably due to the extension
API.
Is this still racey the way you've implemented it?  Like optionsForm will always
get created before the getSelected callback completed.

http://codereview.chromium.org/155675/diff/9/1002#newcode45
Line 45: // If thwe open the window before we call getSelected(), then the
selected
thwe -> we

http://codereview.chromium.org/155675/diff/9/1002#newcode51
Line 51: chrome.tabs.getSelected(windowId, function(tab) {
windowId is an optional parameter.  If you don't provide it, it will default to
the current window id which is what you want here.  The docs talk about this a
little:
http://dev.chromium.org/developers/design-documents/extensions/tabs-api

http://codereview.chromium.org/155675/diff/9/1002#newcode197
Line 197: chrome.windows.getCurrent(function(currentWindow) {
so you don't need this...

[mbelshe, 2009-07-17 21:29:38]

On Fri, Jul 17, 2009 at 10:15 AM, <erikkay@chromium.org> wrote:

> LGTM - just some code you can remove
>
>
>
> http://codereview.chromium.org/155675/diff/9/1002
> File chrome/test/data/extensions/samples/benchmark/toolstrip.html
> (right):
>
> http://codereview.chromium.org/155675/diff/9/1002#newcode42
> Line 42: // This functionality is a bit racey, probably due to the
> extension API.
> Is this still racey the way you've implemented it?  Like optionsForm
> will always get created before the getSelected callback completed.
>

It's okay - reworded to "fragile" instead of "racey".  It's tricky to do
this code in an order which works.

>
> http://codereview.chromium.org/155675/diff/9/1002#newcode45
> Line 45: // If thwe open the window before we call getSelected(), then
> the selected
> thwe -> we


thanks


>
>
> http://codereview.chromium.org/155675/diff/9/1002#newcode51
> Line 51: chrome.tabs.getSelected(windowId, function(tab) {
> windowId is an optional parameter.  If you don't provide it, it will
> default to the current window id which is what you want here.  The docs
> talk about this a little:
> http://dev.chromium.org/developers/design-documents/extensions/tabs-api


That doesn't seem to work; I don't know why.


> <http://dev.chromium.org/developers/design-documents/extensions/tabs-api>
>
> http://codereview.chromium.org/155675/diff/9/1002#newcode197
> Line 197: chrome.windows.getCurrent(function(currentWindow) {
> so you don't need this...
>
>
> http://codereview.chromium.org/155675
>

[Erik does not do reviews, 2009-07-17 22:43:37]

It sounds like rafael helped out here.  I didn't realize this either, but the
first argument should be null, not just omitted.