ResizeObserver pt6: integration

ResizeObserver pt6: integration

Integrates ResizeObserver with the browser.

1) Element notifies ResizeObserver when its size might have changed.

Operations that might change Element's observed size are trapped:
Element::detachLayoutTree traps Element removal from DOM tree
LayoutObject::setMayNeedPaintInvalidation traps size changes

2) Event loop integration

FrameView::notifyResizeObserver implements it per spec.
https://wicg.github.io/ResizeObserver/#html-event-loop

3) Tests

LayoutTests/resize-observer is a comprehensive test suite.

BUG=612962
Committed: https://crrev.com/5b49641e336ba9be7789cc0eb2511d415f4e6d5d
Committed: https://crrev.com/3f41316c60eb95369602fde0ebf52e935a3a91d5
Cr-Original-Commit-Position: refs/heads/master@{#411694}
Cr-Commit-Position: refs/heads/master@{#412149}

[atotic1, 2016-08-01 19:28:55]

Description was changed from

==========
ResizeObserver pt6: integration

Integrates ResizeObserver with the browser. Several parts:

1) Element notifies ResizeObserver when its size might have changed.
Operations that might change Element's observed size are trapped:
Element::detachLayoutTree traps Element removal from DOM tree
LayoutObject::setMayNeedPaintInvalidation traps size changes

2) Event loop integration
FrameView::notifyResizeObserver implements it per spec.
https://wicg.github.io/ResizeObserver/#html-event-loop

3) Tests
LayoutTests/resize-observer is a comprehensive test suite.

BUG=612962
==========

to

==========
ResizeObserver pt6: integration

Integrates ResizeObserver with the browser. 

1) Element notifies ResizeObserver when its size might have changed.

Operations that might change Element's observed size are trapped:
Element::detachLayoutTree traps Element removal from DOM tree
LayoutObject::setMayNeedPaintInvalidation traps size changes

2) Event loop integration

FrameView::notifyResizeObserver implements it per spec.
https://wicg.github.io/ResizeObserver/#html-event-loop

3) Tests

LayoutTests/resize-observer is a comprehensive test suite.

BUG=612962
==========

[atotic1, 2016-08-01 19:28:55]

atotic@google.com changed reviewers:
+ eae@chromium.org, esprehn@chromium.org

[atotic1, 2016-08-01 19:44:47]

This is the final CR for initial implementation of ResizeObserver. Please
review.

It adds integration with the rest of the browser, and the test suite.

One thing I'd like to do is run performance tests. I have new code inside 
LayoutObject::setMayNeedPaintInvalidation, and am unsure whether 
it affects performance. 

I've just learned about run-perf-tests, and will be running it after lunch.

[eae, 2016-08-01 22:30:41]

Please update CL with perf numbers when you have them, thanks!

https://codereview.chromium.org/2204503002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/dom/Element.h (right):

https://codereview.chromium.org/2204503002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/Element.h:585: void sizeMightHaveChanged();
sizeMayHaveChanged()

We tend to prefer the word May over Might in functions like this.

$ grep -Hrni MayHave Source | wc -l 
46
$ grep -Hrni MightHave Source | wc -l
0

https://codereview.chromium.org/2204503002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/frame/FrameView.cpp (right):

https://codereview.chromium.org/2204503002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/frame/FrameView.cpp:2518: if
(!m_frame->document()->resizeObserverController())
Use frame().document() instead (as the method is guaranteed to return a frame)
or add an explicit check to see if set.

[atotic1, 2016-08-02 23:29:08]

All code CR comments are done.

I've ran the WebKit performance tests overnight. Results are at
https://docs.google.com/spreadsheets/d/1R0DhitMEbfAo0FqufO293Tcp6Jm5_-7kceJF_...

I ran two test sets:
1) baseline tests with LayoutObject::setMayNeedPaintInvalidation ->
element.sizeMayHaveChanged() notification commented out
2) tests with notification in.

The results are in the spreadsheet. Column H shows difference in results between
two runs.
Tests marked in red are ones where difference is greater than error margin. This
is all approximate, because error margin itself is variable between two runs.
Out of 172 tests:
60 differed by more than margin of error
21 perfomed worse
39 performed better

It is hard to draw any firm conclusions because of natural test variabilty. To
get a better feel of what's happening, I counted the number of times
LayoutObject::setMayNeedPaintInvalidation was called.

