IntersectionObserver: use an idle callback to send notifications.

IntersectionObserver: use an idle callback to send notifications.

With this change, the tests can no longer use setTimeout(0) to wait
for notifications to be delivered.  Instead, use takeRecords() to
proactively grab notifications right after they are generated
(typically in a RAF right after a layout change).

BUG=540528
R=ojan@chromium.org,haraken@chromium.org

Committed: https://crrev.com/2c168f38b5c0e4e50374be4e54c44901c60738a9
Cr-Commit-Position: refs/heads/master@{#380278}

[szager1, 2016-03-08 01:05:12]

haraken, would you please review the oilpan-related bits?

[ojan, 2016-03-08 01:37:10]

ojan@chromium.org changed reviewers:
+ skyostil@chromium.org

[ojan, 2016-03-08 01:37:11]

Sami, mind doing a once over on the C++ code here since you know idle callback
things? lgtm

https://codereview.chromium.org/1776493002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/http/tests/intersection-observer/iframe-cross-origin.html
(right):

https://codereview.chromium.org/1776493002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/intersection-observer/iframe-cross-origin.html:67:
requestAnimationFrame(() => { iframe.contentWindow.postMessage("", "*") })
Why do you need 2 rafs here?

[haraken, 2016-03-08 03:44:03]

LGTM, but please wait for Sami's approval.

https://codereview.chromium.org/1776493002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/dom/IntersectionObserverController.cpp
(right):

https://codereview.chromium.org/1776493002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/IntersectionObserverController.cpp:43:
options.setTimeout(100);

We want to remove hard-coded timers in the future though... For example, 100 ms
would have a different meaning on desktops and low-RAM devices.

[Sami, 2016-03-08 17:32:42]

Thanks for doing this, seems like a good idea. I left a few comments.

https://codereview.chromium.org/1776493002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/http/tests/intersection-observer/iframe-cross-origin.html
(right):

https://codereview.chromium.org/1776493002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/intersection-observer/iframe-cross-origin.html:67:
requestAnimationFrame(() => { iframe.contentWindow.postMessage("", "*") })
On 2016/03/08 01:37:11, ojan wrote:
> Why do you need 2 rafs here?

Right, this pattern seems to be repeated in a bunch of the other tests too.
Maybe reduce it to one if you can or add a common helper function that
encapsulates the logic?

Since the idle callbacks are scheduled in the lifecycle update, they should get
serviced within the same frame assuming there is idle time. This might get flaky
if there isn't idle time, so maybe we could use some explicit callback instead?
FWIW requestIdleCallbacks run in the requested order, but I guess the spec
doesn't really say whether those have any relation to intersection observer
notifications.

https://codereview.chromium.org/1776493002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/intersection-observer/observer-without-js-reference.html
(right):

https://codereview.chromium.org/1776493002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/intersection-observer/observer-without-js-reference.html:22:
}, 100);
Is there a chance this could add some flake?

https://codereview.chromium.org/1776493002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/dom/IntersectionObserverController.cpp
(right):

https://codereview.chromium.org/1776493002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/IntersectionObserverController.cpp:23: ,
m_timerIsScheduled(false)
nit: s/timer/callback/ (here and below)?

https://codereview.chromium.org/1776493002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/IntersectionObserverController.cpp:32: //
TODO(szager): use idle callback with a timeout.  Until we do that, there is no
Did you mean to remove this TODO?

[szager1, 2016-03-08 18:51:32]

https://codereview.chromium.org/1776493002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/http/tests/intersection-observer/iframe-cross-origin.html
(right):

https://codereview.chromium.org/1776493002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/intersection-observer/iframe-cross-origin.html:67:
requestAnimationFrame(() => { iframe.contentWindow.postMessage("", "*") })
On 2016/03/08 17:32:41, Sami wrote:
> On 2016/03/08 01:37:11, ojan wrote:
> > Why do you need 2 rafs here?
> 
> Right, this pattern seems to be repeated in a bunch of the other tests too.
> Maybe reduce it to one if you can or add a common helper function that
> encapsulates the logic?
> 
> Since the idle callbacks are scheduled in the lifecycle update, they should
get
> serviced within the same frame assuming there is idle time. This might get
flaky
> if there isn't idle time, so maybe we could use some explicit callback
instead?
> FWIW requestIdleCallbacks run in the requested order, but I guess the spec
> doesn't really say whether those have any relation to intersection observer
> notifications.

