1321865 - Enable IntersectionObserver

Status: RESOLVED FIXED

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

Description

[Olli Pettay [:smaug][[e-mail]]]

Once the stability issues have been fixed, the API should be enabled again.

Comment 1

[Nicholas Nethercote [inactive]]

Most of the crash signatures have "IntersectionObserver" in them and so are obviously related. But there were a few ones that weren't like that. From bug 1317415:

> [@ nsGenericHTMLElement::QueryInterface ]
> [@ RtlDispatchException | KiUserExceptionDispatcher ]
> [@ NS_TableDrivenQI ]

Comment 2

[Tobias Schneider [:tobytailor]]

Attachment: Enable IntersectionObserver

Once Bug 1322717 is landed, this patch flips the pref to enable the IntersectionObserver API.

Comment 3

[Nicholas Nethercote [inactive]]

Tobias: are you confident that the patch in bug 1322717 will fix all the crashes, with various signatures, that we've seen? Have you been able to reproduce more than one of the crash signatures yourself? Thanks.

Comment 4

[Tobias Schneider [:tobytailor]]

Yes, Im confident and yes I was able to reproduce them myself (took me a couple of days surfing around on a Windows machine tho). I was wondering about why most reports came from Windows machines, but I think thats just because of the larger share of Nightly users on that platform. I successfully reproduced it on Mac OS X too. It really requires certain circumstances to happen for those crashes which makes it difficult to deterministically reproduce it with a test.

Comment 5

[Nicholas Nethercote [inactive]]

Good to hear! And yes, the Windows population is *much* larger than Mac and Linux on all channels.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Landing this with an implicit r=smaug given the belief that the crashes are indeed fixed now.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8ffd7d13eb96
Enable IntersectionObserver. r=smaug

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for not enabling IntersectionObserverEntry in the whitelist of test_interfaces.html:

https://hg.mozilla.org/integration/mozilla-inbound/rev/83882d91f759155dad8fdb8c68af0e6d65906b93

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=8ffd7d13eb961a3f6df90e17408534790f58bf89
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=40463921&repo=mozilla-inbound

dom/tests/mochitest/general/test_interfaces.html | If this is failing: DANGER, are you sure you want to expose the new interface IntersectionObserverEntry to all webpages as a property on the window (XBL: false)? Do not make a change to this file without a review from a DOM peer for that specific change!!! (or a JS peer for changes to ecmaGlobals)

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

r=smaug to add IntersectionObserver to the test_interfaces.html list.

Comment 10

[Olli Pettay [:smaug][[e-mail]]]

but but, doesn't one need to also enable the tests again too, the one which bug 1320704 disabled.

Comment 11

[Tobias Schneider [:tobytailor]]

Attachment: Enable Tests

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/dc211e6cb0c1
Enable IntersectionObserver r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/ed256c5c7d23
Enable IntersectionObserver Tests. r=smaug

Comment 13

[Carsten Book [:Tomcat]]

backed out for test failurs like https://treeherder.mozilla.org/logviewer.html#?job_id=40567662&repo=mozilla-inbound

Comment 14

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e2e171b4e26d
Backed out changeset ed256c5c7d23 
https://hg.mozilla.org/integration/mozilla-inbound/rev/48dbfd4172e1
Backed out changeset dc211e6cb0c1 for failing on own test

Comment 15

[Tobias Schneider [:tobytailor]]

Attachment: Enable Tests. r=smaug

Keep disabled on android.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3ae78eda5a0c
Enable IntersectionObserver. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/016b87fe9145
Enable IntersectionObserver tests. r=smaug

Comment 17

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3ae78eda5a0c
https://hg.mozilla.org/mozilla-central/rev/016b87fe9145

Comment 18

[Jim Jeffery not reading bug-mail 1/2/11]

Still getting crashes with this enabled:

STR:
1.  Visit: http://www.foxnews.com/tech/2016/12/16/incredible-drone-footage-shows-killer-whales-eating-living-shark.html
2.  Scroll down a bit to the video 
3.  Crash

c-r https://crash-stats.mozilla.com/report/index/7167d7cd-c0b2-48d0-99c2-36a5c2161217

Comment 19

[Andrew McCreight [:mccr8]]

