Get rid of obsolete FrameView::m_shouldUpdateViewportIntersection.

Get rid of obsolete FrameView::m_shouldUpdateViewportIntersection.

This was originally intended as an optimization, but now that all
intersection tracking happens through IntersectionObserver, and all
active observers must run on each frame, it makes no sense to
short-circuit the recursion in updateViewportIntersectionsForSubtree.

This also allows us to remove IntersectionObserver::trackingDocument,
which was a wart introduced to fix a bug where observers defined in
subframes were not generating IntersectionObserver notifications, as
discussed here:

https://codereview.chromium.org/2553343004

BUG=677620
R=skyostil@chromium.org

Review-Url: https://codereview.chromium.org/2610963003
Cr-Commit-Position: refs/heads/master@{#441811}
Committed: https://chromium.googlesource.com/chromium/src/+/33233d198c4f17bc6016bb2f9f59e64232c7f541

[szager1, 2017-01-04 22:39:24]

Description was changed from

==========
Get rid of obsolete FrameView::m_shouldUpdateViewportIntersection.

This was originally intended as an optimization, but now that all
intersection tracking happens through IntersectionObserver, and all
active observers must run on each frame, it makes no sense to
short-circuit the recursion in updateViewportIntersectionsForSubtree.

This also allows us to remove IntersectionObserver::trackingDocument,
which was a wart introduced to fix a bug where observers defined in
subframes were not generating IntersectionObserver notifications, as
discussed here:

https://codereview.chromium.org/2553343004

BUG=677620
R=skyostil@chromium.org
==========

to

==========
Get rid of obsolete FrameView::m_shouldUpdateViewportIntersection.

This was originally intended as an optimization, but now that all
intersection tracking happens through IntersectionObserver, and all
active observers must run on each frame, it makes no sense to
short-circuit the recursion in updateViewportIntersectionsForSubtree.

This also allows us to remove IntersectionObserver::trackingDocument,
which was a wart introduced to fix a bug where observers defined in
subframes were not generating IntersectionObserver notifications, as
discussed here:

https://codereview.chromium.org/2553343004

BUG=677620
R=skyostil@chromium.org
==========

[szager1, 2017-01-04 22:39:24]

szager@chromium.org changed reviewers:
+ mlamouri@chromium.org

[szager1, 2017-01-04 22:39:48]

mlamouri@, you're familiar with IntersectionObserver.  Can you review this since
skyostil is OOO?

[mlamouri (slow - plz ping), 2017-01-05 12:16:09]

lgtm -- mostly removing code, that was easy :)

[szager1, 2017-01-05 17:15:25]

The CQ bit was checked by szager@chromium.org

[commit-bot: I haz the power, 2017-01-05 17:15:51]

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, 2017-01-05 17:22:59]

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

[commit-bot: I haz the power, 2017-01-05 17:23:00]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[szager1, 2017-01-05 19:26:38]

szager@chromium.org changed reviewers:
+ bokan@chromium.org

[szager1, 2017-01-05 19:26:51]

bokan@ for OWNERS

[bokan, 2017-01-05 19:32:04]

rs lgtm

[szager1, 2017-01-05 20:26:31]

The CQ bit was checked by szager@chromium.org

[commit-bot: I haz the power, 2017-01-05 20:27:04]

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, 2017-01-05 20:33:34]

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

[commit-bot: I haz the power, 2017-01-05 20:33:37]

Failed to apply patch for third_party/WebKit/Source/core/frame/FrameView.cpp:
While running git apply --index -p1;
  error: patch failed: third_party/WebKit/Source/core/frame/FrameView.cpp:4544
  error: third_party/WebKit/Source/core/frame/FrameView.cpp: patch does not
apply

Patch:       third_party/WebKit/Source/core/frame/FrameView.cpp
Index: third_party/WebKit/Source/core/frame/FrameView.cpp
diff --git a/third_party/WebKit/Source/core/frame/FrameView.cpp
b/third_party/WebKit/Source/core/frame/FrameView.cpp
index
7befa59cab9596c4259b88f2b9181b14a778f3c7..a396431cd8a82a007aa4cc2f77279423aeee0d0d
100644
--- a/third_party/WebKit/Source/core/frame/FrameView.cpp
+++ b/third_party/WebKit/Source/core/frame/FrameView.cpp
@@ -175,7 +175,6 @@ FrameView::FrameView(LocalFrame& frame)
       m_didScrollTimer(this, &FrameView::didScrollTimerFired),
       m_browserControlsViewportAdjustment(0),
       m_needsUpdateWidgetGeometries(false),