In general, the double-RAF pattern is necessary because the order of events is
something like this:

- onload happens, possibly after a frame has been generated.  Layout is changed
in onload, but the changes won't get rendered until the next frame.

- On the next frame, IntersectionObserver runs its algorithm based on the
previous layout, so no notifications are generated.  The layout change from
onload is rendered.

- One the next frame, IntersectionObserver runs its algorithm based on the new
layout, and generated the expected notifications.

It's racy based on when onload runs, but the double-RAF should avoid any
problems.  All subsequent test steps used a single RAF.

It's possible that for this particular test, because it uses postMessage, the
double-RAF is unnecessary.  I could take it out and see if the test gets flaky. 
I could also just leave it in.

https://codereview.chromium.org/1776493002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/intersection-observer/observer-without-js-reference.html
(right):

https://codereview.chromium.org/1776493002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/intersection-observer/observer-without-js-reference.html:22:
}, 100);
On 2016/03/08 17:32:42, Sami wrote:
> Is there a chance this could add some flake?

In other tests, I added a call to observer.takeRecords() to ensure that the test
wasn't missing any pending notifications.  This test is meant to cover the case
where there are no remaining js references to the observer, so it can't use
takeRecords in this way.

However, the IO spec guarantees that notifications will be delivered within
100ms, so this tests conformance to that spec.

https://codereview.chromium.org/1776493002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/dom/IntersectionObserverController.cpp
(right):

https://codereview.chromium.org/1776493002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/IntersectionObserverController.cpp:23: ,
m_timerIsScheduled(false)
On 2016/03/08 17:32:42, Sami wrote:
> nit: s/timer/callback/ (here and below)?

Done.

https://codereview.chromium.org/1776493002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/IntersectionObserverController.cpp:32: //
TODO(szager): use idle callback with a timeout.  Until we do that, there is no
On 2016/03/08 17:32:42, Sami wrote:
> Did you mean to remove this TODO?

Done.

https://codereview.chromium.org/1776493002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/IntersectionObserverController.cpp:43:
options.setTimeout(100);
On 2016/03/08 03:44:03, haraken wrote:
> 
> We want to remove hard-coded timers in the future though... For example, 100
ms
> would have a different meaning on desktops and low-RAM devices.
> 

100ms is required by the IntersectionObserver spec.  I suppose we might opt for
a smaller number here under some circumstances, but I think we can cross that
bridge when we come to it.

[haraken, 2016-03-08 23:05:58]

https://codereview.chromium.org/1776493002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/dom/IntersectionObserverController.cpp
(right):

https://codereview.chromium.org/1776493002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/IntersectionObserverController.cpp:43:
options.setTimeout(100);
On 2016/03/08 18:51:32, szager1 wrote:
> On 2016/03/08 03:44:03, haraken wrote:
> > 
> > We want to remove hard-coded timers in the future though... For example, 100
> ms
> > would have a different meaning on desktops and low-RAM devices.
> > 
> 
> 100ms is required by the IntersectionObserver spec.  I suppose we might opt
for
> a smaller number here under some circumstances, but I think we can cross that
> bridge when we come to it.

Then maybe worth adding a comment and mention that this is not an arbitrary
selected number.

[szager1, 2016-03-08 23:12:02]

https://codereview.chromium.org/1776493002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/dom/IntersectionObserverController.cpp
(right):

https://codereview.chromium.org/1776493002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/IntersectionObserverController.cpp:43:
options.setTimeout(100);
On 2016/03/08 23:05:58, haraken wrote:
> On 2016/03/08 18:51:32, szager1 wrote:
> > On 2016/03/08 03:44:03, haraken wrote:
> > > 
> > > We want to remove hard-coded timers in the future though... For example,
100
> > ms
> > > would have a different meaning on desktops and low-RAM devices.
> > > 
> > 
> > 100ms is required by the IntersectionObserver spec.  I suppose we might opt
> for
> > a smaller number here under some circumstances, but I think we can cross
that
> > bridge when we come to it.
> 
> Then maybe worth adding a comment and mention that this is not an arbitrary
> selected number.

Done.

[Sami, 2016-03-09 18:29:55]

Thanks, lgtm overall.

https://codereview.chromium.org/1776493002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/http/tests/intersection-observer/iframe-cross-origin.html
(right):