This is what I found:
Hits/run  |  worse% by | margin error | test name
8             |    4.72%       |  3.46%         | /DOM/select-multiple-add:Runs
15           |    5.26%       | 3.42%          | /DOM/select-single-add:Runs
1000       |    5.86%       |  3.62%         | /DOM/select-single-remove:Runs
>38000  |    8.81%        |  6.12%        |  /Layout/floats_100_100_nested:Time
700         |  6.15%         |  3.48%        |  /Layout/hindi-line-layout:Runs

This sample is biased, I was looking for samples that confirm the "we are a bit
worse off" hypothesis.

My intuition based on >38000 calls example is that there is a tiny performance
hit that becomes detectable when LayoutObject::setMayNeedPaintInvalidation() is
called very large number of times (1000+).

https://codereview.chromium.org/2204503002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/dom/Element.h (right):

https://codereview.chromium.org/2204503002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/Element.h:585: void sizeMightHaveChanged();
On 2016/08/01 22:30:40, eae wrote:
> sizeMayHaveChanged()
> 
> We tend to prefer the word May over Might in functions like this.
> 
> $ grep -Hrni MayHave Source | wc -l 
> 46
> $ grep -Hrni MightHave Source | wc -l
> 0

Done.

https://codereview.chromium.org/2204503002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/frame/FrameView.cpp (right):

https://codereview.chromium.org/2204503002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/frame/FrameView.cpp:2518: if
(!m_frame->document()->resizeObserverController())
On 2016/08/01 22:30:40, eae wrote:
> Use frame().document() instead (as the method is guaranteed to return a frame)
> or add an explicit check to see if set.

Done.

[eae, 2016-08-02 23:33:29]

Thank you. Change itself looks good so from that point of view: LGTM

However, please check with Elliot and/or the paint team whether the potential
performance regression is acceptable or not before landing.

[esprehn, 2016-08-03 18:06:52]

This doesn't look like it handles iframes properly. We also shouldn't hook into
the paint invalidation logic, that's making you go through all the resize
observers when something like background color changes. You want to hook closer
to the code that deals with layout changes.

https://codereview.chromium.org/2204503002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/Element.cpp (right):

https://codereview.chromium.org/2204503002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Element.cpp:2671:
HeapHashMap<Member<ResizeObserver>, Member<ResizeObservation>>* roData =
resizeObserverData();
don't abbreviate, I'd either call this |data| or |observerData|

https://codereview.chromium.org/2204503002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Element.cpp:2672: if (roData) {
I'd early return:

if (!data)
  return;

You can en use auto here to make this function very compact.

if (auto* data = resizeObserverData()) {
  for (auto& observation : data->values())
    observation->elementSizeChanged()
}

https://codereview.chromium.org/2204503002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Element.cpp:2673: for (auto& iter : *roData)
->values() and make the loop variable |observation|

https://codereview.chromium.org/2204503002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/FrameView.cpp (right):

https://codereview.chromium.org/2204503002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/FrameView.cpp:2515: void
FrameView::notifyResizeObserver()
Hmm, you're calling this in updateLifecyclePhasesInternal which is suppsoed to
handle the entire frame tree. I don't think this code is right, you don't handle
ResizeObservers in iframes.

https://codereview.chromium.org/2204503002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/FrameView.cpp:2530: if (lifecycle().state()
< DocumentLifecycle::LayoutClean || layoutView()->needsLayout())
don't check the lifecycle or dirty bits manually

https://codereview.chromium.org/2204503002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/FrameView.cpp:2531:
updateStyleAndLayoutIfNeededRecursive();
You don't want to do this, it walks the entire frame tree, you want to just do
document().updateLayout()

Also don't check the lifecycle phase, you should never need to do that, just
call the function and let the dirty bits handle early return.

https://codereview.chromium.org/2204503002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutObject.cpp (right):

https://codereview.chromium.org/2204503002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutObject.cpp:3441:
element.sizeMayHaveChanged();
this doesn't seem like the right place to hook for this. Ojan probably knows
better.

[atotic1, 2016-08-04 23:59:40]

Got the eliot fixes in:

Size detection switched from LayoutObject::setMayNeedPaintInvalidation to
LayoutBox::frameRectChanged.
This detected changes for block elements, but not SVG. I've added notifications
to appropriate SVG elements too. I've added a new svg test suite for this.

notifyResizeObserver now gets called forAllNonThrottledFrameViews. Added a
test case (that was the tricky part).

