Revert of Improve error reporting when indexing tab.

Revert of Improve error reporting when indexing tab. (patchset #7 of https://codereview.chromium.org/449753002/)

Reason for revert:
This cl broke the some VMTest.SimpleVerifyTest on both amd64 and x86 chromium os bots.
https://build.chromium.org/p/chromiumos.chromium/builders/AMD64%20%28chromium%29/builds/1074
https://build.chromium.org/p/chromiumos.chromium/builders/X86%20%28chromium%29/builds/1109

The detailed log shows it is related to this cl:
https://00e9e64bac2888f7376cb33c571e0e5498754fbdb7e8acee1a-apidata.googleusercontent.com/download/storage/v1_internal/b/chromeos-image-archive/o/amd64-generic-tot-chrome-pfq-informational%2FR39-6171.0.0-b1074%2Fvm_test_results_1%2Ftest_harness%2Ffailed%2FSimpleTestVerify%2F1_autotest_tests%2Fresults-41-login_OwnershipNotRetaken%2Fstatus?qk=AD5uMEucYdUcVWJA9A5AhILbAIvsNHJmmpna3Ayxn20xDeOFPsTIUhwjTrXBLHXu6Nrp2Aw233JOIw89byLVRH2YMiThGI6jl1fg4DhFxelxALQuYpLdbrsc87_qTut2vUN5vYvcj-5skMoPi10uRsDikZ1WRKJF4VH2xFhTSMd3sNeOjJMDKs0NsLiZXCYbJK0hcmoGN_Loo9aIW3w8G_TJ8Ti-jSGBr4djBg2LO_7Ccwi4Wh-N_PjGNdpCkIUDUv398Lw1khp5rKPO-pja7JKqjPUIm73RZtRTfQrMNAPERPlpC4WIJPuTC4XuiV9rF9ZmlBfYPVbyfatUFT2bzKV_-ZRBp0YSu9JrZJDqE6Y3IgOJRJdH6CdaOG6i-Q5xYjiLvYuOHHGrurEVE_Za5KstT3qtQa7FA-y8cCl7tIkKYyF5a1B0BYrLPjiP7MqNEnZyHCvfNz1PNlJc8C2to_nZl-Rm4pYcYHWbhD4c0l5gSnK8-Wkuv9pkPaZ40v6ahA5xsaf1HJeWEq4n9GFrTeL_XarhENIPA_wsk3wwb5sgEO3TAtMCNNVhy0C-tPr2PxkChDJUTf6TkGI0Sj1CxZ662e1XoRASW_IaicBqYBoyj5YJM9aE9Wgx7FmDB25eD4YK2SFneTpBGrFYMbJuHRFyoV6pMlDze9nl6fKIH29G6d_tRssGpNOBhSCG8241cbicT6Wit04AODdbFTtOFjxX0uBVJOAJBvxyVdh1o9SwjfvHhPEBsDS3ZhQwjfAuVC-ars862ILNvlpiFWlXStJQWIQ5cJgr545sp88_8MzzYhzLob6feuqyd1RSO4QBH_uY02jx350yzcWQKQs_BwfAFkOkxzkuBXTJzhGlAcOJit3QfPoFKeimNybb4JBGuEvRqkHU-2HaUx6JYy1YjyRTd_20Sz1D8IqxdTMzLkllod0yFFdXnrWYBLH0xp-qCX5LaI_cNE-X.

Original issue's description:
> Improve error reporting when indexing tab.
> 
> If we created 3 new tabs, and 1 of them crashed, then there 
> is no guaranteed that  browser.tabs[1] return the correct tab 
> object. The tabs list can no longer be trusted in this case. Hence if 
> the number of inspectable contexts doesn't match the number 
> of expected tabs (based on the number of 
> tab_list_backend.New & tab_list_backend.Close calls), 
> tab_list_backend's __getitem__ throws a TabCrashException.
> 
> BUG=398467
> 
> Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=290974

[jennyz, 2014-08-21 18:37:19]

Created Revert of Improve error reporting when indexing tab.

[commit-bot: I haz the power, 2014-08-21 18:38:22]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jennyz@chromium.org/474983006/1

[commit-bot: I haz the power, 2014-08-21 18:38:22]

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

[commit-bot: I haz the power, 2014-08-21 18:38:23]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer or
a provisional committer, _not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[jennyz, 2014-08-21 18:38:48]

The CQ bit was checked by jennyz@chromium.org

[commit-bot: I haz the power, 2014-08-21 18:41:49]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jennyz@chromium.org/474983006/1

[commit-bot: I haz the power, 2014-08-21 18:41:50]

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

[commit-bot: I haz the power, 2014-08-21 18:41:51]

No LGTM from a valid reviewer yet. Only full committers are accepted.
Even if an LGTM may have been provided, it was from a non-committer or
a provisional committer, _not_ a full super star committer.
See http://www.chromium.org/getting-involved/become-a-committer
Note that this has nothing to do with OWNERS files.

[achuithb, 2014-08-21 18:52:14]

lgtm

[achuithb, 2014-08-21 18:52:25]

The CQ bit was checked by achuith@chromium.org

[commit-bot: I haz the power, 2014-08-21 18:53:54]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jennyz@chromium.org/474983006/1

[nednguyen, 2014-08-21 19:59:10]

The CQ bit was unchecked by nednguyen@google.com

[nednguyen, 2014-08-21 20:21:35]

On 2014/08/21 19:59:10, nednguyen wrote:
> The CQ bit was unchecked by mailto:nednguyen@google.com

Sorry, I am trying to fix the bug with
https://codereview.chromium.org/499453002/

[achuithb, 2014-08-21 23:08:54]

On 2014/08/21 20:21:35, nednguyen wrote:
> On 2014/08/21 19:59:10, nednguyen wrote:
> > The CQ bit was unchecked by mailto:nednguyen@google.com
> 
> Sorry, I am trying to fix the bug with
> https://codereview.chromium.org/499453002/

Do we have a bug to track this effort? Jenny's gardening shift ends today and we
should capture the context for the next gardener.

We should land the other change or revert this one by eod or the cros pfq will
not pick up a new chrome tonight.

[jennyz, 2014-08-21 23:11:22]

On 2014/08/21 23:08:54, achuithb wrote:
> On 2014/08/21 20:21:35, nednguyen wrote:
> > On 2014/08/21 19:59:10, nednguyen wrote:
> > > The CQ bit was unchecked by mailto:nednguyen@google.com
> > 
> > Sorry, I am trying to fix the bug with
> > https://codereview.chromium.org/499453002/
> 
> Do we have a bug to track this effort? Jenny's gardening shift ends today and
we
> should capture the context for the next gardener.
> 
> We should land the other change or revert this one by eod or the cros pfq will
> not pick up a new chrome tonight.

Here is the bug:
https://code.google.com/p/chromium/issues/detail?id=406010

nednguyen is trying to fix it by:
https://codereview.chromium.org/499453002/

[dtu, 2014-08-21 23:53:37]

On 2014/08/21 23:11:22, jennyz wrote:
> On 2014/08/21 23:08:54, achuithb wrote:
> > On 2014/08/21 20:21:35, nednguyen wrote:
> > > On 2014/08/21 19:59:10, nednguyen wrote:
> > > > The CQ bit was unchecked by mailto:nednguyen@google.com
> > > 
> > > Sorry, I am trying to fix the bug with
> > > https://codereview.chromium.org/499453002/
> > 
> > Do we have a bug to track this effort? Jenny's gardening shift ends today
and
> we
> > should capture the context for the next gardener.
> > 
> > We should land the other change or revert this one by eod or the cros pfq
will
> > not pick up a new chrome tonight.
> 
> Here is the bug:
> https://code.google.com/p/chromium/issues/detail?id=406010
> 
> nednguyen is trying to fix it by:
> https://codereview.chromium.org/499453002/

I'm seeing failures on Windows and Mac that may be related to this patch, but
nednguyen's fix linked above is ChromeOS-only.

http://build.chromium.org/p/chromium.perf/builders/Win%207%20Perf%20%282%29/b...
http://build.chromium.org/p/chromium.perf/builders/Win%208%20Perf%20%282%29/b...
http://build.chromium.org/p/chromium.perf/builders/Mac%2010.6%20Perf%20%282%2...

Specifically it is happening in the session_restore benchmarks, and not during a
clean startup.

Traceback (most recent call last):
  <module> at tools\perf\run_benchmark:25
    sys.exit(test_runner.main())
  main at tools\telemetry\telemetry\test_runner.py:347
    return command().Run(options)
  Run at tools\telemetry\telemetry\test_runner.py:182
    return min(255, self._test().Run(args))
  Run at tools\telemetry\telemetry\benchmark.py:94
    page_runner.Run(pt, ps, expectations, finder_options, results)
  Run at tools\telemetry\telemetry\page\page_runner.py:414
    page, credentials_path, possible_browser, results, state)
  _PrepareAndRunPage at tools\telemetry\telemetry\page\page_runner.py:273
    _RunPage(test, page, state, expectation, results)
  _RunPage at tools\telemetry\telemetry\page\page_runner.py:513
    page_state = PageState(page, test.TabForPage(page, state.browser))
  TabForPage at tools\perf\measurements\session_restore.py:41
    60)
  WaitFor at tools\telemetry\telemetry\core\util.py:76
    res = condition()
  <lambda> at tools\perf\measurements\session_restore.py:40
    browser.foreground_tab),
  foreground_tab at tools\telemetry\telemetry\core\browser.py:84
    if self._tabs[i].EvaluateJavaScript('!document.hidden'):
  __getitem__ at tools\telemetry\telemetry\core\tab_list.py:18
    return self._tab_list_backend.__getitem__(index)
  __getitem__ at
