Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2188983003:
ResizeObserver pt5: size change notification (Closed)
DescriptionResizeObserver pt5: size change notification
This change implements size change notification algorithm.
Design doc at https://docs.google.com/document/d/1G4OmqqlFY3H3erQAUmteKES02-bIECikDjWBrDKH9PA/edit?usp=sharing
Size change notification
We collect and deliver notifications about all Elements
whose size has changed inside the event loop.
The algorithm loops until all notifications are delivered,
or it hits the depth limit.
c = ResizeObserverController();
for (limit = c.gatherObservations(0);
limit != ResizeObserverController::kDepthLimit;
limit = c.gatherObservations(limit)) {
c.deliverObservations();
}
if (c.skippedObservations()) {
c.clearObservations();
sendError()
}
For efficiency, we avoid continuosly polling all elements
for size changes. Instead, an Element notifies ResizeObservation
when its size might have changed. ResizeObservation propagates
this change to ResizeObserver, which propagates to
ResizeObserverController. All of them set a flag indicating that
they might have been changed.
The flag is cleared when observations are delivered.
BUG=612962
Committed: https://crrev.com/3d5b99673799a3067e9a5b8621f2b9ef2ff5d5d5
Cr-Commit-Position: refs/heads/master@{#408867}
Patch Set 1 #
Total comments: 2
Patch Set 2 : Review fixes #
Total comments: 18
Patch Set 3 : All CR comments addressed #
Messages
Total messages: 31 (12 generated)
Description was changed from ========== ResizeObserver pt5: size change notification This change implements size change notification algorithm. Design doc at https://docs.google.com/document/d/1G4OmqqlFY3H3erQAUmteKES02-bIECikDjWBrDKH9... Size change notification We collect and deliver notifications about all Elements whose size has changed inside the event loop. The algorithm loops until all notifications are delivered, or it hits the depth limit. c = ResizeObserverController(); for (limit = c.gatherObservations(0); limit != ResizeObserverController::kDepthLimit; limit = c.gatherObservations(limit)) { c.deliverObservations(); } if (c.skippedObservations()) { c.clearObservations(); sendError() } For efficiency, we avoid continuosly polling all elements for size changes. Instead, an Element notifies ResizeObservation when its size might have changed. ResizeObservation propagates this change to ResizeObserver, which propagates to ResizeObserverController. All of them set a flag indicating that they might have been changed. The flag is cleared when observations are delivered. BUG=612962 ========== to ========== ResizeObserver pt5: size change notification This change implements size change notification algorithm. Design doc at https://docs.google.com/document/d/1G4OmqqlFY3H3erQAUmteKES02-bIECikDjWBrDKH9... Size change notification We collect and deliver notifications about all Elements whose size has changed inside the event loop. The algorithm loops until all notifications are delivered, or it hits the depth limit. c = ResizeObserverController(); for (limit = c.gatherObservations(0); limit != ResizeObserverController::kDepthLimit; limit = c.gatherObservations(limit)) { c.deliverObservations(); } if (c.skippedObservations()) { c.clearObservations(); sendError() } For efficiency, we avoid continuosly polling all elements for size changes. Instead, an Element notifies ResizeObservation when its size might have changed. ResizeObservation propagates this change to ResizeObserver, which propagates to ResizeObserverController. All of them set a flag indicating that they might have been changed. The flag is cleared when observations are delivered. BUG=612962 ==========
atotic@google.com changed reviewers: + eae@chromium.org, ikilpatrick@chromium.org
Hi Emil and Ian, this is part 5 of ResizeObserver. It completes the ResizeObserver API, future CRs will deal with integration into existing code. Please review, Aleks PS: thanks for the last speedy review
Would you mind explaining the expectations around iterating over gatherObservations, especially around how the min-depth it returns is used? I don't quite understand how the example in the CL description would work. If the goal is to return the observations in depth order (shallowest first) wouldn't it be easier to sort the list once rather than iterating over the entire list each invocation? Perhaps I'm missing something? https://codereview.chromium.org/2188983003/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/observer/ResizeObservation.h (right): https://codereview.chromium.org/2188983003/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/observer/ResizeObservation.h:38: // True if Element reported size change. Please remove unnecessary comment, the name is clear enough :)
On 2016/07/28 18:18:32, eae wrote: > Would you mind explaining the expectations around iterating over > gatherObservations, especially around how the min-depth it returns is used? > > I don't quite understand how the example in the CL description would work. If > the goal is to return the observations in depth order (shallowest first) > wouldn't it be easier to sort the list once rather than iterating over the > entire list each invocation? Perhaps I'm missing something? Algorithm implemented is described as "Depth limit" in: https://github.com/WICG/ResizeObserver/issues/7 The description is not strictly correct, because it does not take the possibilty of moving nodes while iterating into account. Here is a little more friendly explanation: The initial goal of the algorithm was to report all the observations: while (gatherObservations()) deliverObservations(); This was a possible infinite loop if deliverObservations() generated new observations. Depth limit is a way out of the loop. Instead of delivering all observations in every loop, only notifications deeper than the shallowest notification from the previous iteration are delivered.
On 2016/07/28 19:09:59, atotic1 wrote: > On 2016/07/28 18:18:32, eae wrote: > > Would you mind explaining the expectations around iterating over > > gatherObservations, especially around how the min-depth it returns is used? > > > > I don't quite understand how the example in the CL description would work. If > > the goal is to return the observations in depth order (shallowest first) > > wouldn't it be easier to sort the list once rather than iterating over the > > entire list each invocation? Perhaps I'm missing something? > > Algorithm implemented is described as "Depth limit" in: > https://github.com/WICG/ResizeObserver/issues/7 > The description is not strictly correct, because it does not take the possibilty > of moving nodes while iterating into account. > > Here is a little more friendly explanation: > > The initial goal of the algorithm was to report all the observations: > while (gatherObservations()) > deliverObservations(); > > This was a possible infinite loop if deliverObservations() generated new > observations. > > Depth limit is a way out of the loop. Instead of delivering all observations in > every loop, > only notifications deeper than the shallowest notification from the previous > iteration are delivered. Oh, right. Now I recall that from the previous discussion. Thank you! Would you mind adding a brief comment to the code about this? LGTM
> Oh, right. Now I recall that from the previous discussion. Thank you!
> Would you mind adding a brief comment to the code about this?
>
> LGTM
Would this be enough:
// ResizeObserverController keeps track of all ResizeObservers
// in a single Document.
//
// The observation API is used to integrate ResizeObserver
// and the event loop. It delivers notification in a loop.
// In each iteration, only notifications deeper than the
// shallowest notification from previous iteration are delivered.
Or should the code sample be included too:
// It should be used like this:
// c = ResizeObserverController();
// for (limit = c.gatherObservations(0);
// limit != ResizeObserverController::kDepthLimit;
// limit = c.gatherObservations(limit)) {
// c.deliverObservations();
// }
// if (c.skippedObservations()) {
// c.clearObservations();
// sendError()
// }
On 2016/07/28 19:42:17, atotic1 wrote:
> > Oh, right. Now I recall that from the previous discussion. Thank you!
> > Would you mind adding a brief comment to the code about this?
> >
> > LGTM
>
> Would this be enough:
>
> // ResizeObserverController keeps track of all ResizeObservers
> // in a single Document.
> //
> // The observation API is used to integrate ResizeObserver
> // and the event loop. It delivers notification in a loop.
> // In each iteration, only notifications deeper than the
> // shallowest notification from previous iteration are delivered.
>
> Or should the code sample be included too:
>
> // It should be used like this:
> // c = ResizeObserverController();
> // for (limit = c.gatherObservations(0);
> // limit != ResizeObserverController::kDepthLimit;
> // limit = c.gatherObservations(limit)) {
> // c.deliverObservations();
> // }
> // if (c.skippedObservations()) {
> // c.clearObservations();
> // sendError()
> // }
Thought about it some more, decided not to include code sample. The API has only
one user, no need to elaborate.
https://codereview.chromium.org/2188983003/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/observer/ResizeObservation.h (right): https://codereview.chromium.org/2188983003/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/observer/ResizeObservation.h:38: // True if Element reported size change. On 2016/07/28 18:18:32, eae wrote: > Please remove unnecessary comment, the name is clear enough :) Done.
The CQ bit was checked by atotic@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
I'm catching up a bit, what is the test strategy here? I.e. can we check in some of the tests now?
On 2016/07/28 20:39:05, ikilpatrick wrote: > I'm catching up a bit, what is the test strategy here? I.e. can we check in some > of the tests now? We had some tests checked in for C++ IDL APIs. Do you think this API needs C++ tests too? The battery of JS LayoutTests is coming in the next checkin when it all gets hooked up to event loop.
Dead code is always a bit scary, next patch is hooking into the event loop right? A lot of the layout tests will be there? https://codereview.chromium.org/2188983003/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/observer/ResizeObserver.cpp (right): https://codereview.chromium.org/2188983003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/observer/ResizeObserver.cpp:69: size_t ResizeObserver::gatherObservations(size_t deeperThan) Ok, so we should store the m_observations in an ordered list here, and then invalidate when a node is being inserted into the tree. Quickly spoke with Ojan and Element::insertedInto is the right place to invalidate here. https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/dom/Eleme... @esphren might have thoughts here. https://codereview.chromium.org/2188983003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/observer/ResizeObserver.cpp:79: auto depth = observation->targetDepth(); This is currently O(n*m*o) which we'll want to fix is well. Invalidating on tree insertion can mean that we can also dirty all the target depths and recompute when this happens. https://codereview.chromium.org/2188983003/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/observer/ResizeObserver.h (right): https://codereview.chromium.org/2188983003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/observer/ResizeObserver.h:36: bool skippedObservations() { return m_skippedObservations; } skippedObservations isn't immediately clear what it does / used for, comment. https://codereview.chromium.org/2188983003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/observer/ResizeObserver.h:44: explicit ResizeObserver(ResizeObserverCallback*, Document&); explicit isn't needed?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2016/07/28 21:48:40, ikilpatrick wrote: > Dead code is always a bit scary, next patch is hooking into the event loop > right? A lot of the layout tests will be there? > > https://codereview.chromium.org/2188983003/diff/20001/third_party/WebKit/Sour... > File third_party/WebKit/Source/core/observer/ResizeObserver.cpp (right): > > https://codereview.chromium.org/2188983003/diff/20001/third_party/WebKit/Sour... > third_party/WebKit/Source/core/observer/ResizeObserver.cpp:69: size_t > ResizeObserver::gatherObservations(size_t deeperThan) > Ok, so we should store the m_observations in an ordered list here, and then > invalidate when a node is being inserted into the tree. > > Quickly spoke with Ojan and Element::insertedInto is the right place to > invalidate here. > > https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/dom/Eleme... > > @esphren might have thoughts here. > > https://codereview.chromium.org/2188983003/diff/20001/third_party/WebKit/Sour... > third_party/WebKit/Source/core/observer/ResizeObserver.cpp:79: auto depth = > observation->targetDepth(); > This is currently O(n*m*o) which we'll want to fix is well. > > Invalidating on tree insertion can mean that we can also dirty all the target > depths and recompute when this happens. > > https://codereview.chromium.org/2188983003/diff/20001/third_party/WebKit/Sour... > File third_party/WebKit/Source/core/observer/ResizeObserver.h (right): > > https://codereview.chromium.org/2188983003/diff/20001/third_party/WebKit/Sour... > third_party/WebKit/Source/core/observer/ResizeObserver.h:36: bool > skippedObservations() { return m_skippedObservations; } > skippedObservations isn't immediately clear what it does / used for, comment. > > https://codereview.chromium.org/2188983003/diff/20001/third_party/WebKit/Sour... > third_party/WebKit/Source/core/observer/ResizeObserver.h:44: explicit > ResizeObserver(ResizeObserverCallback*, Document&); > explicit isn't needed? > https://codereview.chromium.org/2188983003/diff/20001/third_party/WebKit/Sour... > File third_party/WebKit/Source/core/observer/ResizeObserver.cpp (right): > > https://codereview.chromium.org/2188983003/diff/20001/third_party/WebKit/Sour... > third_party/WebKit/Source/core/observer/ResizeObserver.cpp:69: size_t > ResizeObserver::gatherObservations(size_t deeperThan) > Ok, so we should store the m_observations in an ordered list here, and then > invalidate when a node is being inserted into the tree. > > Quickly spoke with Ojan and Element::insertedInto is the right place to > invalidate here. > > https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/dom/Eleme... > > @esphren might have thoughts here. > > https://codereview.chromium.org/2188983003/diff/20001/third_party/WebKit/Sour... > third_party/WebKit/Source/core/observer/ResizeObserver.cpp:79: auto depth = > observation->targetDepth(); > This is currently O(n*m*o) which we'll want to fix is well. To cache depth or not, that is the question. We've had some hallway discussions about this, no firm resolution. Here is what we know: Definitions: W: number of elements being watched V: number of elements whose size has changed in a single rAF Vlife: numer of elements whose size has changed in ResizeObserver lifetime L: number of loops in a resize loop D: depth of average node Current costs per rAF: - depth gets computed only if element's size has changed. Depth cost per rAF: V * L * D Depth lifetime cost: Vlife * D Cacheing depth can reduce this cost since depth is constant unless Element moves its location in dom tree. Cache invalidation is not obvious. Current proposal is to invalidate ALL cached depths when any Element moves in dom tree, with callbacks from Element::insertedInto, and Element::removedFrom The cost of this proposal is very different, it depends on how often cache is invalidated. For every cache invalidation, we pay the price of: W: clear the flag of all elements being watched if any depths have been cached Recalculation of depth: V * L * D The worst case here is cost of W being larger than V*L*D. My personal preference is to do nothing: I feel that cost computing depth when size has changed will be miniscule compared to cost of repaint, etc. If there are real-world pages that demo problems, we can target a fix that addresses the problems. Ian disagrees. He believes that this is a performance cut, and Chrome performance dies by a 1000 cuts. We'd love another opinion. @esprehn ? @eae ?
Only remaining issue is whether to cache Element's depth or not. Waiting on feedback from @esprehn or @eae https://codereview.chromium.org/2188983003/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/observer/ResizeObserver.cpp (right): https://codereview.chromium.org/2188983003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/observer/ResizeObserver.cpp:69: size_t ResizeObserver::gatherObservations(size_t deeperThan) On 2016/07/28 21:48:40, ikilpatrick wrote: > Ok, so we should store the m_observations in an ordered list here, and then > invalidate when a node is being inserted into the tree. > > Quickly spoke with Ojan and Element::insertedInto is the right place to > invalidate here. > > https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/dom/Eleme... > > @esphren might have thoughts here. TBD: wrote a lenghty reply. https://codereview.chromium.org/2188983003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/observer/ResizeObserver.cpp:79: auto depth = observation->targetDepth(); On 2016/07/28 21:48:40, ikilpatrick wrote: > This is currently O(n*m*o) which we'll want to fix is well. > > Invalidating on tree insertion can mean that we can also dirty all the target > depths and recompute when this happens. TBD: wrote a lenghty reply. https://codereview.chromium.org/2188983003/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/observer/ResizeObserver.h (right): https://codereview.chromium.org/2188983003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/observer/ResizeObserver.h:36: bool skippedObservations() { return m_skippedObservations; } On 2016/07/28 21:48:40, ikilpatrick wrote: > skippedObservations isn't immediately clear what it does / used for, comment. Will create a comment in ResizeObserverController.h, where this API is documented. Done. https://codereview.chromium.org/2188983003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/observer/ResizeObserver.h:44: explicit ResizeObserver(ResizeObserverCallback*, Document&); On 2016/07/28 21:48:40, ikilpatrick wrote: > explicit isn't needed? Done.
ok, lgtm modulo comments. esphren convinced me that walking up won't be that bad :). https://codereview.chromium.org/2188983003/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/observer/ResizeObserver.cpp (right): https://codereview.chromium.org/2188983003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/observer/ResizeObserver.cpp:61: for (auto observation : observations) { auto& ? https://codereview.chromium.org/2188983003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/observer/ResizeObserver.cpp:92: m_elementSizeChanged = m_skippedObservations; Why does this flag get set here & like this? Potentially add comment. https://codereview.chromium.org/2188983003/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/observer/ResizeObserver.h (right): https://codereview.chromium.org/2188983003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/observer/ResizeObserver.h:44: explicit ResizeObserver(ResizeObserverCallback*, Document&); On 2016/07/29 20:04:16, atotic1 wrote: > On 2016/07/28 21:48:40, ikilpatrick wrote: > > explicit isn't needed? > > Done. Not done yet? https://codereview.chromium.org/2188983003/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/observer/ResizeObserverController.h (right): https://codereview.chromium.org/2188983003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/observer/ResizeObserverController.h:32: bool skippedObservations(); comment? https://codereview.chromium.org/2188983003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/observer/ResizeObserverController.h:42: // True if observers were changed since last notification clarify comment, is this the set of observers that changed?
The CQ bit was checked by atotic@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from eae@chromium.org, ikilpatrick@chromium.org Link to the patchset: https://codereview.chromium.org/2188983003/#ps40001 (title: "All CR comments addressed")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
All comments addressed, commiting. https://codereview.chromium.org/2188983003/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/observer/ResizeObserver.cpp (right): https://codereview.chromium.org/2188983003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/observer/ResizeObserver.cpp:61: for (auto observation : observations) { On 2016/07/29 23:18:38, ikilpatrick wrote: > auto& ? Done. https://codereview.chromium.org/2188983003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/observer/ResizeObserver.cpp:92: m_elementSizeChanged = m_skippedObservations; On 2016/07/29 23:18:38, ikilpatrick wrote: > Why does this flag get set here & like this? Potentially add comment. Added: // We can only clear this flag after all observations have been // broadcast. Done https://codereview.chromium.org/2188983003/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/observer/ResizeObserver.h (right): https://codereview.chromium.org/2188983003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/observer/ResizeObserver.h:44: explicit ResizeObserver(ResizeObserverCallback*, Document&); On 2016/07/29 23:18:38, ikilpatrick wrote: > On 2016/07/29 20:04:16, atotic1 wrote: > > On 2016/07/28 21:48:40, ikilpatrick wrote: > > > explicit isn't needed? > > > > Done. > > Not done yet? Done. Did not check in changes while waiting on targetDepth discussion. https://codereview.chromium.org/2188983003/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/observer/ResizeObserverController.h (right): https://codereview.chromium.org/2188983003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/observer/ResizeObserverController.h:32: bool skippedObservations(); On 2016/07/29 23:18:39, ikilpatrick wrote: > comment? Done. I just did not upload changes before waiting on reply for targetDepth. https://codereview.chromium.org/2188983003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/observer/ResizeObserverController.h:42: // True if observers were changed since last notification On 2016/07/29 23:18:39, ikilpatrick wrote: > clarify comment, is this the set of observers that changed? Changed to: // True if ANY observers were changed since last notification Done.
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
The CQ bit was checked by atotic@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== ResizeObserver pt5: size change notification This change implements size change notification algorithm. Design doc at https://docs.google.com/document/d/1G4OmqqlFY3H3erQAUmteKES02-bIECikDjWBrDKH9... Size change notification We collect and deliver notifications about all Elements whose size has changed inside the event loop. The algorithm loops until all notifications are delivered, or it hits the depth limit. c = ResizeObserverController(); for (limit = c.gatherObservations(0); limit != ResizeObserverController::kDepthLimit; limit = c.gatherObservations(limit)) { c.deliverObservations(); } if (c.skippedObservations()) { c.clearObservations(); sendError() } For efficiency, we avoid continuosly polling all elements for size changes. Instead, an Element notifies ResizeObservation when its size might have changed. ResizeObservation propagates this change to ResizeObserver, which propagates to ResizeObserverController. All of them set a flag indicating that they might have been changed. The flag is cleared when observations are delivered. BUG=612962 ========== to ========== ResizeObserver pt5: size change notification This change implements size change notification algorithm. Design doc at https://docs.google.com/document/d/1G4OmqqlFY3H3erQAUmteKES02-bIECikDjWBrDKH9... Size change notification We collect and deliver notifications about all Elements whose size has changed inside the event loop. The algorithm loops until all notifications are delivered, or it hits the depth limit. c = ResizeObserverController(); for (limit = c.gatherObservations(0); limit != ResizeObserverController::kDepthLimit; limit = c.gatherObservations(limit)) { c.deliverObservations(); } if (c.skippedObservations()) { c.clearObservations(); sendError() } For efficiency, we avoid continuosly polling all elements for size changes. Instead, an Element notifies ResizeObservation when its size might have changed. ResizeObservation propagates this change to ResizeObserver, which propagates to ResizeObserverController. All of them set a flag indicating that they might have been changed. The flag is cleared when observations are delivered. BUG=612962 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== ResizeObserver pt5: size change notification This change implements size change notification algorithm. Design doc at https://docs.google.com/document/d/1G4OmqqlFY3H3erQAUmteKES02-bIECikDjWBrDKH9... Size change notification We collect and deliver notifications about all Elements whose size has changed inside the event loop. The algorithm loops until all notifications are delivered, or it hits the depth limit. c = ResizeObserverController(); for (limit = c.gatherObservations(0); limit != ResizeObserverController::kDepthLimit; limit = c.gatherObservations(limit)) { c.deliverObservations(); } if (c.skippedObservations()) { c.clearObservations(); sendError() } For efficiency, we avoid continuosly polling all elements for size changes. Instead, an Element notifies ResizeObservation when its size might have changed. ResizeObservation propagates this change to ResizeObserver, which propagates to ResizeObserverController. All of them set a flag indicating that they might have been changed. The flag is cleared when observations are delivered. BUG=612962 ========== to ========== ResizeObserver pt5: size change notification This change implements size change notification algorithm. Design doc at https://docs.google.com/document/d/1G4OmqqlFY3H3erQAUmteKES02-bIECikDjWBrDKH9... Size change notification We collect and deliver notifications about all Elements whose size has changed inside the event loop. The algorithm loops until all notifications are delivered, or it hits the depth limit. c = ResizeObserverController(); for (limit = c.gatherObservations(0); limit != ResizeObserverController::kDepthLimit; limit = c.gatherObservations(limit)) { c.deliverObservations(); } if (c.skippedObservations()) { c.clearObservations(); sendError() } For efficiency, we avoid continuosly polling all elements for size changes. Instead, an Element notifies ResizeObservation when its size might have changed. ResizeObservation propagates this change to ResizeObserver, which propagates to ResizeObserverController. All of them set a flag indicating that they might have been changed. The flag is cleared when observations are delivered. BUG=612962 Committed: https://crrev.com/3d5b99673799a3067e9a5b8621f2b9ef2ff5d5d5 Cr-Commit-Position: refs/heads/master@{#408867} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/3d5b99673799a3067e9a5b8621f2b9ef2ff5d5d5 Cr-Commit-Position: refs/heads/master@{#408867} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