Backed out for causing bug 1324209 (which is the top crash by a large margin on the latest Nightly):
https://hg.mozilla.org/integration/mozilla-inbound/rev/1016d96bdabb1522645aa97d18a3e438a3cc35b2
https://hg.mozilla.org/integration/mozilla-inbound/rev/f22c6dc53276d9cb1e7365e47dc52657fac

Comment 20

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/1016d96bdabb
https://hg.mozilla.org/mozilla-central/rev/f22c6dc53276

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1e3ce9502a04
Enable IntersectionObserver. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/394aea8026a3
Enable IntersectionObserver tests. r=smaug

Comment 22

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/1e3ce9502a04
https://hg.mozilla.org/mozilla-central/rev/394aea8026a3

Comment 23

[Nicholas Nethercote [inactive]]

Can one of tschneider, RyanVM or smaug please back this out ASAP? Given the history of this new API -- it's been the top cause of crashes in Nightly on three different occasions now -- I think a disable-first-and-ask-questions-later approach is appropriate.

bsmedberg has been talking about writing test plans for new features to ensure a minimum standard of quality for new features. (https://wiki.mozilla.org/QA/Test_Plan_Template is one relevant document, and there are undoubtedly others.) Perhaps a test plan should be created for this feature, and an appropriate level of testing done, before we re-enable it again on Nightly.

Comment 24

[Tobias Schneider [:tobytailor]]

RyanVM can you please back this out until I have the crashes fixed?

Comment 25

[Olli Pettay [:smaug][[e-mail]]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3346ef79c433e6f01f6fb392a9d909a888af9a38
https://hg.mozilla.org/integration/mozilla-inbound/rev/81a71a4ea34d61a51fdee2836a7ea4663bc19f06

Comment 26

[Ryan VanderMeulen [:RyanVM]]

Was comment 23 ever addressed regarding a test plan? FWIW, I'm inclined to agree with Nick's assessment here.

Comment 27

[Tobias Schneider [:tobytailor]]

I agree with what Nick said and that we need a test plan for this. Actually, I'm already working on defining one. The latest crash was a regression introduces by myself, trying to fix another crash. Totally my fault. I would like to give this another shot. I tested affected websites for a couple of days without running into any further issues. I will have an eye on this and if needed asking for blackout a soon as crashes should come in.

Comment 28

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8beb4b722b73
Enable IntersectionObserver. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/1f9103460d1a
Enable Tests. r=smaug

Comment 29

[Carsten Book [:Tomcat]]

backed out for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=67922908&repo=mozilla-inbound

Comment 30

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2d8495cffc78
Backed out changeset 1f9103460d1a 
https://hg.mozilla.org/integration/mozilla-inbound/rev/45038edd8121
Backed out changeset 8beb4b722b73 for errors like in test_intersectionobservers.html

Comment 31

[Tobias Schneider [:tobytailor]]

Attachment: Enable IntersectionObserver

Rebased patch.

Comment 32

[Tobias Schneider [:tobytailor]]

Attachment: Enable IntersectionObserver

The real one.

Comment 33

[Tobias Schneider [:tobytailor]]

Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=677470bff8e880d5b732fb6f43a2374243f3f0e6&selectedJob=68189945

Comment 34

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cd23e1fc730b
Enable IntersectionObserver. r=smaug
https://hg.mozilla.org/integration/mozilla-inbound/rev/31511bed6415
Enable IntersectionObserver Tests. r=smaug

Comment 35

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/cd23e1fc730b
https://hg.mozilla.org/mozilla-central/rev/31511bed6415

Comment 36

[Tobias Schneider [:tobytailor]]

Ryan, please back out again due to newly reported crashes.

Comment 37

[Ryan VanderMeulen [:RyanVM]]

By my count, this is now the 4th time this has landed and been backed out for stability issues (and is now going to slip the 53 release as well). I'm sorry for the bluntness here, but we clearly have insufficient test coverage of this feature, and that needs to block this being re-enabled. Can we get some targeted fuzzing, code coverage results, whatever to figure out what we're missing so that this can be turned back on when we're truly confident in its readiness for doing so? At an absolute bare minimum, we clearly need targeted testing from QA before flipping the pref again for nightly users.

https://hg.mozilla.org/mozilla-central/rev/d5e37c0a776f1f2c21ddac4612529f819e13733b

Comment 38

[Jet Villegas (inactive)]

Tobias: Please set me to r? before landing this one again. Thx!

Comment 39

[Olli Pettay [:smaug][[e-mail]]]

Might we worth to explain the ownership model of InterserctionObserver to some DOM folks who have dealt with GC and CC.
Happy to help here.

Comment 40

[Calixte Denizet (:calixte)]

Right now we've 1087 crashes in nightly with build-id 20170122030212.

Comment 41

[Tyson Smith [:tsmith]]

:truber is working on adding support to existing fuzzers to get some fuzz coverage.

Comment 42

[David Baron :dbaron:]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #37)
> By my count, this is now the 4th time this has landed and been backed out
> for stability issues (and is now going to slip the 53 release as well). I'm
> sorry for the bluntness here, but we clearly have insufficient test coverage
> of this feature, and that needs to block this being re-enabled. Can we get
> some targeted fuzzing, code coverage results, whatever to figure out what
> we're missing so that this can be turned back on when we're truly confident
> in its readiness for doing so? At an absolute bare minimum, we clearly need
> targeted testing from QA before flipping the pref again for nightly users.

I think the main thing that's going to help here is more review from (and discussion with) folks who have dealt with these issues before, a la comment 53.  Would be great to see that happen in time for 54.

Comment 43

[David Baron :dbaron:]

er, s/comment 53/comment 39/.  Sorry.

Comment 44

[Jet Villegas (inactive)]

(In reply to David Baron :dbaron: ⌚️UTC-8 from comment #42)
> (In reply to Ryan VanderMeulen [:RyanVM] from comment #37)
> > By my count, this is now the 4th time this has landed and been backed out
> > for stability issues (and is now going to slip the 53 release as well). I'm
> > sorry for the bluntness here, but we clearly have insufficient test coverage
> > of this feature, and that needs to block this being re-enabled. Can we get
> > some targeted fuzzing, code coverage results, whatever to figure out what
> > we're missing so that this can be turned back on when we're truly confident
> > in its readiness for doing so? At an absolute bare minimum, we clearly need
> > targeted testing from QA before flipping the pref again for nightly users.
> 
> I think the main thing that's going to help here is more review from (and
> discussion with) folks who have dealt with these issues before, a la comment
> 53.  Would be great to see that happen in time for 54.

Yes, these discussions are happening and will continue. Another priority is to formalize a Test Plan for the feature. Tobias will share a link when that's ready for wider review.

Comment 45

[Tobias Schneider [:tobytailor]]

See https://wiki.mozilla.org/QA/IntersectionObserver

Comment 46

[Tobias Schneider [:tobytailor]]

Attachment: Test Plan

Comment 47

[Tobias Schneider [:tobytailor]]

Put together a document describing the lifetime management of the Intersection Observer API: https://docs.google.com/document/d/1bHK0_aTVyi5cBYODRYyuK-wQdJ2bTY6QPZu4F0AF0Io/edit?usp=sharing

Comment 48

[Jet Villegas (inactive)]

Comment on attachment 8825915 [details] [diff] [review]
Enable IntersectionObserver

Experimental deployment looks good. We'll go to 100% on the test group and then enable this feature. After it's in Nightly for a bit, and we don't take more code changes, please flag this patch for uplift to FF54. Thanks!

Comment 49

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/718fb66559f7
Enable IntersectionObserver and tests. r=smaug, r=jet

Comment 50

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/718fb66559f7

Comment 51

[Joel Maher ( :jmaher ) (UTC -8) (PTO'ish until July 5)]

I just backed this out:
https://hg.mozilla.org/integration/mozilla-inbound/rev/35af33f46d9499052b4c1ccdee93907f602b6698

we are almost failing once/push for the new test case- please revisit and show some try pushes/etc. I would recommend retriggering a few times mochitest-e10s-1.  More details in bug 1313972.

Comment 52

[Ryan VanderMeulen [:RyanVM]]

Merge of backout:
https://hg.mozilla.org/mozilla-central/rev/35af33f46d94

Comment 53

[Tobias Schneider [:tobytailor]]

With Bug 1313972 being fixed, let's re-land this.

Comment 54

[Ryan VanderMeulen [:RyanVM]]

Can we verify that on Try first?

Comment 55

[Tobias Schneider [:tobytailor]]

See https://treeherder.mozilla.org/#/jobs?repo=try&revision=aa08a5a24e2a38861ae19dae375d00d9a774a189

Comment 56

[Ryan VanderMeulen [:RyanVM]]

Lovely, thanks :). Don't hesitate to include recent Try links in the future when requesting checkin as well.

Comment 57

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/83041db77fcf
Enable IntersectionObserver and tests. r=smaug, r=jet

Comment 58

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out for frequently failing test_intersectionobservers.html:

https://hg.mozilla.org/integration/autoland/rev/edbc280464588c955d0b184e37b661cc5985ccc7

A push with failures: https://treeherder.mozilla.org/logviewer.html#?job_id=91441042&repo=autoland
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=91441042&repo=autoland

[task 2017-04-13T20:49:26.669319Z] 20:49:26     INFO - TEST-PASS | dom/base/test/test_intersectionobservers.html | boundingClientRect matches target.getBoundingClientRect() for an element inside an iframe [observe subframe] 
[task 2017-04-13T20:49:26.671435Z] 20:49:26     INFO - Buffered messages finished
[task 2017-04-13T20:49:26.674169Z] 20:49:26     INFO - TEST-UNEXPECTED-FAIL | dom/base/test/test_intersectionobservers.html | rootBounds should is set to null for cross-origin observations [observe subframe] 
[task 2017-04-13T20:49:26.676908Z] 20:49:26     INFO -     get be/fn.ok@dom/base/test/test_intersectionobservers.html:97:13
[task 2017-04-13T20:49:26.678630Z] 20:49:26     INFO -     window.onmessage@dom/base/test/test_intersectionobservers.html:973:11
[task 2017-04-13T20:49:26.680859Z] 20:49:26     INFO -     EventHandlerNonNull*@dom/base/test/test_intersectionobservers.html:972:9
[task 2017-04-13T20:49:26.682889Z] 20:49:26     INFO -     next@dom/base/test/test_intersectionobservers.html:144:9
[task 2017-04-13T20:49:26.684948Z] 20:49:26     INFO -     next/<@dom/base/test/test_intersectionobservers.html:146:11
[task 2017-04-13T20:49:26.686916Z] 20:49:26     INFO -     io<@dom/base/test/test_intersectionobservers.html:959:11
[task 2017-04-13T20:49:26.689039Z] 20:49:26     INFO -     IntersectionCallback*@dom/base/test/test_intersectionobservers.html:953:14
[task 2017-04-13T20:49:26.691277Z] 20:49:26     INFO -     next@dom/base/test/test_intersectionobservers.html:144:9
[task 2017-04-13T20:49:26.693733Z] 20:49:26     INFO -     next/<@dom/base/test/test_intersectionobservers.html:146:11
[task 2017-04-13T20:49:26.695537Z] 20:49:26     INFO -     targetEl4.onload/<@dom/base/test/test_intersectionobservers.html:942:13

Comment 59

[Tobias Schneider [:tobytailor]]

Mh, did that Try task run with the patch from Bug 1313972?

Comment 60

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/25b64d100bda
Enable IntersectionObserver and tests. r=smaug, r=jet

Comment 61

[Phil Ringnalda (:philor)]

Ah, Groundhog Day is late this year, apparently.

Backed out again in https://hg.mozilla.org/integration/autoland/rev/2a494e1f39e7

Comment 62

[Jet Villegas (inactive)]

(In reply to Phil Ringnalda (:philor) from comment #61)
> Ah, Groundhog Day is late this year, apparently.
> 
> Backed out again in
> https://hg.mozilla.org/integration/autoland/rev/2a494e1f39e7

The failing run:
https://hg.mozilla.org/integration/autoland/file/25b64d100bda232ad31a4e72fdb7b92e3220955f/dom/base/test

...doesn't seem to have the required test fixes already in m-c from bug 1313972:
https://hg.mozilla.org/mozilla-central/rev/05af3a08fb6f

Are the integration branches out of sync?

Comment 63

[Ryan VanderMeulen [:RyanVM]]

No, the reason this got backed out was due to a timeout this time:
https://treeherder.mozilla.org/logviewer.html#?job_id=91517297&repo=autoland

That's from a run after the test fix was merged to autoland. Comment 60 was literally the push prior to said merge arriving.

Comment 64

[Tobias Schneider [:tobytailor]]

If didn't have have the latest patch from Bug 1313972 then the timeout was most likely from a regression introduced by the earlier patch in that same bug. The latest patch was supposed to revert that.

Comment 65

[Ryan VanderMeulen [:RyanVM]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #63)
> That's from a run after the test fix was merged to autoland.

The timeouts started on autoland *after* bug 1313972 was merged there, per the above comment.

Comment 66

[Tobias Schneider [:tobytailor]]

Was that the first try run after the patches landed? I see a couple other timeouts, which might be just coincidence tho.

Comment 67

[Ryan VanderMeulen [:RyanVM]]

https://treeherder.mozilla.org/#/jobs?repo=autoland&filter-searchStr=linux%20m(2)&tochange=2a494e1f39e78f9bb25681a3b6616c6697e48685

Comment 68

[Tobias Schneider [:tobytailor]]

Latest try run finally looking good after Bug 1354163, 1324135 and 1313972 being fixed: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8b443f737b08d934457b900e83707dd021c306cf

Comment 69

[Jet Villegas (inactive)]

Comment on attachment 8819102 [details] [diff] [review]
Enable Tests. r=smaug


> [test_intersectionobservers.html]
>-skip-if = true # Track Bug 1320704
>+skip-if = (os == "android") # Timing issues

I'm not happy about shipping on Android without tests. Either disable the feature on Android or enable the tests.

Comment 70

[Jet Villegas (inactive)]

(In reply to Jet Villegas (:jet) from comment #69)
> Comment on attachment 8819102 [details] [diff] [review]
> Enable Tests. r=smaug
> 
> 
> > [test_intersectionobservers.html]
> >-skip-if = true # Track Bug 1320704
> >+skip-if = (os == "android") # Timing issues
> 
> I'm not happy about shipping on Android without tests. Either disable the
> feature on Android or enable the tests.

To be clear, I strongly prefer enabling the tests but can live with a follow-up bug that enables both the feature and tests on Android.

Comment 71

[Tobias Schneider [:tobytailor]]

I just kicked of another try run to see if the timing issues we had on Android in the past are still a concern after latest test fixes landed. See https://treeherder.mozilla.org/#/jobs?repo=try&revision=2752540554df9a496acf3fee2ab584c2658413da.

Comment 72

[Tobias Schneider [:tobytailor]]

Attachment: Enable Intersection Observer tests on Android

Looks like its save now to enable all tests on Android.

Comment 73

[Jet Villegas (inactive)]

Comment on attachment 8859343 [details] [diff] [review]
Enable Intersection Observer tests on Android

LGTM, Thx!

Comment 74

[Ryan VanderMeulen [:RyanVM]]

FWIW, when dealing with flaky tests, you almost certainly need to retrigger the run multiple times to get a good feel for whether it's truly working better or not. I went ahead and did so and it appears that bug 1313927 is still happening on Android.
https://treeherder.mozilla.org/logviewer.html#?job_id=92516193&repo=try

Comment 75

[Jet Villegas (inactive)]

(In reply to [Out of Office Until 24-April] Ryan VanderMeulen [:RyanVM] from comment #74)
> FWIW, when dealing with flaky tests, you almost certainly need to retrigger
> the run multiple times to get a good feel for whether it's truly working
> better or not. I went ahead and did so and it appears that bug 1313927 is
> still happening on Android.
> https://treeherder.mozilla.org/logviewer.html#?job_id=92516193&repo=try

We'll need to disable just that one test on Android due to multi-window flakiness on that platform. That seems to do the trick:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3f7bdcdede610a083f6de5a6fb78e387e91d12a5

Comment 76

[Tobias Schneider [:tobytailor]]

Try run with patche from Bug 1313927: https://treeherder.mozilla.org/#/jobs?repo=try&revision=48324feaf319a2f1a3878f64d15d81035bfb975c

Comment 77

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Canceled the Try pushes for this bug because they run everything 20 times, causing a huge backlog on Windows 7 and OSX.

Comment 78

[Tobias Schneider [:tobytailor]]

New, single, try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3f92377d96988126fe5356911c3133cffb24e1b7

Comment 79

[Jet Villegas (inactive)]

dholbert will land the changesets once Try completes and results look good.

Comment 80

[Daniel Holbert [:dholbert]]

After looking at this a bit closer, to prepare for landing, I left a bit of feedback on bug 1313927 comment 39 - 40 and in #layout.

Ideally, I'd like for us to structure things so we can land the pref-flip patch as an atomic change (separate from the test changes), so that we can trivially flip the pref again to disable this feature, if we need to.  That is a goal worth striving for, for *all* pref-controlled features -- it's not always possible, but I think it's pretty easy to do here -- I just posted in #layout with some thoughts on how we can accomplish that by tweaking bug 1313927's patch.  I don't think that should hold this up too much -- though it might mean we want one more Try run.

(Inbound seems to be closed for the time being, too, FWIW, so we can't land this for a little while regardless.)

Comment 81

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b66b6bb87d11
Enable IntersectionObserver tests. r=jet
https://hg.mozilla.org/integration/mozilla-inbound/rev/32dbe6ca353f
Enable IntersectionObserver. r=jet

Comment 82

[Daniel Holbert [:dholbert]]

Patches landed. I took the patches from this Try push:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=95acba62d571c397d08bed74cf963d9f700f9e7e

...and I did the following:
 - Adjusted the test patch on this bug to leave the test disabled on android, so that this bug's patches can stand on their own without bug 1313927.
 - Updated bug 1313927 to remove that annotation when it splits the test. (So we end up in the same state, but with each intermediate point building&passing tests)
 - Tested this bug's amended patches on their own, and then with bug 1313927 layered on top:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=a8b06a3bf9de769e26b26c615ef400cf5f795212
https://treeherder.mozilla.org/#/jobs?repo=try&revision=8fba39752893521bb2a0804f9c67f57bd2c5eede
(Ignore the dom mochitest failure there that produced a bunch of orange 7's -- that's unrelated inbound bustage)
 - Added "r=jet" to them (I'm assuming they were substantially similar to the ones on the bug here)
 - Landed!

Comment 83

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b66b6bb87d11
https://hg.mozilla.org/mozilla-central/rev/32dbe6ca353f

Comment 84

[Tobias Schneider [:tobytailor]]

Attachment: Patch for beta uplift (Enable tests only)

Comment 85

[Tobias Schneider [:tobytailor]]

Comment on attachment 8864639 [details] [diff] [review]
Patch for beta uplift (Enable tests only)

Approval Request Comment
[Feature/Bug causing the regression]: N/A
[User impact if declined]: IntersectionObserver API not available. Chrome already ships this feature.
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: Yes
[List of other uplifts needed for the feature/fix]: 1353529, 1313972, 1324135, 1313927

Comment 86

[Tobias Schneider [:tobytailor]]

Attachment: Patch for beta uplift (Enable tests only)

Comment 87

[Tobias Schneider [:tobytailor]]

Attachment: Patch for beta uplift (Enable tests only)

Comment 88

[Tobias Schneider [:tobytailor]]

Attachment: Patch for beta uplift (Enable tests only)

Comment 89

[Tobias Schneider [:tobytailor]]

Attachment: Patch for beta uplift (Enable tests only)

Comment 90

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/d833c4cac4a7

Comment 91

[Ryan VanderMeulen [:RyanVM]]

I had to disable test_intersectionobservers.html due to failures on Beta:
https://treeherder.mozilla.org/logviewer.html#?job_id=96759262&repo=mozilla-beta

https://hg.mozilla.org/releases/mozilla-beta/rev/4b40534bf2f8

Comment 92

[Tobias Schneider [:tobytailor]]

Can we re-land the updated patch in https://bugzilla.mozilla.org/show_bug.cgi?id=1313972 and re-enable?

Comment 93

[Ryan VanderMeulen [:RyanVM]]

Ran it through Try and it looks good.
https://hg.mozilla.org/releases/mozilla-beta/rev/e767c39dad3a

Comment 94

[Jet Villegas (inactive)]

The Beta patches don't enable IntersectionObserver in FF54, just the tests (which use SpecialPowers to enable the feature for test runs.) The plan is to test IntersectionObserver in Beta via an Experiment.

Comment 95

[Tobias Schneider [:tobytailor]]

Yes, we only uplifted the test enabling part here. Landing the pref-flipping patches will happen via Bug 1362168 if the experiment (Bug 1362418) was successful.

Comment 96

[Eric Shepherd [:sheppy]]

The Intersection Observer docs are essentially done; there are a few things I'd like to polish (primarily around diagrams and text to help clarify how things work in places), but for now this is as done as it will be for the next little while. See bug 1386607 for that work.