RESOLVED FIXED 236346
Continual resizeobserver-driven flashing on Jupyter notebook page 
https://bugs.webkit.org/show_bug.cgi?id=236346
Summary Continual resizeobserver-driven flashing on Jupyter notebook page
Simon Fraser (smfr)
Reported 2022-02-08 19:54:42 PST
Turn on always-on scrollbars on macOS, and load https://github.com/pytorch/opacus/blob/main/tutorials/intro_to_advanced_features.ipynb The content continually oscillates between two sizes, with and without the scrollbar showing. This resize is driven by a resizerObserver. Does not happen in Chrome.
Attachments
Patch (2.73 KB, patch)
2022-03-02 05:36 PST, cathiechen 		no flags
DetailsFormatted DiffDiff
Patch (8.51 KB, patch)
2022-03-09 08:32 PST, cathiechen 		no flags
DetailsFormatted DiffDiff
Patch (9.23 KB, patch)
2022-03-09 23:30 PST, cathiechen 		no flags
DetailsFormatted DiffDiff
Patch (10.09 KB, patch)
2022-03-10 01:35 PST, cathiechen 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Reduction (based on testcase) (1.58 KB, text/html)
2022-07-07 10:07 PDT, Simon Fraser (smfr) 		no flags
Details
Test showing incorrect iframe sizing behavior (1.06 KB, text/html)
2022-07-07 14:59 PDT, Simon Fraser (smfr) 		no flags
Details
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Simon Fraser (smfr)
Comment 1 2022-02-14 13:38:29 PST
<rdar://problem/88399120>
cathiechen
Comment 2 2022-02-15 07:11:47 PST
Yeah, I can reproduce it, and I will take a look today.
Simon Fraser (smfr)
Comment 3 2022-02-22 14:58:49 PST
Any progress here?
cathiechen
Comment 4 2022-02-23 08:47:24 PST
I'm trying to debug the page, but it seems very hard to simplify the page, so the progress is slow. It seems there is a loop in calculating the iframe size. The iframe with `width: 100%; height: 100%` is inside a `<div>` as a render container, iframe's size is determined by this `<div>`. Inside iframe, there is an resizeObserver observing the size of html, and it sends the html size to the main frame by a resize event. When the main frame gets the resize event, it sets the size to the `<div>`. The wierd part is, after the `<div>` gets the size that is calculated when iframe is without scrollbar, somehow with this size, iframe shows scrollbar, that causes a different size of iframe `<html>`, because the width is changed if scrollbar is on. Then it triggers another round, on and on.
cathiechen
Comment 5 2022-03-02 05:36:53 PST
Created attachment 453593 [details] Patch
cathiechen
Comment 6 2022-03-09 08:32:12 PST
Created attachment 454241 [details] Patch
cathiechen
Comment 7 2022-03-09 09:07:04 PST
Comment on attachment 454241 [details] Patch Hi Simon! I think the patch is ready for review. Please take a look, thanks!
cathiechen
Comment 8 2022-03-09 23:30:05 PST
Created attachment 454321 [details] Patch
cathiechen
Comment 9 2022-03-10 01:35:02 PST
Created attachment 454325 [details] Patch
Simon Fraser (smfr)
Comment 10 2022-07-07 10:07:04 PDT
Created attachment 460740 [details] Reduction (based on testcase)
Simon Fraser (smfr)
Comment 11 2022-07-07 12:14:43 PDT
(In reply to cathiechen from comment #4) > The wierd part is, after the `<div>` gets the size that is calculated when > iframe is without scrollbar, somehow with this size, iframe shows scrollbar, > that causes a different size of iframe `<html>`, because the width is > changed if scrollbar is on. > Then it triggers another round, on and on. I think this is the key. The testcase oscillates between two heights for the <iframe>, 328px (scrollbar hidden) and 346px (scrollbar visible). However, when we lay out with the height of 346px, we drop the scrollbar and size the iframe's <html> to 200x328, showing that we should be able to render in that size without the scrollbar. But ScrollView::updateScrollbars() brings the scrollbar back.
Simon Fraser (smfr)
Comment 12 2022-07-07 12:31:16 PDT
When the iframe height is set to 328px, we hit RenderWidget::setWidgetGeometry() with 200x328 and call into updateScrollbars(), but the document still has the old height of 346 so here: newHasVerticalScrollbar = docSize.height() > visibleHeight(); but this is based on stale layout and we don't do a second pass of scrollbars updates to figure out that we can actually fit.
Simon Fraser (smfr)
Comment 13 2022-07-07 14:32:36 PDT
Comment on attachment 454325 [details] Patch It's not clear to me that this is the correct fix yet. I think the fix is more about updateScrollbars()
Simon Fraser (smfr)
Comment 14 2022-07-07 14:59:51 PDT
Created attachment 460747 [details] Test showing incorrect iframe sizing behavior
Simon Fraser (smfr)
Comment 15 2022-07-07 22:16:33 PDT
Comment on attachment 454325 [details] Patch Actually this patch is very close to what I cam up with independently, so I'm confident it's close to correct. I will suggest some renaming. I also have a non-ResizeObserver testcase for iframe resizing which is a clearer demonstration of the underlying issue.
Simon Fraser (smfr)
Comment 16 2022-07-07 22:18:43 PDT
Sorry for the long delay in reviewing, Cathie, and I'm glad we converged on a similar solution. The code around updating scrollbars and layout is tricky, and I wanted to be sure that this was the right fix.
cathiechen
Comment 17 2022-07-08 15:01:24 PDT
Hi Simon! It's all right, I understand this part is tricky, and we need to be careful. And thanks for the review! I will rebase the code later:)
Simon Fraser (smfr)
Comment 18 2022-07-08 21:41:31 PDT
Pull request: https://github.com/WebKit/WebKit/pull/2258
EWS
Comment 19 2022-07-13 23:17:23 PDT
Committed 252440@main (912b33473fa3): <https://commits.webkit.org/252440@main> Reviewed commits have been landed. Closing PR #2258 and removing active labels.
Note You need to log in before you can comment on or make changes to this bug.