https://codereview.chromium.org/1776493002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/intersection-observer/iframe-cross-origin.html:67:
requestAnimationFrame(() => { iframe.contentWindow.postMessage("", "*") })
On 2016/03/08 18:51:32, szager1 wrote:
> On 2016/03/08 17:32:41, Sami wrote:
> > On 2016/03/08 01:37:11, ojan wrote:
> > > Why do you need 2 rafs here?
> > 
> > Right, this pattern seems to be repeated in a bunch of the other tests too.
> > Maybe reduce it to one if you can or add a common helper function that
> > encapsulates the logic?
> > 
> > Since the idle callbacks are scheduled in the lifecycle update, they should
> get
> > serviced within the same frame assuming there is idle time. This might get
> flaky
> > if there isn't idle time, so maybe we could use some explicit callback
> instead?
> > FWIW requestIdleCallbacks run in the requested order, but I guess the spec
> > doesn't really say whether those have any relation to intersection observer
> > notifications.
> 
> In general, the double-RAF pattern is necessary because the order of events is
> something like this:
> 
> - onload happens, possibly after a frame has been generated.  Layout is
changed
> in onload, but the changes won't get rendered until the next frame.
> 
> - On the next frame, IntersectionObserver runs its algorithm based on the
> previous layout, so no notifications are generated.  The layout change from
> onload is rendered.
> 
> - One the next frame, IntersectionObserver runs its algorithm based on the new
> layout, and generated the expected notifications.
> 
> It's racy based on when onload runs, but the double-RAF should avoid any
> problems.  All subsequent test steps used a single RAF.
> 
> It's possible that for this particular test, because it uses postMessage, the
> double-RAF is unnecessary.  I could take it out and see if the test gets
flaky. 
> I could also just leave it in.

I thought IO runs after layout, so the notifications are based on what is
currently being rendered, not the previous layout? Or is the first frame a
special case since we don't have anything to compare against?

In any case, could you add that as a comment here to make it more obvious that
this is deliberate?

https://codereview.chromium.org/1776493002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/intersection-observer/observer-without-js-reference.html
(right):

https://codereview.chromium.org/1776493002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/intersection-observer/observer-without-js-reference.html:22:
}, 100);
On 2016/03/08 18:51:32, szager1 wrote:
> On 2016/03/08 17:32:42, Sami wrote:
> > Is there a chance this could add some flake?
> 
> In other tests, I added a call to observer.takeRecords() to ensure that the
test
> wasn't missing any pending notifications.  This test is meant to cover the
case
> where there are no remaining js references to the observer, so it can't use
> takeRecords in this way.
> 
> However, the IO spec guarantees that notifications will be delivered within
> 100ms, so this tests conformance to that spec.

I see, that makes sense. Maybe also add corresponding comment here?

[szager1, 2016-03-09 20:45:25]

https://codereview.chromium.org/1776493002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/http/tests/intersection-observer/iframe-cross-origin.html
(right):

https://codereview.chromium.org/1776493002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/http/tests/intersection-observer/iframe-cross-origin.html:67:
requestAnimationFrame(() => { iframe.contentWindow.postMessage("", "*") })
On 2016/03/09 18:29:55, Sami wrote:
> On 2016/03/08 18:51:32, szager1 wrote:
> > On 2016/03/08 17:32:41, Sami wrote:
> > > On 2016/03/08 01:37:11, ojan wrote:
> > > > Why do you need 2 rafs here?
> > > 
> > > Right, this pattern seems to be repeated in a bunch of the other tests
too.
> > > Maybe reduce it to one if you can or add a common helper function that
> > > encapsulates the logic?
> > > 
> > > Since the idle callbacks are scheduled in the lifecycle update, they
should
> > get
> > > serviced within the same frame assuming there is idle time. This might get
> > flaky
> > > if there isn't idle time, so maybe we could use some explicit callback
> > instead?
> > > FWIW requestIdleCallbacks run in the requested order, but I guess the spec
> > > doesn't really say whether those have any relation to intersection
observer
> > > notifications.
> > 
> > In general, the double-RAF pattern is necessary because the order of events
is
> > something like this:
> > 
> > - onload happens, possibly after a frame has been generated.  Layout is
> changed
> > in onload, but the changes won't get rendered until the next frame.
> > 
> > - On the next frame, IntersectionObserver runs its algorithm based on the
> > previous layout, so no notifications are generated.  The layout change from
> > onload is rendered.
> > 
> > - One the next frame, IntersectionObserver runs its algorithm based on the
new
> > layout, and generated the expected notifications.
> > 
> > It's racy based on when onload runs, but the double-RAF should avoid any
> > problems.  All subsequent test steps used a single RAF.
> > 
> > It's possible that for this particular test, because it uses postMessage,
the
> > double-RAF is unnecessary.  I could take it out and see if the test gets
> flaky. 
> > I could also just leave it in.
> 
> I thought IO runs after layout, so the notifications are based on what is
> currently being rendered, not the previous layout? Or is the first frame a
> special case since we don't have anything to compare against?
> 
> In any case, could you add that as a comment here to make it more obvious that
> this is deliberate?