tools\telemetry\telemetry\core\backends\chrome\tab_list_backend.py:70
    assert self._num_expected_tabs == len(self)
AssertionError

[jennyz, 2014-08-21 23:57:29]

If so, the cl for fixing it is not good enough, maybe we should just revert
the original cl.

Jenny


On Thu, Aug 21, 2014 at 4:53 PM, <dtu@chromium.org> wrote:

> On 2014/08/21 23:11:22, jennyz wrote:
>
>> On 2014/08/21 23:08:54, achuithb wrote:
>> > On 2014/08/21 20:21:35, nednguyen wrote:
>> > > On 2014/08/21 19:59:10, nednguyen wrote:
>> > > > The CQ bit was unchecked by mailto:nednguyen@google.com
>> > >
>> > > Sorry, I am trying to fix the bug with
>> > > https://codereview.chromium.org/499453002/
>> >
>> > Do we have a bug to track this effort? Jenny's gardening shift ends
>> today
>>
> and
>
>> we
>> > should capture the context for the next gardener.
>> >
>> > We should land the other change or revert this one by eod or the cros
>> pfq
>>
> will
>
>> > not pick up a new chrome tonight.
>>
>
>  Here is the bug:
>> https://code.google.com/p/chromium/issues/detail?id=406010
>>
>
>  nednguyen is trying to fix it by:
>> https://codereview.chromium.org/499453002/
>>
>
> I'm seeing failures on Windows and Mac that may be related to this patch,
> but
> nednguyen's fix linked above is ChromeOS-only.
>
> http://build.chromium.org/p/chromium.perf/builders/Win%
> 207%20Perf%20%282%29/builds/15539
> http://build.chromium.org/p/chromium.perf/builders/Win%
> 208%20Perf%20%282%29/builds/2971
> http://build.chromium.org/p/chromium.perf/builders/Mac%
> 2010.6%20Perf%20%282%29/builds/35718
>
> Specifically it is happening in the session_restore benchmarks, and not
> during a
> clean startup.
>
> Traceback (most recent call last):
>   <module> at tools\perf\run_benchmark:25
>     sys.exit(test_runner.main())
>   main at tools\telemetry\telemetry\test_runner.py:347
>     return command().Run(options)
>   Run at tools\telemetry\telemetry\test_runner.py:182
>     return min(255, self._test().Run(args))
>   Run at tools\telemetry\telemetry\benchmark.py:94
>     page_runner.Run(pt, ps, expectations, finder_options, results)
>   Run at tools\telemetry\telemetry\page\page_runner.py:414
>     page, credentials_path, possible_browser, results, state)
>   _PrepareAndRunPage at tools\telemetry\telemetry\page\page_runner.py:273
>     _RunPage(test, page, state, expectation, results)
>   _RunPage at tools\telemetry\telemetry\page\page_runner.py:513
>     page_state = PageState(page, test.TabForPage(page, state.browser))
>   TabForPage at tools\perf\measurements\session_restore.py:41
>     60)
>   WaitFor at tools\telemetry\telemetry\core\util.py:76
>     res = condition()
>   <lambda> at tools\perf\measurements\session_restore.py:40
>     browser.foreground_tab),
>   foreground_tab at tools\telemetry\telemetry\core\browser.py:84
>     if self._tabs[i].EvaluateJavaScript('!document.hidden'):
>   __getitem__ at tools\telemetry\telemetry\core\tab_list.py:18
>     return self._tab_list_backend.__getitem__(index)
>   __getitem__ at
> tools\telemetry\telemetry\core\backends\chrome\tab_list_backend.py:70
>     assert self._num_expected_tabs == len(self)
> AssertionError
>
>
> https://codereview.chromium.org/474983006/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[chromium-reviews, 2014-08-22 00:22:26]