Please review.

https://codereview.chromium.org/2204503002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/Element.cpp (right):

https://codereview.chromium.org/2204503002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Element.cpp:2671:
HeapHashMap<Member<ResizeObserver>, Member<ResizeObservation>>* roData =
resizeObserverData();
On 2016/08/03 18:06:51, esprehn wrote:
> don't abbreviate, I'd either call this |data| or |observerData|

Done.

https://codereview.chromium.org/2204503002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Element.cpp:2672: if (roData) {
On 2016/08/03 18:06:51, esprehn wrote:
> I'd early return:
> 
> if (!data)
>   return;
> 
> You can en use auto here to make this function very compact.
> 
> if (auto* data = resizeObserverData()) {
>   for (auto& observation : data->values())
>     observation->elementSizeChanged()
> }

Done.

https://codereview.chromium.org/2204503002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Element.cpp:2673: for (auto& iter : *roData)
On 2016/08/03 18:06:51, esprehn wrote:
> ->values() and make the loop variable |observation|

Done. Thanks for providing the solution. Still feeling my way around different
C++ collections and auto.

https://codereview.chromium.org/2204503002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/FrameView.cpp (right):

https://codereview.chromium.org/2204503002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/FrameView.cpp:2515: void
FrameView::notifyResizeObserver()
On 2016/08/03 18:06:51, esprehn wrote:
> Hmm, you're calling this in updateLifecyclePhasesInternal which is suppsoed to
> handle the entire frame tree. I don't think this code is right, you don't
handle
> ResizeObservers in iframes.

Done.

I added forAllNonThrottledFrameViews loop to notify all frames, and added a test
case.

https://codereview.chromium.org/2204503002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/FrameView.cpp:2530: if (lifecycle().state()
< DocumentLifecycle::LayoutClean || layoutView()->needsLayout())
On 2016/08/03 18:06:51, esprehn wrote:
> don't check the lifecycle or dirty bits manually

Done.

https://codereview.chromium.org/2204503002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/FrameView.cpp:2531:
updateStyleAndLayoutIfNeededRecursive();
On 2016/08/03 18:06:51, esprehn wrote:
> You don't want to do this, it walks the entire frame tree, you want to just do
> document().updateLayout()
> 
> Also don't check the lifecycle phase, you should never need to do that, just
> call the function and let the dirty bits handle early return.

My fix: frame().document()->updateStyleAndLayout();
This was a guess,  there is also updateStyleAndLayoutTree(), and I am not sure
what the difference is.

https://codereview.chromium.org/2204503002/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutObject.cpp (right):

https://codereview.chromium.org/2204503002/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutObject.cpp:3441:
element.sizeMayHaveChanged();
On 2016/08/03 18:06:51, esprehn wrote:
> this doesn't seem like the right place to hook for this. Ojan probably knows
> better.

Removed. Size changes are now monitored in LayoutBox::frameRectChanged, and many
SVGLayout elements. Added an svg test suite too.

Done.

[atotic1, 2016-08-05 19:51:28]

Just uploaded a new patch: ian asked me to add tests for moving dom inside event
loop.

[esprehn, 2016-08-10 04:27:10]

I think I'd rename sizeMayHaveChanged to setNeedsResizeObserverUpdate() or
something more explicit for the purpose. It's so generic right now I worry about
it getting used for other things.

lgtm assuming you have the right test coverage, please do consider the better
name in a follow up patch.

https://codereview.chromium.org/2204503002/diff/80001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/resize-observer/eventloop.html (right):

https://codereview.chromium.org/2204503002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/resize-observer/eventloop.html:2: <head>
I'd leave out the <head> and <body> in all of your tests since you don't need it

https://codereview.chromium.org/2204503002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/FrameView.cpp (right):

https://codereview.chromium.org/2204503002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/FrameView.cpp:2520: void
FrameView::notifyResizeObserver()
Observers() I think you want plural?

https://codereview.chromium.org/2204503002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/FrameView.cpp:2539:
resizeController.clearObservations();
Hmm, if we clear the observations how does scheduling a frame below go about
calling all of the observers again?

https://codereview.chromium.org/2204503002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/FrameView.cpp:2582:
frameView.notifyResizeObserver();
Did you add some iframe tests where the new ResizeObserver call is inside the
iframe?

https://codereview.chromium.org/2204503002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/svg/LayoutSVGContainer.cpp (right):

