| OLD | NEW | 
|   1 All of the IntersectionObserver tests feature the following idiom: |   1 All of the IntersectionObserver tests feature the following idiom: | 
|   2  |   2  | 
|   3 <script> |   3 <script> | 
|   4 var observer = new IntersectionObserver(...) |   4 var observer = new IntersectionObserver(...) | 
|   5 function test_function() { |   5 function test_function() { | 
|   6   var entries = observer.takeRecords(); |   6   var entries = observer.takeRecords(); | 
|   7   // Verify entries |   7   // Verify entries | 
|   8 } |   8 } | 
|   9 onload = function() { |   9 onload = function() { | 
|  10   observer.observe(target); |  10   observer.observe(target); | 
|  11   requestAnimationFrame(() => { requestAnimationFrame(test_function) }); |  11   requestAnimationFrame(() => { | 
 |  12     setTimeout(() => { | 
 |  13       setTimeout(test_function); | 
 |  14     }); | 
 |  15   }); | 
|  12 } |  16 } | 
|  13  |  17  | 
|  14 Subsequent steps in the test use a single RAF to give the observer a chance to |  | 
|  15 generate notifications, but the double RAF is required in the onload handler. |  | 
|  16 Here's the chain of events: |  18 Here's the chain of events: | 
|  17  |  19  | 
|  18 - onload |  20 - onload | 
|  19   - observer.observe() |  21   - observer.observe() | 
|  20   - First RAF handler is registered |  22   - RAF handler is registered | 
|  21 - BeginFrame |  23 - BeginFrame | 
|  22   - RAF handlers run |  24   - RAF handler runs | 
|  23     - Second RAF handler is registered  |  25     - First setTimeout is registered  | 
|  24   - UpdateAllLifecyclePhases |  26   - UpdateAllLifecyclePhases | 
|  25     - IntersectionObserver generates notifications |  27     - IntersectionObserver generates notifications | 
 |  28   - First setTimeout runs | 
 |  29     - Second setTimeout is regsitered | 
 |  30   - Notifications that haven't be taken via takeRecords are delivered | 
 |  31   - Second setTimeout runs and queues another RAF handler | 
|  26 - BeginFrame |  32 - BeginFrame | 
|  27   - RAF handlers run |  33   - RAF handler runs | 
|  28     - Second RAF handler verifies observer notifications. |  34     - RAF handler verifies observer notifications via takeRecords or | 
|  29  |  35       does the setTimeout dance to respond after they've been delivered. | 
|  30 #------------------------------------------------------------------------------- |  | 
|  31  |  | 
|  32 All of the IntersectionObserver tests are currently listed in LeakExpecations. |  | 
|  33  |  | 
|  34 To avoid leaking DOM objects, the tests must ensure that all posted tasks have |  | 
|  35 run before exiting.  IntersectionObserverController requests an idle callback |  | 
|  36 through the document's ScriptedIdleTaskController with a timeout of 100ms.  That |  | 
|  37 callback must be allowed to run before the test finishes to avoid a leak. |  | 
|  38  |  | 
|  39 ScriptedIdleTaskController posts two tasks to the WebScheduler: one to run at |  | 
|  40 the next idle time, and another to run when the timeout expires. |  | 
|  41 crbug.com/595155 explains that when one of those tasks runs, it does not release |  | 
|  42 its reference to the ScriptedIdleTaskController, which is needlessly kept alive |  | 
|  43 until the both tasks have fired.  If a test exits before both tasks have fired, |  | 
|  44 it will have a DOM leak. |  | 
|  45  |  | 
|  46 crbug.com/595152 explains that when running without the threaded compositor -- |  | 
|  47 as the layout tests do -- idle tasks are never serviced.  They will still run |  | 
|  48 when their timeout expires, but the idle task posted by |  | 
|  49 ScriptedIdleTaskController will never run. |  | 
|  50  |  | 
|  51 Fixing the first bug means that requestIdleCallback will not leak as long as it |  | 
|  52 actually runs before exit.  The second bug means that when running without the |  | 
|  53 threaded compositor, requestIdleCallback will only ever run when its timeout |  | 
|  54 expires. |  | 
|  55  |  | 
|  56 The upshot of all of this is that the only way to ensure that |  | 
|  57 IntersectionObserver tests don't leak is to ensure that their idle tasks all run |  | 
|  58 before the tests exit, and those idle tasks will only run when their 100ms |  | 
|  59 timeout expires. |  | 
|  60  |  | 
|  61 Note that all of this still holds when observer.takeRecords() is called before |  | 
|  62 the idle task runs.  In that case, the idle task is not cancelled (because the |  | 
|  63 idle task needs to service all observers tracked by a given |  | 
|  64 IntersectionObserverController); when the idle task runs, it's simply a no-op. |  | 
|  65 There is no way in javascript to detect that such a no-op has occurred, but |  | 
|  66 using requestIdleCallback with a timeout of 100 should be guaranteed to run |  | 
|  67 after the IntersectionObserverController's idle task has run, as long as |  | 
|  68 ScriptedIdleTaskController honors first-in-first-out semantics. |  | 
|  69  |  | 
|  70 At the time of writing, there is a patch out to add testRunner.runIdleTasks(): |  | 
|  71  |  | 
|  72 https://codereview.chromium.org/1806133002/ |  | 
|  73  |  | 
|  74 With that patch, the tests can be removed from LeakExpectations as long as they |  | 
|  75 end with: |  | 
|  76  |  | 
|  77 if (window.testRunner) |  | 
|  78   testRunner.runIdleTasks(finishJSTest); |  | 
| OLD | NEW |