Actually, my explanation was totally wrong.  I added
LayoutTests/intersection-observer/README with a full explanation, and added
comments referring to the README in each test.

https://codereview.chromium.org/1776493002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/intersection-observer/observer-without-js-reference.html
(right):

https://codereview.chromium.org/1776493002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/intersection-observer/observer-without-js-reference.html:22:
}, 100);
On 2016/03/09 18:29:55, Sami wrote:
> On 2016/03/08 18:51:32, szager1 wrote:
> > On 2016/03/08 17:32:42, Sami wrote:
> > > Is there a chance this could add some flake?
> > 
> > In other tests, I added a call to observer.takeRecords() to ensure that the
> test
> > wasn't missing any pending notifications.  This test is meant to cover the
> case
> > where there are no remaining js references to the observer, so it can't use
> > takeRecords in this way.
> > 
> > However, the IO spec guarantees that notifications will be delivered within
> > 100ms, so this tests conformance to that spec.
> 
> I see, that makes sense. Maybe also add corresponding comment here?

Done.

[szager1, 2016-03-09 20:45:32]

The CQ bit was checked by szager@chromium.org

[szager1, 2016-03-09 20:45:32]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, ojan@chromium.org, skyostil@chromium.org
Link to the patchset: https://codereview.chromium.org/1776493002/#ps60001
(title: "Add explanation of double-RAF in tests")

[commit-bot: I haz the power, 2016-03-09 20:46:04]

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

[commit-bot: I haz the power, 2016-03-09 22:01:34]

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

[commit-bot: I haz the power, 2016-03-09 22:01:35]

Failed to apply patch for
third_party/WebKit/Source/core/dom/IntersectionObserverController.cpp:
While running git apply --index -3 -p1;
  error: patch failed:
third_party/WebKit/Source/core/dom/IntersectionObserverController.cpp:28
  Falling back to three-way merge...
  Applied patch to
'third_party/WebKit/Source/core/dom/IntersectionObserverController.cpp' with
conflicts.
  U third_party/WebKit/Source/core/dom/IntersectionObserverController.cpp

Patch:      
third_party/WebKit/Source/core/dom/IntersectionObserverController.cpp
Index: third_party/WebKit/Source/core/dom/IntersectionObserverController.cpp
diff --git
a/third_party/WebKit/Source/core/dom/IntersectionObserverController.cpp
b/third_party/WebKit/Source/core/dom/IntersectionObserverController.cpp
index
15cfa0c58d3abf30b990eb31af2990fcf99f6554..845e84caeac0d6fd6094b8c2b43cb40004134290
100644
--- a/third_party/WebKit/Source/core/dom/IntersectionObserverController.cpp
+++ b/third_party/WebKit/Source/core/dom/IntersectionObserverController.cpp
@@ -5,6 +5,7 @@
 #include "core/dom/IntersectionObserverController.h"
 
 #include "core/dom/Document.h"
