Remove tab_list_backend.Reset.

Remove tab_list_backend.Reset.

tab_list_backend._UpdateTabList() does the right thing, Reset is not necessary. Also, Reset ends up calling _UpdateTabList and then setting _tab_list to []. Then _UpdateTabList gets called again and _tab_list is correctly updated.

BUG=177372
TEST=run_tests --browser=cros-chrome --remote=172.22.71.22 testNewCloseTab
NOTRY=true

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

[achuithb, 2013-02-23 01:21:38]

Glenn, Dave: Could you please take a look?

[dtu, 2013-02-23 01:47:55]

Makes sense. I think crbug.com/166886 is fixed.

https://codereview.chromium.org/12319081/diff/1/tools/telemetry/telemetry/cor...
File tools/telemetry/telemetry/core/chrome/cros_browser_backend.py (right):

https://codereview.chromium.org/12319081/diff/1/tools/telemetry/telemetry/cor...
tools/telemetry/telemetry/core/chrome/cros_browser_backend.py:96: #
TODO(hartmanng): crbug.com/166886 (Remove these temporary hacks when
Remove this too.

[dtu, 2013-02-23 01:50:19]

https://codereview.chromium.org/12319081/diff/1/tools/telemetry/telemetry/cor...
File tools/telemetry/telemetry/core/chrome/tab_list_backend.py (right):

https://codereview.chromium.org/12319081/diff/1/tools/telemetry/telemetry/cor...
tools/telemetry/telemetry/core/chrome/tab_list_backend.py:76: return None
This one too.

[achuithb, 2013-02-23 02:03:20]

PTAL, Dave.

https://codereview.chromium.org/12319081/diff/1/tools/telemetry/telemetry/cor...
File tools/telemetry/telemetry/core/chrome/cros_browser_backend.py (right):

https://codereview.chromium.org/12319081/diff/1/tools/telemetry/telemetry/cor...
tools/telemetry/telemetry/core/chrome/cros_browser_backend.py:96: #
TODO(hartmanng): crbug.com/166886 (Remove these temporary hacks when
On 2013/02/23 01:47:55, Dave Tu wrote:
> Remove this too.

Done.

https://codereview.chromium.org/12319081/diff/1/tools/telemetry/telemetry/cor...
File tools/telemetry/telemetry/core/chrome/tab_list_backend.py (right):

https://codereview.chromium.org/12319081/diff/1/tools/telemetry/telemetry/cor...
tools/telemetry/telemetry/core/chrome/tab_list_backend.py:76: return None
On 2013/02/23 01:50:19, Dave Tu wrote:
> This one too.

I've removed the TODO, but don't think it's safe to add the assert. I don't
think it's safe to assume that tab_info will be found, right?

[dtu, 2013-02-23 02:13:04]

https://codereview.chromium.org/12319081/diff/1/tools/telemetry/telemetry/cor...
File tools/telemetry/telemetry/core/chrome/tab_list_backend.py (right):

https://codereview.chromium.org/12319081/diff/1/tools/telemetry/telemetry/cor...
tools/telemetry/telemetry/core/chrome/tab_list_backend.py:76: return None
On 2013/02/23 02:03:20, achuith.bhandarkar wrote:
> On 2013/02/23 01:50:19, Dave Tu wrote:
> > This one too.
> 
> I've removed the TODO, but don't think it's safe to add the assert. I don't
> think it's safe to assume that tab_info will be found, right?

Ah, yeah, you're right, that's still a problem. I guess the comment should stay.

[achuithb, 2013-02-23 02:23:38]

https://codereview.chromium.org/12319081/diff/1/tools/telemetry/telemetry/cor...
File tools/telemetry/telemetry/core/chrome/tab_list_backend.py (right):

https://codereview.chromium.org/12319081/diff/1/tools/telemetry/telemetry/cor...
tools/telemetry/telemetry/core/chrome/tab_list_backend.py:76: return None
On 2013/02/23 02:13:04, Dave Tu wrote:
> On 2013/02/23 02:03:20, achuith.bhandarkar wrote:
> > On 2013/02/23 01:50:19, Dave Tu wrote:
> > > This one too.
> > 
> > I've removed the TODO, but don't think it's safe to add the assert. I don't
> > think it's safe to assume that tab_info will be found, right?
> 
> Ah, yeah, you're right, that's still a problem. I guess the comment should
stay.

Is this really a problem? It seems reasonable for the tab to not exist? Maybe we
should raise an exception instead of returning None?

[dtu, 2013-02-23 03:04:12]

https://chromiumcodereview.appspot.com/12319081/diff/1/tools/telemetry/teleme...
File tools/telemetry/telemetry/core/chrome/tab_list_backend.py (right):

https://chromiumcodereview.appspot.com/12319081/diff/1/tools/telemetry/teleme...
tools/telemetry/telemetry/core/chrome/tab_list_backend.py:76: return None
On 2013/02/23 02:23:38, achuith.bhandarkar wrote:
> On 2013/02/23 02:13:04, Dave Tu wrote:
> > On 2013/02/23 02:03:20, achuith.bhandarkar wrote:
> > > On 2013/02/23 01:50:19, Dave Tu wrote:
> > > > This one too.
> > > 
> > > I've removed the TODO, but don't think it's safe to add the assert. I
don't
> > > think it's safe to assume that tab_info will be found, right?
> > 
> > Ah, yeah, you're right, that's still a problem. I guess the comment should
> stay.
> 
> Is this really a problem? It seems reasonable for the tab to not exist? Maybe
we
> should raise an exception instead of returning None?

This should only be called from tab.url, so the tab should exist if you reach
this point. If the tab doesn't exist, somebody is doing something wrong by
keeping a reference to the tab, hence the assert.

However, the bug here is that _FindTabInfo won't find the tab if we're connected
to that tab's websocket, because Chrome no longer gives the websocketDebuggerUrl
in that case. I'm not sure if tab.url is written to disconnect first.

[achuithb, 2013-02-23 03:10:44]

I've put the comments back in. PTAL, Dave

https://chromiumcodereview.appspot.com/12319081/diff/1/tools/telemetry/teleme...
File tools/telemetry/telemetry/core/chrome/tab_list_backend.py (right):

https://chromiumcodereview.appspot.com/12319081/diff/1/tools/telemetry/teleme...
tools/telemetry/telemetry/core/chrome/tab_list_backend.py:76: return None
On 2013/02/23 03:04:12, Dave Tu wrote:
> On 2013/02/23 02:23:38, achuith.bhandarkar wrote:
> > On 2013/02/23 02:13:04, Dave Tu wrote:
> > > On 2013/02/23 02:03:20, achuith.bhandarkar wrote:
> > > > On 2013/02/23 01:50:19, Dave Tu wrote:
> > > > > This one too.
> > > > 
> > > > I've removed the TODO, but don't think it's safe to add the assert. I
> don't
> > > > think it's safe to assume that tab_info will be found, right?
> > > 
> > > Ah, yeah, you're right, that's still a problem. I guess the comment should
> > stay.
> > 
> > Is this really a problem? It seems reasonable for the tab to not exist?
Maybe
> we
> > should raise an exception instead of returning None?
> 
> This should only be called from tab.url, so the tab should exist if you reach
> this point. If the tab doesn't exist, somebody is doing something wrong by
> keeping a reference to the tab, hence the assert.
> 
> However, the bug here is that _FindTabInfo won't find the tab if we're
connected
> to that tab's websocket, because Chrome no longer gives the
websocketDebuggerUrl
> in that case. I'm not sure if tab.url is written to disconnect first.

Done.

[dtu, 2013-02-23 03:23:03]

lgtm

[commit-bot: I haz the power, 2013-02-23 22:59:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/achuith@chromium.org/12319081/4003

[commit-bot: I haz the power, 2013-02-24 00:17:24]

Change committed as 184334