https://codereview.chromium.org/2204503002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/svg/LayoutSVGContainer.cpp:170:
element()->sizeMayHaveChanged();
I assume you have tests for all of these callers?

[atotic1, 2016-08-10 05:01:59]

On 2016/08/10 04:27:10, esprehn wrote:
> I think I'd rename sizeMayHaveChanged to setNeedsResizeObserverUpdate() or
> something more explicit for the purpose. It's so generic right now I worry
about
> it getting used for other things.

My intention is for it to be a generic routine, that can be used by others if
they need it. Why do you think it should be restricted to RO?

[esprehn, 2016-08-10 05:10:24]

On 2016/08/10 at 05:01:59, atotic wrote:
> On 2016/08/10 04:27:10, esprehn wrote:
> > I think I'd rename sizeMayHaveChanged to setNeedsResizeObserverUpdate() or
> > something more explicit for the purpose. It's so generic right now I worry
about
> > it getting used for other things.
> 
> My intention is for it to be a generic routine, that can be used by others if
> they need it. Why do you think it should be restricted to RO?

There's no reason for this generic routine in the DOM, we don't need others to
use it, they should use ResizeObserver instead. Every back door we use
internally is bugs and debt for us to deal with later. There's no reason for
this kind of generic callback today. :)

[atotic1, 2016-08-10 18:24:53]

On 2016/08/10 05:10:24, esprehn wrote:
> On 2016/08/10 at 05:01:59, atotic wrote:
> > On 2016/08/10 04:27:10, esprehn wrote:
> > > I think I'd rename sizeMayHaveChanged to setNeedsResizeObserverUpdate() or
> > > something more explicit for the purpose. It's so generic right now I worry
> about
> > > it getting used for other things.
> > 
> > My intention is for it to be a generic routine, that can be used by others
if
> > they need it. Why do you think it should be restricted to RO?
> 
> There's no reason for this generic routine in the DOM, we don't need others to
> use it, they should use ResizeObserver instead. Every back door we use
> internally is bugs and debt for us to deal with later. There's no reason for
> this kind of generic callback today. :)

I thought about this for a while, and I think I understand where you are coming
from.
If anyone else used sizeMayHaveChanged, they would not take advantage of 
event-loop throttling, and that's bad.

Done.

[atotic1, 2016-08-10 18:25:33]

All comments have been addressed.

https://codereview.chromium.org/2204503002/diff/80001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/resize-observer/eventloop.html (right):

https://codereview.chromium.org/2204503002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/resize-observer/eventloop.html:2: <head>
On 2016/08/10 04:27:09, esprehn wrote:
> I'd leave out the <head> and <body> in all of your tests since you don't need
it

Done.

https://codereview.chromium.org/2204503002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/frame/FrameView.cpp (right):

https://codereview.chromium.org/2204503002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/FrameView.cpp:2520: void
FrameView::notifyResizeObserver()
On 2016/08/10 04:27:09, esprehn wrote:
> Observers() I think you want plural?

Done.

I usually keep everything singular for simplicity.

https://codereview.chromium.org/2204503002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/FrameView.cpp:2539:
resizeController.clearObservations();
On 2016/08/10 04:27:09, esprehn wrote:
> Hmm, if we clear the observations how does scheduling a frame below go about
> calling all of the observers again?

This is non-obvious.

ResizeObserver has a set of active observations.
gather/deliver/clear/skipped API operates on this set.

ResizeObserverController/ResizeObserver/ResizeObservation have "changed" flags. 

Active observations are regenerated for every event loop from a set of 
"changed" flags.

"changed" flags are only cleared when observations on deliver(), not on clear().

https://codereview.chromium.org/2204503002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/frame/FrameView.cpp:2582:
frameView.notifyResizeObserver();
On 2016/08/10 04:27:09, esprehn wrote:
> Did you add some iframe tests where the new ResizeObserver call is inside the
> iframe?

observer.html, test6()

https://codereview.chromium.org/2204503002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/svg/LayoutSVGContainer.cpp (right):

https://codereview.chromium.org/2204503002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/svg/LayoutSVGContainer.cpp:170:
element()->sizeMayHaveChanged();
On 2016/08/10 04:27:09, esprehn wrote:
> I assume you have tests for all of these callers?

Yes, a whole new test file: svg.html

[atotic1, 2016-08-10 18:25:50]