+#include "core/dom/IdleRequestOptions.h"
 
 namespace blink {
 
@@ -19,8 +20,8 @@ IntersectionObserverController*
IntersectionObserverController::create(Document*
 
 IntersectionObserverController::IntersectionObserverController(Document*
document)
     : ActiveDOMObject(document)
-    , m_timer(this,
&IntersectionObserverController::deliverIntersectionObservations)
-    , m_timerFiredWhileSuspended(false)
+    , m_callbackIsScheduled(false)
+    , m_callbackFiredWhileSuspended(false)
 {
 }
 
@@ -28,28 +29,39 @@
IntersectionObserverController::~IntersectionObserverController() { }
 
 void
IntersectionObserverController::scheduleIntersectionObserverForDelivery(IntersectionObserver&
observer)
 {
-    // TODO(szager): use idle callback with a timeout.  Until we do that, there
is no
-    // reliable way to write a test for takeRecords, because it's impossible to
guarantee
-    // that javascript will get a chance to run before the timer fires.
-    if (!m_timer.isActive())
-        m_timer.startOneShot(0, BLINK_FROM_HERE);
     m_pendingIntersectionObservers.add(&observer);
+    if (m_callbackIsScheduled)
+        return;
+    Document* document = toDocument(executionContext());
+    if (!document)
+        return;
+    m_callbackIsScheduled = true;
+    IdleRequestOptions options;
+    // The IntersectionObserver spec mandates that notifications be sent within
100ms.
+    options.setTimeout(100);
+    document->requestIdleCallback(this, options);
 }
 
 void IntersectionObserverController::resume()
 {
-    // If the timer fired while DOM objects were suspended, notifications might
be late, so deliver
-    // them right away (rather than waiting for m_timer to fire again).
-    if (m_timerFiredWhileSuspended) {
-        m_timerFiredWhileSuspended = false;
-        deliverIntersectionObservations(nullptr);
+    // If the callback fired while DOM objects were suspended, notifications
might be late, so deliver
+    // them right away (rather than waiting to fire again).
+    if (m_callbackFiredWhileSuspended) {
+        m_callbackFiredWhileSuspended = false;
+        deliverIntersectionObservations();
     }
 }
 
-void
IntersectionObserverController::deliverIntersectionObservations(Timer<IntersectionObserverController>*)
+void IntersectionObserverController::handleEvent(IdleDeadline*)
+{
+    m_callbackIsScheduled = false;
+    deliverIntersectionObservations();
+}
+
+void IntersectionObserverController::deliverIntersectionObservations()
 {
     if (executionContext()->activeDOMObjectsAreSuspended()) {
-        m_timerFiredWhileSuspended = true;
+        m_callbackFiredWhileSuspended = true;
         return;
     }
     IntersectionObserverVector observers;
@@ -88,6 +100,7 @@ DEFINE_TRACE(IntersectionObserverController)
     visitor->trace(m_trackedIntersectionObservers);
     visitor->trace(m_pendingIntersectionObservers);
     ActiveDOMObject::trace(visitor);
+    IdleRequestCallback::trace(visitor);
 }
 
 } // namespace blink

[szager1, 2016-03-09 22:15:02]

The CQ bit was checked by szager@chromium.org

[szager1, 2016-03-09 22:15:02]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, skyostil@chromium.org, ojan@chromium.org
Link to the patchset: https://codereview.chromium.org/1776493002/#ps80001
(title: "rebase")

[commit-bot: I haz the power, 2016-03-09 22:15:18]

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

[commit-bot: I haz the power, 2016-03-09 22:25:24]

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

[commit-bot: I haz the power, 2016-03-09 22:25:26]

Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[szager1, 2016-03-09 23:04:19]

The CQ bit was checked by szager@chromium.org

[szager1, 2016-03-09 23:04:20]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, skyostil@chromium.org, ojan@chromium.org
Link to the patchset: https://codereview.chromium.org/1776493002/#ps100001
(title: "syntax error")

[commit-bot: I haz the power, 2016-03-09 23:04:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1776493002/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1776493002/100001

[commit-bot: I haz the power, 2016-03-10 01:04:49]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-03-10 01:06:25]

Description was changed from

==========
IntersectionObserver: use an idle callback to send notifications.

With this change, the tests can no longer use setTimeout(0) to wait
for notifications to be delivered.  Instead, use takeRecords() to
proactively grab notifications right after they are generated
(typically in a RAF right after a layout change).

BUG=540528
R=ojan@chromium.org,haraken@chromium.org
==========

to

==========
IntersectionObserver: use an idle callback to send notifications.

With this change, the tests can no longer use setTimeout(0) to wait
for notifications to be delivered.  Instead, use takeRecords() to
proactively grab notifications right after they are generated
(typically in a RAF right after a layout change).

BUG=540528
R=ojan@chromium.org,haraken@chromium.org

Committed: https://crrev.com/2c168f38b5c0e4e50374be4e54c44901c60738a9
Cr-Commit-Position: refs/heads/master@{#380278}
==========

[commit-bot: I haz the power, 2016-03-10 01:06:26]

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

[sof, 2016-03-10 08:38:54]

A revert of this CL (patchset #6 id:100001) has been created in
https://codereview.chromium.org/1780163002/ by sigbjornf@opera.com.

The reason for reverting is: Tests added are leaking,
https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Leak/b....