Fix flaky NPE in oobe.js.

Fix flaky NPE in oobe.js.

Under some racy condition, telemetry accesses the DOM object before ready.
This CL observes a new variable which is set to true when DOM is ready.

BUG=chromium:709041
TEST=Ran try. Ran on ChromeOS device with corresponding CL.

Review-Url: https://codereview.chromium.org/2803043002
Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/9a55abab029cb9ae94f5160ded11b09a4638a955

[hidehiko, 2017-04-06 15:19:40]

The CQ bit was checked by hidehiko@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-06 15:19:45]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-06 15:44:10]

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

[commit-bot: I haz the power, 2017-04-06 15:44:11]

Dry run: This issue passed the CQ dry run.

[hidehiko, 2017-04-06 17:11:20]

hidehiko@chromium.org changed reviewers:
+ achuith@chromium.org

[hidehiko, 2017-04-06 17:11:21]

Corresponding CL is here.
https://codereview.chromium.org/2805033002

Could you take a look?

[hidehiko, 2017-04-10 07:25:02]

On 2017/04/06 17:11:21, hidehiko wrote:
> Corresponding CL is here.
> https://codereview.chromium.org/2805033002
> 
> Could you take a look?

friendly ping?

[achuithb, 2017-04-10 08:48:56]

lgtm

[achuithb, 2017-04-10 08:49:22]

The CQ bit was checked by achuith@chromium.org

[commit-bot: I haz the power, 2017-04-10 08:49:29]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-10 09:12:57]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1491814162712720, "parent_rev":
"87e8335e7464200d1d500fc5f7ff6ab1f527714c", "commit_rev":
"9a55abab029cb9ae94f5160ded11b09a4638a955"}

[commit-bot: I haz the power, 2017-04-10 09:13:03]

Description was changed from

==========
Fix flaky NPE in oobe.js.

Under some racy condition, telemetry accesses the DOM object before ready.
This CL observes a new variable which is set to true when DOM is ready.

BUG=chromium:709041
TEST=Ran try. Ran on ChromeOS device with corresponding CL.
==========

to

==========
Fix flaky NPE in oobe.js.

Under some racy condition, telemetry accesses the DOM object before ready.
This CL observes a new variable which is set to true when DOM is ready.

BUG=chromium:709041
TEST=Ran try. Ran on ChromeOS device with corresponding CL.

Review-Url: https://codereview.chromium.org/2803043002
Committed:
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...
==========

[commit-bot: I haz the power, 2017-04-10 09:13:04]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...

[stephen.kyle, 2017-04-12 08:59:01]

Hi,

Since this CL landed, telemetry is no longer working for us when testing Chrome
on ChromeOS:

INFO:root:Invoking Oobe.loginForTesting
ERROR:root:Failure while starting browser backend.
Traceback (most recent call last):
  File
"/work/google/chrome/src/third_party/catapult/telemetry/telemetry/internal/browser/browser.py",
line 58, in __init__
    self._browser_backend.Start()
  File
"/work/google/chrome/src/third_party/catapult/common/py_trace_event/py_trace_event/trace_event_impl/decorators.py",
line 75, in traced_function
    return func(*args, **kwargs)
  File
"/work/google/chrome/src/third_party/catapult/telemetry/telemetry/internal/backends/chrome/cros_browser_backend.py",
line 163, in Start
    self._gaia_id, not self.browser_options.disable_gaia_services)
  File
"/work/google/chrome/src/third_party/catapult/common/py_trace_event/py_trace_event/trace_event_impl/decorators.py",
line 75, in traced_function
    return func(*args, **kwargs)
  File
"/work/google/chrome/src/third_party/catapult/telemetry/telemetry/internal/backends/chrome/oobe.py",
line 61, in NavigateFakeLogin
    enterprise_enroll)
  File
"/work/google/chrome/src/third_party/catapult/common/py_trace_event/py_trace_event/trace_event_impl/decorators.py",
line 75, in traced_function
    return func(*args, **kwargs)
  File
"/work/google/chrome/src/third_party/catapult/telemetry/telemetry/internal/backends/chrome/oobe.py",
line 41, in _ExecuteOobeApi
    "typeof Oobe == 'function' && Oobe.readyForTesting", timeout=120)
  File
