Removing browser.Start() and instead starts the browser at the point of creation.

Removing browser.Start() and instead starts the browser at the point of creation.

BUG=412126
Committed: https://crrev.com/01c68e8765254d9971296b291ff718b4f6af3004
Cr-Commit-Position: refs/heads/master@{#295591}

Committed: https://crrev.com/74c6ef17a08754abadb6fb64da14249898285fa8
Cr-Commit-Position: refs/heads/master@{#296107}

Committed: https://crrev.com/351c0f63466ac66eb03faaa85b27b98ae9c2b43c
Cr-Commit-Position: refs/heads/master@{#296500}

[wuhu, 2014-09-12 00:09:18]

wuhu@google.com changed reviewers:
+ nednguyen@google.com

[wuhu, 2014-09-12 00:09:18]

Note that we now have to pass Test object to the Browser so that
Test.WillStartBrowser (eg. for power metric) can be called before the browser
backend is started in browser constructor.  This is kinda ugly.  Maybe there is
a better way?

[nednguyen, 2014-09-12 00:17:25]

On 2014/09/12 00:09:18, wuhu wrote:
> Note that we now have to pass Test object to the Browser so that
> Test.WillStartBrowser (eg. for power metric) can be called before the browser
> backend is started in browser constructor.  This is kinda ugly.  Maybe there
is
> a better way?

Let's see if we can move Test.WillStartBrowser(browser) to
Test.WillStartBrowser(platform). I suspect we should be able to do so. Let's
file a bug to move WillStartBrowser(browser) to WillStartBrowser(platform) and
make that bug blocks crbug.com/412126

[wuhu, 2014-09-12 00:23:50]

413481

[nednguyen, 2014-09-17 15:40:41]

nednguyen@google.com changed reviewers:
+ slamm@chromium.org

[nednguyen, 2014-09-17 15:40:57]

nednguyen@google.com changed reviewers:
+ slamm@google.com

[nednguyen, 2014-09-17 16:20:14]

LGTM

https://codereview.chromium.org/565803002/diff/20001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/core/possible_browser.py (right):

https://codereview.chromium.org/565803002/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/core/possible_browser.py:84: self._test = test
Why do we need this?

[wuhu, 2014-09-17 20:15:43]

Thanks for catching that, cleanup done.

[wuhu, 2014-09-17 20:16:05]

The CQ bit was checked by wuhu@google.com

[commit-bot: I haz the power, 2014-09-17 20:16:46]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/565803002/40001

[commit-bot: I haz the power, 2014-09-17 20:27:39]

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

[commit-bot: I haz the power, 2014-09-17 20:27:40]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[chromium-reviews, 2014-09-17 21:12:21]

The chromium_presubmit error looks legit.

On Wed, Sep 17, 2014 at 1:27 PM, <commit-bot@chromium.org> wrote:

> Try jobs failed on following builders:
>   chromium_presubmit on tryserver.chromium.linux
> (http://build.chromium.org/p/tryserver.chromium.linux/
> builders/chromium_presubmit/builds/11676)
>
> https://codereview.chromium.org/565803002/
>

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

[wuhu, 2014-09-18 00:35:32]

Looks like a bug in pylint which was fixed in 0.25.2
(http://www.logilab.org/ticket/89092).  My system is running 0.25.1.

Btw, the line was moved over from page_runner.py (line 49 in the old version). 
Puzzling how it got through pylint, or was pylint was added to the process
later?

[nednguyen, 2014-09-18 01:22:07]

On 2014/09/18 00:35:32, wuhu wrote:
> Looks like a bug in pylint which was fixed in 0.25.2
> (http://www.logilab.org/ticket/89092).  My system is running 0.25.1.
> 
> Btw, the line was moved over from page_runner.py (line 49 in the old version).

> Puzzling how it got through pylint, or was pylint was added to the process
> later?

You can either suppress the pylint with # pylint: disable=E0202 or contact
chrome-infrastructure-team@google.com  about this.

[wuhu, 2014-09-18 01:23:49]

Done, suppressing the error for now (as done else where in telemetry code base).

[wuhu, 2014-09-18 01:24:17]

The CQ bit was checked by wuhu@google.com

[nednguyen, 2014-09-18 01:25:35]

https://codereview.chromium.org/565803002/diff/60001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/core/browser_credentials.py (right):

https://codereview.chromium.org/565803002/diff/60001/tools/telemetry/telemetr...
tools/telemetry/telemetry/core/browser_credentials.py:70: def
credentials_path(self):  #pylint: disable=E0202
Nit: please follow the style "   # pylint: disable=E0202"

[commit-bot: I haz the power, 2014-09-18 01:25:38]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/565803002/60001

[nednguyen, 2014-09-18 01:25:51]

The CQ bit was unchecked by nednguyen@google.com

[wuhu, 2014-09-18 16:22:35]

The CQ bit was checked by wuhu@google.com

[commit-bot: I haz the power, 2014-09-18 16:23:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/565803002/60001

[nednguyen, 2014-09-18 16:27:08]

The CQ bit was unchecked by nednguyen@google.com

[wuhu, 2014-09-18 16:35:55]

The CQ bit was checked by wuhu@google.com

[commit-bot: I haz the power, 2014-09-18 16:36:30]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/565803002/80001

[commit-bot: I haz the power, 2014-09-18 22:38:44]

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

[commit-bot: I haz the power, 2014-09-18 22:38:45]

Exceeded time limit waiting for builds to trigger.

[wuhu, 2014-09-18 22:54:06]

The CQ bit was checked by wuhu@google.com

[commit-bot: I haz the power, 2014-09-18 22:54:40]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/565803002/80001

[commit-bot: I haz the power, 2014-09-18 23:14:04]

Committed patchset #5 (id:80001) as 432128763d797207b30840859de9a2f6bea39278

[commit-bot: I haz the power, 2014-09-18 23:14:47]

Patchset 5 (id:??) landed as
https://crrev.com/01c68e8765254d9971296b291ff718b4f6af3004
Cr-Commit-Position: refs/heads/master@{#295591}

[wuhu, 2014-09-19 22:11:43]

A revert of this CL (patchset #5 id:80001) has been created in
https://codereview.chromium.org/587853003/ by wuhu@google.com.

The reason for reverting is: missed changing constructor for CrOS browser.

[wuhu, 2014-09-19 22:28:29]

Patchset #6 (id:100001) has been deleted

[wuhu, 2014-09-22 23:31:58]

The CQ bit was checked by wuhu@google.com

[commit-bot: I haz the power, 2014-09-22 23:33:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/565803002/120001

[commit-bot: I haz the power, 2014-09-23 00:34:05]

Committed patchset #6 (id:120001) as be6b5b0b2230baae4a4074ecef5209f97f65193b

[commit-bot: I haz the power, 2014-09-23 00:35:04]

Patchset 6 (id:??) landed as
https://crrev.com/74c6ef17a08754abadb6fb64da14249898285fa8
Cr-Commit-Position: refs/heads/master@{#296107}

[wuhu, 2014-09-23 18:10:36]

A revert of this CL (patchset #6 id:120001) has been created in
https://codereview.chromium.org/601473002/ by wuhu@google.com.

The reason for reverting is: investigate build error.

[wuhu, 2014-09-24 19:33:55]

The CQ bit was checked by wuhu@google.com

[commit-bot: I haz the power, 2014-09-24 19:34:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/565803002/160001

[commit-bot: I haz the power, 2014-09-24 20:13:20]

Committed patchset #8 (id:160001) as 1e40d0c8afb5fae0dddaf4efd9de1a3667bf3834

[commit-bot: I haz the power, 2014-09-24 20:13:55]

Patchset 8 (id:??) landed as
https://crrev.com/351c0f63466ac66eb03faaa85b27b98ae9c2b43c
Cr-Commit-Position: refs/heads/master@{#296500}

[achuithb, 2014-09-24 23:39:11]

On 2014/09/24 20:13:55, I haz the power (commit-bot) wrote:
> Patchset 8 (id:??) landed as
> https://crrev.com/351c0f63466ac66eb03faaa85b27b98ae9c2b43c
> Cr-Commit-Position: refs/heads/master@{#296500}

Looks like this one should stick:
http://build.chromium.org/p/chromiumos.chromium/builders/X86%20%28chromium%29...