-      m_needsUpdateViewportIntersection(true),
 #if ENABLE(ASSERT)
       m_hasBeenDisposed(false),
 #endif
@@ -3580,7 +3579,6 @@ void FrameView::setParent(Widget* parentView) {
   Widget::setParent(parentView);
 
   updateParentScrollableAreaSet();
-  setNeedsUpdateViewportIntersection();
   setupRenderThrottling();
 
   if (parentFrameView())
@@ -3631,7 +3629,6 @@ void FrameView::frameRectsChanged() {
   if (layoutSizeFixedToFrameSize())
     setLayoutSizeInternal(frameRect().size());
 
-  setNeedsUpdateViewportIntersection();
   if (RuntimeEnabledFeatures::slimmingPaintInvalidationEnabled()) {
     // The overflow clip property depends on the frame rect.
     setNeedsPaintPropertyUpdate();
@@ -4544,12 +4541,6 @@ void FrameView::collectAnnotatedRegions(
     collectAnnotatedRegions(*curr, regions);
 }
 
-void FrameView::setNeedsUpdateViewportIntersection() {
-  for (FrameView* parent = parentFrameView(); parent;
-       parent = parent->parentFrameView())
-    parent->m_needsUpdateViewportIntersectionInSubtree = true;
-}
-
 void FrameView::updateViewportIntersectionsForSubtree(
     DocumentLifecycle::LifecycleState targetState) {
   // Notify javascript IntersectionObservers
@@ -4560,10 +4551,6 @@ void FrameView::updateViewportIntersectionsForSubtree(
         ->intersectionObserverController()
         ->computeTrackedIntersectionObservations();
 
-  if (!m_needsUpdateViewportIntersectionInSubtree)
-    return;
-  m_needsUpdateViewportIntersectionInSubtree = false;
-
   for (Frame* child = m_frame->tree().firstChild(); child;
        child = child->tree().nextSibling()) {
     if (!child->isLocalFrame())

[szager1, 2017-01-05 20:38:06]

The CQ bit was checked by szager@chromium.org

[szager1, 2017-01-05 20:38:07]

The patchset sent to the CQ was uploaded after l-g-t-m from bokan@chromium.org,
mlamouri@chromium.org
Link to the patchset: https://codereview.chromium.org/2610963003/#ps40001
(title: "rebase")

[commit-bot: I haz the power, 2017-01-05 20:38:29]

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, 2017-01-06 01:56:04]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1483648686884330,
"parent_rev": "9f12c413bd54610816c90bc31a7768113463f55b", "commit_rev":
"33233d198c4f17bc6016bb2f9f59e64232c7f541"}

[commit-bot: I haz the power, 2017-01-06 01:56:31]

Description was changed from

==========
Get rid of obsolete FrameView::m_shouldUpdateViewportIntersection.

This was originally intended as an optimization, but now that all
intersection tracking happens through IntersectionObserver, and all
active observers must run on each frame, it makes no sense to
short-circuit the recursion in updateViewportIntersectionsForSubtree.

This also allows us to remove IntersectionObserver::trackingDocument,
which was a wart introduced to fix a bug where observers defined in
subframes were not generating IntersectionObserver notifications, as
discussed here:

https://codereview.chromium.org/2553343004

BUG=677620
R=skyostil@chromium.org
==========

to

==========
Get rid of obsolete FrameView::m_shouldUpdateViewportIntersection.

This was originally intended as an optimization, but now that all
intersection tracking happens through IntersectionObserver, and all
active observers must run on each frame, it makes no sense to
short-circuit the recursion in updateViewportIntersectionsForSubtree.

This also allows us to remove IntersectionObserver::trackingDocument,
which was a wart introduced to fix a bug where observers defined in
subframes were not generating IntersectionObserver notifications, as
discussed here:

https://codereview.chromium.org/2553343004

BUG=677620
R=skyostil@chromium.org

Review-Url: https://codereview.chromium.org/2610963003
Cr-Commit-Position: refs/heads/master@{#441811}
Committed:
https://chromium.googlesource.com/chromium/src/+/33233d198c4f17bc6016bb2f9f59...
==========

[commit-bot: I haz the power, 2017-01-06 01:56:32]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/33233d198c4f17bc6016bb2f9f59...

[Sami, 2017-01-10 15:53:25]

lgtm, thanks for the cleanup!

Part of me wonders about the CPU overhead of these calculations but they'll
eventually show up on profiles if they are an issue.