I also have fix for mac and win in a separate patch.
On Aug 21, 2014 4:57 PM, "Jenny Zhang(章瑛)" <jennyz@chromium.org> wrote:

> If so, the cl for fixing it is not good enough, maybe we should just
> revert the original cl.
>
> Jenny
>
>
> On Thu, Aug 21, 2014 at 4:53 PM, <dtu@chromium.org> wrote:
>
>> On 2014/08/21 23:11:22, jennyz wrote:
>>
>>> On 2014/08/21 23:08:54, achuithb wrote:
>>> > On 2014/08/21 20:21:35, nednguyen wrote:
>>> > > On 2014/08/21 19:59:10, nednguyen wrote:
>>> > > > The CQ bit was unchecked by mailto:nednguyen@google.com
>>> > >
>>> > > Sorry, I am trying to fix the bug with
>>> > > https://codereview.chromium.org/499453002/
>>> >
>>> > Do we have a bug to track this effort? Jenny's gardening shift ends
>>> today
>>>
>> and
>>
>>> we
>>> > should capture the context for the next gardener.
>>> >
>>> > We should land the other change or revert this one by eod or the cros
>>> pfq
>>>
>> will
>>
>>> > not pick up a new chrome tonight.
>>>
>>
>>  Here is the bug:
>>> https://code.google.com/p/chromium/issues/detail?id=406010
>>>
>>
>>  nednguyen is trying to fix it by:
>>> https://codereview.chromium.org/499453002/
>>>
>>
>> I'm seeing failures on Windows and Mac that may be related to this patch,
>> but
>> nednguyen's fix linked above is ChromeOS-only.
>>
>> http://build.chromium.org/p/chromium.perf/builders/Win%
>> 207%20Perf%20%282%29/builds/15539
>> http://build.chromium.org/p/chromium.perf/builders/Win%
>> 208%20Perf%20%282%29/builds/2971
>> http://build.chromium.org/p/chromium.perf/builders/Mac%
>> 2010.6%20Perf%20%282%29/builds/35718
>>
>> Specifically it is happening in the session_restore benchmarks, and not
>> during a
>> clean startup.
>>
>> Traceback (most recent call last):
>>   <module> at tools\perf\run_benchmark:25
>>     sys.exit(test_runner.main())
>>   main at tools\telemetry\telemetry\test_runner.py:347
>>     return command().Run(options)
>>   Run at tools\telemetry\telemetry\test_runner.py:182
>>     return min(255, self._test().Run(args))
>>   Run at tools\telemetry\telemetry\benchmark.py:94
>>     page_runner.Run(pt, ps, expectations, finder_options, results)
>>   Run at tools\telemetry\telemetry\page\page_runner.py:414
>>     page, credentials_path, possible_browser, results, state)
>>   _PrepareAndRunPage at tools\telemetry\telemetry\page\page_runner.py:273
>>     _RunPage(test, page, state, expectation, results)
>>   _RunPage at tools\telemetry\telemetry\page\page_runner.py:513
>>     page_state = PageState(page, test.TabForPage(page, state.browser))
>>   TabForPage at tools\perf\measurements\session_restore.py:41
>>     60)
>>   WaitFor at tools\telemetry\telemetry\core\util.py:76
>>     res = condition()
>>   <lambda> at tools\perf\measurements\session_restore.py:40
>>     browser.foreground_tab),
>>   foreground_tab at tools\telemetry\telemetry\core\browser.py:84
>>     if self._tabs[i].EvaluateJavaScript('!document.hidden'):
>>   __getitem__ at tools\telemetry\telemetry\core\tab_list.py:18
>>     return self._tab_list_backend.__getitem__(index)
>>   __getitem__ at
>> tools\telemetry\telemetry\core\backends\chrome\tab_list_backend.py:70
>>     assert self._num_expected_tabs == len(self)
>> AssertionError
>>
>>
>> https://codereview.chromium.org/474983006/
>>
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.