"/work/google/chrome/src/third_party/catapult/common/py_trace_event/py_trace_event/trace_event_impl/decorators.py",
line 75, in traced_function
    return func(*args, **kwargs)
  File
"/work/google/chrome/src/third_party/catapult/telemetry/telemetry/internal/browser/web_contents.py",
line 191, in WaitForJavaScriptCondition
    return self._inspector_backend.WaitForJavaScriptCondition(*args, **kwargs)
  File
"/work/google/chrome/src/third_party/catapult/common/py_trace_event/py_trace_event/trace_event_impl/decorators.py",
line 75, in traced_function
    return func(*args, **kwargs)
  File
"/work/google/chrome/src/third_party/catapult/telemetry/telemetry/internal/backends/chrome_inspector/inspector_backend.py",
line 294, in WaitForJavaScriptCondition
    e.message + '\n' + debug_message)
TimeoutException: Timed out while waiting 120s for IsJavaScriptExpressionTrue.
Console output:
(warning)
chrome://resources/polymer/v1_0/iron-flex-layout/classes/iron-shadow-flex-layout-extracted.js:1:
This file is deprecated. Please use
`iron-flex-layout/iron-flex-layout-classes.html`, and one of the specific
dom-modules instead
(warning)
chrome://resources/polymer/v1_0/iron-flex-layout/classes/iron-flex-layout-extracted.js:1:
This file is deprecated. Please use
`iron-flex-layout/iron-flex-layout-classes.html`, and one of the specific
dom-modules instead
(error) chrome://oobe/oobe.js:8545: TypeError: Cannot read property
'addObserver' of null
    at HTMLDivElement.decorate (chrome://oobe/oobe.js:17344:19)
    at HTMLDivElement.initializeImpl_ (chrome://oobe/oobe.js:226:12)
    at HTMLDivElement.initialize (chrome://oobe/oobe.js:122:35)
    at Object.api.register (chrome://oobe/oobe.js:435:18)
    at Function.initialize (chrome://oobe/oobe.js:18034:32)
    at HTMLDocument.<anonymous> (chrome://oobe/oobe.js:8539:10)
(log) chrome://oobe/oobe.js:8042: Loading asset bundle oauth-enrollment
(log) chrome://oobe/oobe.js:8026: Finished loading asset bundle oauth-enrollment

WARNING:root:Cannot dump browser state: No browser.

The browser we're building and running telemetry against contains commit
d5027325193408828b4dab341dabbc6ff0b7791e. Did you encounter anything like this
during testing?

[hidehiko, 2017-04-12 09:22:38]

On 2017/04/12 08:59:01, stephen.kyle wrote:
> Hi,
> 
> Since this CL landed, telemetry is no longer working for us when testing
Chrome
> on ChromeOS:
> 
> INFO:root:Invoking Oobe.loginForTesting
> ERROR:root:Failure while starting browser backend.
> Traceback (most recent call last):
>   File
>
"/work/google/chrome/src/third_party/catapult/telemetry/telemetry/internal/browser/browser.py",
> line 58, in __init__
>     self._browser_backend.Start()
>   File
>
"/work/google/chrome/src/third_party/catapult/common/py_trace_event/py_trace_event/trace_event_impl/decorators.py",
> line 75, in traced_function
>     return func(*args, **kwargs)
>   File
>
"/work/google/chrome/src/third_party/catapult/telemetry/telemetry/internal/backends/chrome/cros_browser_backend.py",
> line 163, in Start
>     self._gaia_id, not self.browser_options.disable_gaia_services)
>   File
>
"/work/google/chrome/src/third_party/catapult/common/py_trace_event/py_trace_event/trace_event_impl/decorators.py",
> line 75, in traced_function
>     return func(*args, **kwargs)
>   File
>
"/work/google/chrome/src/third_party/catapult/telemetry/telemetry/internal/backends/chrome/oobe.py",
> line 61, in NavigateFakeLogin
>     enterprise_enroll)
>   File
>
"/work/google/chrome/src/third_party/catapult/common/py_trace_event/py_trace_event/trace_event_impl/decorators.py",
> line 75, in traced_function
>     return func(*args, **kwargs)
>   File
>
"/work/google/chrome/src/third_party/catapult/telemetry/telemetry/internal/backends/chrome/oobe.py",
> line 41, in _ExecuteOobeApi
>     "typeof Oobe == 'function' && Oobe.readyForTesting", timeout=120)
>   File
>
"/work/google/chrome/src/third_party/catapult/common/py_trace_event/py_trace_event/trace_event_impl/decorators.py",
> line 75, in traced_function
>     return func(*args, **kwargs)
>   File
>
"/work/google/chrome/src/third_party/catapult/telemetry/telemetry/internal/browser/web_contents.py",
> line 191, in WaitForJavaScriptCondition
>     return self._inspector_backend.WaitForJavaScriptCondition(*args, **kwargs)
>   File
>
"/work/google/chrome/src/third_party/catapult/common/py_trace_event/py_trace_event/trace_event_impl/decorators.py",
> line 75, in traced_function
>     return func(*args, **kwargs)
>   File
>
"/work/google/chrome/src/third_party/catapult/telemetry/telemetry/internal/backends/chrome_inspector/inspector_backend.py",
> line 294, in WaitForJavaScriptCondition
>     e.message + '\n' + debug_message)
> TimeoutException: Timed out while waiting 120s for IsJavaScriptExpressionTrue.
> Console output:
> (warning)
>
chrome://resources/polymer/v1_0/iron-flex-layout/classes/iron-shadow-flex-layout-extracted.js:1:
> This file is deprecated. Please use
> `iron-flex-layout/iron-flex-layout-classes.html`, and one of the specific
> dom-modules instead
> (warning)
>
chrome://resources/polymer/v1_0/iron-flex-layout/classes/iron-flex-layout-extracted.js:1:
> This file is deprecated. Please use
> `iron-flex-layout/iron-flex-layout-classes.html`, and one of the specific
> dom-modules instead
> (error) chrome://oobe/oobe.js:8545: TypeError: Cannot read property
> 'addObserver' of null
>     at HTMLDivElement.decorate (chrome://oobe/oobe.js:17344:19)
>     at HTMLDivElement.initializeImpl_ (chrome://oobe/oobe.js:226:12)
>     at HTMLDivElement.initialize (chrome://oobe/oobe.js:122:35)
>     at Object.api.register (chrome://oobe/oobe.js:435:18)
>     at Function.initialize (chrome://oobe/oobe.js:18034:32)
>     at HTMLDocument.<anonymous> (chrome://oobe/oobe.js:8539:10)
> (log) chrome://oobe/oobe.js:8042: Loading asset bundle oauth-enrollment
> (log) chrome://oobe/oobe.js:8026: Finished loading asset bundle
oauth-enrollment
> 
> WARNING:root:Cannot dump browser state: No browser.
> 
> The browser we're building and running telemetry against contains commit
> d5027325193408828b4dab341dabbc6ff0b7791e. Did you encounter anything like this
> during testing?

No, I didn't hit it.
Looks like the page is not loaded for 2 mins, maybe caused by missing context?

[achuithb, 2017-04-12 19:03:19]

The informational builders seem to be working ok with this change?

https://uberchromegw.corp.google.com/i/chromiumos.chromium/builders/amd64-gen...

[stephen.kyle, 2017-04-13 08:04:30]

> No, I didn't hit it.
> Looks like the page is not loaded for 2 mins, maybe caused by missing context?

Looks like it's been reported in
https://bugs.chromium.org/p/chromium/issues/detail?id=710975 as well. I reported
it in https://bugs.chromium.org/p/chromium/issues/detail?id=710387.

[Shuhei Takahashi, 2017-04-13 08:27:16]

On 2017/04/13 08:04:30, stephen.kyle wrote:
> > No, I didn't hit it.
> > Looks like the page is not loaded for 2 mins, maybe caused by missing
context?
> 
> Looks like it's been reported in
> https://bugs.chromium.org/p/chromium/issues/detail?id=710975 as well. I
reported
> it in https://bugs.chromium.org/p/chromium/issues/detail?id=710387.

Thanks for the pointers. I commented in the first thread, and I think it's fine
to revert this change.

[kinaba, 2017-04-13 08:38:27]

A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/2808763010/ by kinaba@chromium.org.

The reason for reverting is: Breaking existing tests. (crbug.com/710975).