Initial ResizeObserver implementation

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 84b3a0981aeb35b5957de7a87ba35604a8f99d5a..310e5e36fd1e678192995db23dedfbc97234a232 100644
--- a/third_party/WebKit/Source/core/frame/FrameView.cpp
+++ b/third_party/WebKit/Source/core/frame/FrameView.cpp
@@ -26,6 +26,7 @@
 
 #include "core/frame/FrameView.h"
 
+#include "bindings/core/v8/ScriptCallStack.h"

[szager1, 2016/05/23 19:49:10]

I don't think you use this.

[atotic1, 2016/05/25 00:15:52]

I am using it, through magic of templates. Won't compile without it.

 #include "core/HTMLNames.h"
 #include "core/MediaTypeNames.h"
 #include "core/css/FontFaceSet.h"
@@ -37,6 +38,7 @@
 #include "core/editing/FrameSelection.h"
 #include "core/editing/RenderedPosition.h"
 #include "core/editing/markers/DocumentMarkerController.h"
+#include "core/events/ErrorEvent.h"
 #include "core/fetch/ResourceFetcher.h"
 #include "core/frame/EventHandlerRegistry.h"
 #include "core/frame/FrameHost.h"
@@ -75,6 +77,7 @@
 #include "core/layout/svg/LayoutSVGRoot.h"
 #include "core/loader/FrameLoader.h"
 #include "core/loader/FrameLoaderClient.h"
+#include "core/observer/ResizeObserverController.h"
 #include "core/page/AutoscrollController.h"
 #include "core/page/ChromeClient.h"
 #include "core/page/FocusController.h"
@@ -2411,6 +2414,57 @@ void FrameView::scheduleVisualUpdateForPaintInvalidationIfNeeded()
     // Otherwise the paint invalidation will be handled in paint invalidation phase of this cycle.
 }
 
+void FrameView::notifyResizeObserver()
+{
+    if (!m_frame->document()->hasResizeObserverController())
+        return;
+
+    ResizeObserverController& resizeController = m_frame->document()->ensureResizeObserverController();
+
+    // Messy looping logic:
+    // Loop until :
+    // - no notifications were delivered, or
+    // - more than kRenderLoopLimit resizeObservations were processed
+    // DocumentLifecycle should be LayoutClean before and after processing all notifications.
+
+    size_t resizeNotifyCount = 0;
+    bool done = false;
+
+    while (!done) {

[szager1, 2016/05/23 19:49:10]

I think this loop might read better as:

do {
  resizeController.deliverObservations();
  if (state >= LayoutClean && !needsLayout())
    break;

  updateStyleAndLayoutIfNeededRecursive();

  if (++resizeNotifyCount > limit) {
    generate error;
    break;
  }

  resizeObserver.gatherObservations();
} while (resizeController.hasObservations());

ASSERT(!layoutView()->needsLayout());



Advantages:

- It clarifies that the exit condition for the loop is when there are no more
observations.
- The document is in a consistent state when the loop exits (hence the ASSERT).

[atotic1, 2016/05/25 00:15:52]

I reworked the loop into something more understandeable.

+
+        // This does not work because lifecycle does not roll back.
+        // if (lifecycle().state() < DocumentLifecycle::LayoutClean)
+
+        // This causes the bug too.
+        // if (m_frame->document()->needsLayoutTreeUpdate())
+        //    updateStyleAndLayoutIfNeededRecursive();
+
+        // FIXME(atotic) -- unsure if this is the right way to
+        // loop over layout pipeline.
+        if (lifecycle().state() < DocumentLifecycle::LayoutClean
+            || layoutView()->needsLayout())
+            updateStyleAndLayoutIfNeededRecursive();
+
+        resizeController.gatherObservations();
+        if (!resizeController.hasObservations()) {
+            // No resize entries delivered, we are done.
+            done = true;
+        } else {
+            resizeController.deliverObservations();
+            resizeNotifyCount++;
+            if (resizeNotifyCount == ResizeObserverController::kRenderLoopLimit) {

[szager1, 2016/05/23 19:49:10]

If you're going to generate an error, you should do it *before* gather/deliver,
and early-out.

[atotic1, 2016/05/25 00:15:52]

Acknowledged.

+                // report the error
+                ErrorEvent * error = ErrorEvent::create("ResizeObserver loop limit exceeded.", "", 0, 0, nullptr);
+                m_frame->document()->reportException(error, 0, nullptr, NotSharableCrossOrigin);
+                done = true;
+            }
+        }
+    }
+
+    if (layoutView()->needsLayout())
+        updateStyleAndLayoutIfNeededRecursive();
+}
+
 // TODO(leviw): We don't assert lifecycle information from documents in child PluginViews.
 void FrameView::updateLifecyclePhasesInternal(LifeCycleUpdateOption phases)
 {
@@ -2428,6 +2482,9 @@ void FrameView::updateLifecyclePhasesInternal(LifeCycleUpdateOption phases)
     }
 
     updateStyleAndLayoutIfNeededRecursive();
+
+    notifyResizeObserver();

[szager1, 2016/05/23 19:49:10]

Maybe rename this to runUpdateNotifyLoop(), and remove the
updateStyleAndLayoutIfNeededRecursive() call just before this.

[atotic1, 2016/05/25 00:15:52]

I'd prefer not to rename it until it does something more than resize.
updateNotify does not convey much information.
If resizeObserver is undefined, it will not run the
updateStyleAndLayoutIfNeededRecursive()
Leaving it in for now.

This whole code segment needs a review by someone who understands the event
loop.

I do not understand the event cycle/layout enough to understand interaction
between
lifecycle().state(),  needsLayout(), and updateStyleAndLayoutIfNeededRecursive()

+
     ASSERT(lifecycle().state() >= DocumentLifecycle::LayoutClean);
 
     if (phases == OnlyUpToLayoutClean) {