Regression: Blank sheet is seen on preview in Google drive
Reported by
nutan.ga...@etouch.net,
Dec 18 2017
|
|||||||
Issue descriptionChrome version: 65.0.3298.0 6586a208880a70a00856529493741971ecfea5c2-refs/heads/master@{#524617} OS: Windows Step to reproduce? 1. Launch chrome, navigate to https://drive.google.com and login in with valid crediential 2. Right click on any sheet and click on preview 3. Observe Actual: Blank sheet is observed on preview Expected: Content should be seen on preview This is an Regression issue seen from M-65, will soon update other info:
,
Dec 18 2017
Adding release blocker for this issue.Please remove if not the case. Thank You!
,
Dec 18 2017
This smells similar to the pdf plugin issue. Investigating.
,
Dec 18 2017
I've confirmed that setting '--disable-blink-features=DisplayNoneIFrameCreatesNoLayoutObject' fixes the problem. It looks like the HTML element is incorrectly having its height set to 0. If I use the element inspector to manually set the height of the HTML element to 800, the height of the body element to 800, and the height&width of div id="sheets-viewport" to 800, then some contents shows up.
,
Dec 18 2017
Thank you erikchen@. Can this "--disable-blink-features=DisplayNoneIFrameCreatesNoLayoutObject" via Finch to unblock tomorrow's dev release?
,
Dec 18 2017
Per erikchen@, revert in CQ: https://chromium-review.googlesource.com/c/chromium/src/+/833347. I've requested to merge the revert to 3298 branch once it lands.
,
Dec 19 2017
skobes: I've got a repro and potential fixes.
The repro looks like this:
"""
function resize() {
var el = document.getElementById('blah');
el.style.width = window.innerWidth;
el.style.height = window.innerHeight;
}
resize();
window.onresize = resize
"""
The assumption is that this will cause "el" to stay in sync with window. This assumption is violated by this feature. Adding a call to EnqueueResizeEvent() in Document::WillChangeFrameOwnerProperties fixes the problem, e.g.
"""
if (RuntimeEnabledFeatures::DisplayNoneIFrameCreatesNoLayoutObjectEnabled()) {
if (documentElement()) {
if (is_display_none != owner->IsDisplayNone()) {
documentElement()->LazyReattachIfAttached();
if (!is_display_none)
EnqueueResizeEvent();
}
}
}
"""
Or perhaps the right solution is to update LocalFrameView::WasViewportResized and LocalFrameView::SendResizeEventIfNeeded() so that a resize event is sent GetLayoutViewItem changes from null to non-null.
I think the latter is cleaner. Thoughts?
,
Dec 19 2017
The LayoutView shouldn't be null, even when the iframe is display: none. It makes sense that innerWidth and innerHeight are 0 when the iframe is display: none. Therefore, it makes sense to fire a resize event when the iframe becomes displayed. When the frame beomes displayed, it should run a layout, and call SendResizeEventIfNeeded. Does WasViewportResized return false at this time? If so, why?
,
Dec 19 2017
Erik merged the revert to branch 3298 to unblock tomorrow's dev release - https://chromium.googlesource.com/chromium/src.git/+/af1f64f3e9f29a8e287e7945542810687c22b5f6. Thank you Erik.
,
Dec 19 2017
update in comment 10: Able to see 2 sec white page before the sheet preview opens also in Dev #65.0.3298.3 Thank you
,
Dec 19 2017
Erik, do you think this is a Dev blocker?
,
Dec 19 2017
If you believe there's a regression, feel free to take a bisect and find the issue. I turned on a feature, and then turned it back off.
,
Dec 19 2017
,
Dec 19 2017
Please file a different bug with a bisect for c#10. This issue tracks the failure-to-appear for the sheet itself.
,
Dec 19 2017
In my opinion, the video in comment #10 shows the expected behavior, and not a bug. The two-second period in which the page appears white corresponds to the time spent fetching data from the network, as evidenced by the "Waiting for docs.google.com..." status indicator in the lower left.
,
Dec 20 2017
@skobes: I've written a layout test with a minimized repro: https://chromium-review.googlesource.com/c/chromium/src/+/834770 With the feature disabled, the first call to blink::LocalFrameView::UpdateLayout sets first_layout_ to false, and last_viewport_size_ to 0x0. """ 437 3 libblink_core.dylib 0x000000013c3a8927 blink::LocalFrameView::UpdateLayout() + 2903 438 4 libblink_core.dylib 0x000000013c3bdba4 blink::LocalFrameView::UpdateStyleAndLayoutIfNeededRecursiveInternal() + 884 439 5 libblink_core.dylib 0x000000013c3bdda3 blink::LocalFrameView::UpdateStyleAndLayoutIfNeededRecursiveInternal() + 1395 440 6 libblink_core.dylib 0x000000013c3bb1ac blink::LocalFrameView::UpdateStyleAndLayoutIfNeededRecursive() + 140 441 7 libblink_core.dylib 0x000000013c3b9955 blink::LocalFrameView::UpdateLifecyclePhasesInternal(blink::DocumentLifecycle::LifecycleState) + 901 442 8 libblink_core.dylib 0x000000013c3b95c2 blink::LocalFrameView::UpdateAllLifecyclePhases() + 50 443 9 libblink_core.dylib 0x000000013d0ac625 blink::PageAnimator::UpdateAllLifecyclePhases(blink::LocalFrame&) + 85 444 10 libblink_core.dylib 0x000000013d0b3db5 blink::PageWidgetDelegate::UpdateAllLifecyclePhases(blink::Page&, blink::LocalFrame&) + 37 445 11 libblink_core.dylib 0x000000013c2d980c blink::WebViewImpl::UpdateAllLifecyclePhases() + 428 446 12 libblink_core.dylib 0x000000013c4a08b1 blink::WebViewFrameWidget::UpdateAllLifecyclePhases() + 33 447 13 libcontent.dylib 0x000000012d6f505b content::RenderWidget::UpdateVisualState() + 43 448 14 libcontent.dylib 0x000000012d3652fa content::RenderWidgetCompositor::UpdateLayerTreeHost() + 26 449 15 libcc.dylib 0x000000010da4497a cc::LayerTreeHost::RequestMainFrameUpdate() + 26 """ The second call to blink::LocalFrameView::UpdateLayout sets layout_size_ to 50x40, but last_viewport_size_ is still 0x0. """ 515 3 libblink_core.dylib 0x000000013c3a26ad blink::LocalFrameView::SetLayoutSizeInternal(blink::IntSize const&) + 349 516 4 libblink_core.dylib 0x000000013c3a6839 blink::LocalFrameView::FrameRectsChanged() + 297 517 5 libblink_core.dylib 0x000000013c3a6605 blink::LocalFrameView::SetFrameRect(blink::IntRect const&) + 261 518 6 libblink_core.dylib 0x000000013cc3bc3b blink::LayoutEmbeddedContent::UpdateGeometry(blink::EmbeddedContentView&) + 475 519 7 libblink_core.dylib 0x000000013c3afdae blink::LocalFrameView::UpdateGeometry() + 510 520 8 libblink_core.dylib 0x000000013c3af979 blink::LocalFrameView::UpdateGeometries() + 201 521 9 libblink_core.dylib 0x000000013c3ab96e blink::LocalFrameView::PerformPostLayoutTasks() + 798 522 10 libblink_core.dylib 0x000000013c3adea7 blink::LocalFrameView::ScheduleOrPerformPostLayoutTasks() + 103 523 11 libblink_core.dylib 0x000000013c3a96a1 blink::LocalFrameView::UpdateLayout() + 6353 524 12 libblink_core.dylib 0x000000013bd763c0 blink::Document::ImplicitClose() + 1008 525 13 libblink_core.dylib 0x000000013bd75dcb blink::Document::CheckCompleted() + 235 526 14 libblink_core.dylib 0x000000013cf8e2b0 blink::FrameLoader::DidFinishNavigation() + 624 527 15 libblink_core.dylib 0x000000013bd75faa blink::Document::CheckCompleted() + 714 528 16 libblink_core.dylib 0x000000013cf8dced blink::FrameLoader::FinishedParsing() + 317 529 17 libblink_core.dylib 0x000000013bd87534 blink::Document::FinishedParsing() + 884 530 18 libblink_core.dylib 0x000000013df95588 blink::HTMLConstructionSite::FinishedParsing() + 232 531 19 libblink_core.dylib 0x000000013e0272e0 blink::HTMLTreeBuilder::Finished() + 384 532 20 libblink_core.dylib 0x000000013dfae55f blink::HTMLDocumentParser::end() + 383 533 21 libblink_core.dylib 0x000000013dfa2cc8 blink::HTMLDocumentParser::AttemptToRunDeferredScriptsAndEnd() + 440 534 22 libblink_core.dylib 0x000000013dfa2a05 blink::HTMLDocumentParser::PrepareToStopParsing() + 581 535 23 libblink_core.dylib 0x000000013dfa9c8b blink::HTMLDocumentParser::ProcessTokenizedChunkFromBackgroundParser(std::__1::unique_ptr<blink::HTMLDocumentParser::TokenizedChunk, std::__1::default_delete<blink::HTMLDocumentParser: :TokenizedChunk> >) + 5851 536 24 libblink_core.dylib 0x000000013dfa45f3 blink::HTMLDocumentParser::PumpPendingSpeculations() + 1571 537 25 libblink_core.dylib 0x000000013dfb0357 blink::HTMLDocumentParser::ResumeParsingAfterPause() + 1383 538 26 libblink_core.dylib 0x000000013dfb089a blink::HTMLDocumentParser::NotifyScriptLoaded(blink::PendingScript*) + 490 539 27 libblink_core.dylib 0x000000013dfdb981 blink::HTMLParserScriptRunner::PendingScriptFinished(blink::PendingScript*) + 497 540 28 libblink_core.dylib 0x000000013bd1d486 blink::ClassicPendingScript::AdvanceReadyState(blink::ClassicPendingScript::ReadyState) + 950 541 29 libblink_core.dylib 0x000000013bd1ce8f blink::ClassicPendingScript::FinishWaitingForStreaming() + 479 542 30 libblink_core.dylib 0x000000013bd1da50 blink::ClassicPendingScript::NotifyFinished(blink::Resource*) + 384 543 31 libblink_platform.dylib 0x0000000143841850 blink::Resource::NotifyFinished() + 304 """ This causes future calls to LocalFrameView::SendResizeEventIfNeeded to trigger a EnqueueResizeEvent. With the feature enabled, the first call never happens. When the second call happens, first_layout_ is true, so both layout_size_ and last_viewport_size_ are set to 50x40. Thus, the resize event is never queued. When considering the main frame, we only see a call to blink::LocalFrameView::UpdateLayout, which causes the resize event to correctly not be called.
,
Dec 22 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6c95c9a63d9e8ad3c03c54a5cb160b87ba59866d commit 6c95c9a63d9e8ad3c03c54a5cb160b87ba59866d Author: Steve Kobes <skobes@chromium.org> Date: Fri Dec 22 22:30:01 2017 Mark empty LayoutView for initial layout. Typically the LayoutView is marked for layout when it acquires children, who mark themselves and their containers. But we want an initial layout pass even if the LayoutView has no children, for bookkeeping tasks like setting LocalFrameView::last_viewport_size_. Setting last_viewport_size_ during the initial layout ensures that we send a resize event when an iframe leaves the display: none state. Two paint invalidation tests have updated baselines reflecting earlier removal of an iframe's initial vertical scrollbar. Bug: 795661 Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: I3e7984b2f4cd012056e964114a803b484da584f1 Reviewed-on: https://chromium-review.googlesource.com/838249 Reviewed-by: Erik Chen <erikchen@chromium.org> Commit-Queue: Steve Kobes <skobes@chromium.org> Cr-Commit-Position: refs/heads/master@{#526073} [add] https://crrev.com/6c95c9a63d9e8ad3c03c54a5cb160b87ba59866d/third_party/WebKit/LayoutTests/fast/events/resize-display-none-iframe.html [modify] https://crrev.com/6c95c9a63d9e8ad3c03c54a5cb160b87ba59866d/third_party/WebKit/LayoutTests/flag-specific/enable-slimming-paint-v2/paint/invalidation/iframe-display-none-to-display-block-expected.txt [modify] https://crrev.com/6c95c9a63d9e8ad3c03c54a5cb160b87ba59866d/third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/mac/paint/invalidation/iframe-display-none-to-display-block-expected.txt [modify] https://crrev.com/6c95c9a63d9e8ad3c03c54a5cb160b87ba59866d/third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/mac/paint/invalidation/position/shift-relative-positioned-container-with-image-addition-expected.txt [modify] https://crrev.com/6c95c9a63d9e8ad3c03c54a5cb160b87ba59866d/third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/win/paint/invalidation/iframe-display-none-to-display-block-expected.txt [modify] https://crrev.com/6c95c9a63d9e8ad3c03c54a5cb160b87ba59866d/third_party/WebKit/LayoutTests/flag-specific/root-layer-scrolls/platform/win/paint/invalidation/position/shift-relative-positioned-container-with-image-addition-expected.txt [modify] https://crrev.com/6c95c9a63d9e8ad3c03c54a5cb160b87ba59866d/third_party/WebKit/LayoutTests/platform/mac/paint/invalidation/iframe-display-none-to-display-block-expected.txt [modify] https://crrev.com/6c95c9a63d9e8ad3c03c54a5cb160b87ba59866d/third_party/WebKit/LayoutTests/platform/mac/paint/invalidation/position/shift-relative-positioned-container-with-image-addition-expected.txt [modify] https://crrev.com/6c95c9a63d9e8ad3c03c54a5cb160b87ba59866d/third_party/WebKit/LayoutTests/platform/win/paint/invalidation/iframe-display-none-to-display-block-expected.txt [modify] https://crrev.com/6c95c9a63d9e8ad3c03c54a5cb160b87ba59866d/third_party/WebKit/LayoutTests/platform/win/paint/invalidation/position/shift-relative-positioned-container-with-image-addition-expected.txt [modify] https://crrev.com/6c95c9a63d9e8ad3c03c54a5cb160b87ba59866d/third_party/WebKit/LayoutTests/virtual/spv175/paint/invalidation/iframe-display-none-to-display-block-expected.txt [modify] https://crrev.com/6c95c9a63d9e8ad3c03c54a5cb160b87ba59866d/third_party/WebKit/LayoutTests/virtual/spv175/paint/invalidation/position/shift-relative-positioned-container-with-image-addition-expected.txt [modify] https://crrev.com/6c95c9a63d9e8ad3c03c54a5cb160b87ba59866d/third_party/WebKit/Source/core/frame/LocalFrameView.cpp
,
Dec 22 2017
|
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by nutan.ga...@etouch.net
, Dec 18 2017Owner: erikc...@chromium.org
Status: Assigned (was: Untriaged)
1.2 MB
1.2 MB View Download