1026265 - [Datastore] JavaScript Error: "this.children[i].window is undefined" {file: "resource://gre/modules/DataStoreChangeNotifier.jsm" line: 117}

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Kevin Grandon :kgrandon]

I'm still not entirely sure of reproduction steps, but we are occasionally seeing this message in logs.

I don't have any STR unfortunately. Gecko build is believed to be a nightly build from 6/16/2014.

Comment 1

[Kevin Grandon :kgrandon]

Andrea - Could this be bad if we've see this in the logs occasionally?

Comment 2

[Andrea Marchesini [:baku]]

Attachment: bug.patch

https://tbpl.mozilla.org/?tree=Try&rev=1da81d1d98c6

Comment 3

[(no longer active)]

Comment on attachment 8441199 [details] [diff] [review]
bug.patch

Review of attachment 8441199 [details] [diff] [review]:
-----------------------------------------------------------------

I hate JS! :-)

Comment 4

[Andrea Marchesini [:baku]]

 https://hg.mozilla.org/integration/mozilla-inbound/rev/76486dd239b0

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/76486dd239b0

Comment 6

[Kevin Grandon :kgrandon]

Do we need to request uplift of this?

Comment 7

[Kevin Grandon :kgrandon]

Comment on attachment 8441199 [details] [diff] [review]
bug.patch

Bug caused by (feature/regressing bug #): Unknown.
User impact if declined: This could potentially break the vertical homescreen.
Testing completed (on m-c, etc.): Lots of manual testing and verification on master.
Risk to taking this patch (and alternatives if risky): None. This is a one character change. 
String or IDL/UUID changes made by this patch: None.

This is needed for the vertical homescreen. Please approve uplift for 2.0.

Comment 8

[(no longer active)]

Andrea, can you write a test for this please?

Comment 9

[Andrea Marchesini [:baku]]

Well.. In theory the test is already there: dom/datastore/tests/test_changes.html  but for some reason I didn't see this error when it runs for e10s.

Comment 10

[(no longer active)]

(In reply to Andrea Marchesini (:baku) from comment #9)
> Well.. In theory the test is already there:
> dom/datastore/tests/test_changes.html  but for some reason I didn't see this
> error when it runs for e10s.

That's very surprising?  Is the problem us eating the error message or something?  Or not running this code path at all?

Comment 11

[Andrea Marchesini [:baku]]

 0:16.06 System JS : ERROR resource://gre/modules/DataStoreChangeNotifier.jsm:117 - TypeError: this.children[i].window is undefined

The error was there but the tests did not throw any exception.

Comment 12

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8441199 [details] [diff] [review]
bug.patch

Aurora approval granted.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/3c08ec5b0b00