The CQ bit was checked by atotic@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-10 18:26:51]

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, 2016-08-10 20:33:14]

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

[commit-bot: I haz the power, 2016-08-10 20:33:15]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[atotic1, 2016-08-11 23:37:43]

The CQ bit was checked by atotic@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-11 23:39:24]

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, 2016-08-11 23:43:58]

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

[commit-bot: I haz the power, 2016-08-11 23:44:01]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[atotic1, 2016-08-11 23:49:03]

eventloop:test0 failed on the Mac. I fixed it.
Do I need a new LGTM, or should I check this in? Doing the dryrun right now.

The fix is:

test0 checks what happens when notifications are skipped because of depth.
What is supposed to happen was:
notify depth 0
notify depth 1 + skip
rAF
notify depth 0

What happened was: 
notify depth 0
notify depth 1 + skip
notify depth 0
rAF

The cause of failure was that notifyResizeObservers ran twice without rAF in the
middle.
The fix is a guard that guarantees notifyResizeObservers runs only when we
intend to paint (targetState == DocumentLifecycle::PaintClean)

[atotic1, 2016-08-12 00:17:49]

The CQ bit was checked by atotic@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-12 00:18:49]

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, 2016-08-12 01:42:55]

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

[commit-bot: I haz the power, 2016-08-12 01:42:56]

Dry run: This issue passed the CQ dry run.

[atotic1, 2016-08-12 17:37:28]

The CQ bit was checked by atotic@google.com

[atotic1, 2016-08-12 17:37:28]

The patchset sent to the CQ was uploaded after l-g-t-m from eae@chromium.org,
esprehn@chromium.org
Link to the patchset: https://codereview.chromium.org/2204503002/#ps140001
(title: "Fresh rebase")

[commit-bot: I haz the power, 2016-08-12 17:38:00]

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, 2016-08-12 17:43:11]

Description was changed from

==========
ResizeObserver pt6: integration

Integrates ResizeObserver with the browser. 

1) Element notifies ResizeObserver when its size might have changed.

Operations that might change Element's observed size are trapped:
Element::detachLayoutTree traps Element removal from DOM tree
LayoutObject::setMayNeedPaintInvalidation traps size changes

2) Event loop integration

FrameView::notifyResizeObserver implements it per spec.
https://wicg.github.io/ResizeObserver/#html-event-loop

3) Tests

LayoutTests/resize-observer is a comprehensive test suite.

BUG=612962
==========

to

==========
ResizeObserver pt6: integration

Integrates ResizeObserver with the browser. 

1) Element notifies ResizeObserver when its size might have changed.

Operations that might change Element's observed size are trapped:
Element::detachLayoutTree traps Element removal from DOM tree
LayoutObject::setMayNeedPaintInvalidation traps size changes

2) Event loop integration

FrameView::notifyResizeObserver implements it per spec.
https://wicg.github.io/ResizeObserver/#html-event-loop

3) Tests

LayoutTests/resize-observer is a comprehensive test suite.

BUG=612962
==========

[commit-bot: I haz the power, 2016-08-12 17:43:13]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-08-12 17:45:11]

Description was changed from

==========
ResizeObserver pt6: integration

Integrates ResizeObserver with the browser. 

1) Element notifies ResizeObserver when its size might have changed.

Operations that might change Element's observed size are trapped:
Element::detachLayoutTree traps Element removal from DOM tree
LayoutObject::setMayNeedPaintInvalidation traps size changes

2) Event loop integration

FrameView::notifyResizeObserver implements it per spec.
https://wicg.github.io/ResizeObserver/#html-event-loop

3) Tests

LayoutTests/resize-observer is a comprehensive test suite.

BUG=612962
==========

to

==========
ResizeObserver pt6: integration

Integrates ResizeObserver with the browser.

1) Element notifies ResizeObserver when its size might have changed.

Operations that might change Element's observed size are trapped:
Element::detachLayoutTree traps Element removal from DOM tree
LayoutObject::setMayNeedPaintInvalidation traps size changes

2) Event loop integration

FrameView::notifyResizeObserver implements it per spec.
https://wicg.github.io/ResizeObserver/#html-event-loop

3) Tests

LayoutTests/resize-observer is a comprehensive test suite.

BUG=612962

Committed: https://crrev.com/5b49641e336ba9be7789cc0eb2511d415f4e6d5d
Cr-Commit-Position: refs/heads/master@{#411694}
==========

[commit-bot: I haz the power, 2016-08-12 17:45:14]

Patchset 8 (id:??) landed as
https://crrev.com/5b49641e336ba9be7789cc0eb2511d415f4e6d5d
Cr-Commit-Position: refs/heads/master@{#411694}

[xidachen, 2016-08-12 19:05:00]

A revert of this CL (patchset #8 id:140001) has been created in
https://codereview.chromium.org/2242773003/ by xidachen@chromium.org.

The reason for reverting is: Causing layout test failure:
https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Leak/b....

[atotic1, 2016-08-15 06:13:16]

Got a fix for this. Only changes were in the testing code, the problem was how I
was using the async tests. I was reusing the same observer for multiple
unrelated tests. Now unrelated tests use separate observers.

It looks like this CR can no longer be checked in. Do I need to create a new
one? You can take a look at code deltas here.

[atotic1, 2016-08-15 16:08:03]

The CQ bit was checked by atotic@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-15 16:08:19]

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

[atotic1, 2016-08-15 16:09:39]

On 2016/08/15 at 06:13:16, atotic1 wrote:
> It looks like this CR can no longer be checked in. Do I need to create a new
one? You can take a look at code deltas here.

Got some help, and reopened the issue. Doing the dry run, will commit if I can
once it completes.

[commit-bot: I haz the power, 2016-08-15 16:10:41]

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

[commit-bot: I haz the power, 2016-08-15 16:10:42]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[atotic1, 2016-08-15 16:18:43]

The CQ bit was checked by atotic@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-15 16:19:00]

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, 2016-08-15 17:48:31]

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

[commit-bot: I haz the power, 2016-08-15 17:48:32]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[atotic1, 2016-08-16 00:56:09]

The CQ bit was checked by atotic@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-16 00:56:39]

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

[atotic1, 2016-08-16 02:39:56]

The CQ bit was unchecked by atotic@google.com

[atotic1, 2016-08-16 02:45:15]

The CQ bit was checked by atotic@google.com

[atotic1, 2016-08-16 02:45:15]

The patchset sent to the CQ was uploaded after l-g-t-m from
esprehn@chromium.org, eae@chromium.org
Link to the patchset: https://codereview.chromium.org/2204503002/#ps220001
(title: "Restore missing code that was blown away by rebase")

[commit-bot: I haz the power, 2016-08-16 02:45:37]

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, 2016-08-16 03:00:25]

Committed patchset #12 (id:220001)

[commit-bot: I haz the power, 2016-08-16 03:01:40]

Description was changed from

==========
ResizeObserver pt6: integration

Integrates ResizeObserver with the browser.

1) Element notifies ResizeObserver when its size might have changed.

Operations that might change Element's observed size are trapped:
Element::detachLayoutTree traps Element removal from DOM tree
LayoutObject::setMayNeedPaintInvalidation traps size changes

2) Event loop integration

FrameView::notifyResizeObserver implements it per spec.
https://wicg.github.io/ResizeObserver/#html-event-loop

3) Tests

LayoutTests/resize-observer is a comprehensive test suite.

BUG=612962

Committed: https://crrev.com/5b49641e336ba9be7789cc0eb2511d415f4e6d5d
Cr-Commit-Position: refs/heads/master@{#411694}
==========

to

==========
ResizeObserver pt6: integration

Integrates ResizeObserver with the browser.

1) Element notifies ResizeObserver when its size might have changed.

Operations that might change Element's observed size are trapped:
Element::detachLayoutTree traps Element removal from DOM tree
LayoutObject::setMayNeedPaintInvalidation traps size changes

2) Event loop integration

FrameView::notifyResizeObserver implements it per spec.
https://wicg.github.io/ResizeObserver/#html-event-loop

3) Tests

LayoutTests/resize-observer is a comprehensive test suite.

BUG=612962

Committed: https://crrev.com/5b49641e336ba9be7789cc0eb2511d415f4e6d5d
Committed: https://crrev.com/3f41316c60eb95369602fde0ebf52e935a3a91d5
Cr-Original-Commit-Position: refs/heads/master@{#411694}
Cr-Commit-Position: refs/heads/master@{#412149}
==========

[commit-bot: I haz the power, 2016-08-16 03:01:41]

Patchset 12 (id:??) landed as
https://crrev.com/3f41316c60eb95369602fde0ebf52e935a3a91d5
Cr-Commit-Position: refs/heads/master@{#412149}