wwozniak92@ - Thanks for the issue..

Able to reproduce the issue on Windows 7, Mac 10.12.6 and Ubuntu 14.04 using chrome stable version #60.0.3112.101 and latest canary #62.0.3194.0

Bisect Information:
=====================
Good build: 59.0.3056.0	 
Bad Build : 59.0.3057.0	 

Change Log URL: 
https://chromium.googlesource.com/chromium/src/+log/174eb44312202a5132de900553f7c302da0feef6..85bab14c835935e79a0b63f24b47f671840373de

From the above change log suspecting below change
Review URL: https://codereview.chromium.org/2743053003

erikchen@  Could you please check whether this issue is related with to your change, if not please help us in assigning it to the right owner. 

Thanks...!!

weird. I was able to reproduce this on my macOS 10.12.6 laptop, but not with my macOS 10.12.6 desktop [tried trackpad + mice].

As an aside, this repro also produces issues in Safari. Trackpad scrolling works prior to step (2), but fails after step (2). 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1b7a82bab5c46e2cd2ab53c8d2e91f27a6d0ac37

commit 1b7a82bab5c46e2cd2ab53c8d2e91f27a6d0ac37
Author: Erik Chen <erikchen@chromium.org>
Date: Tue Aug 29 17:15:53 2017

Move feature behind RuntimeEnabledFeatures conditional.

The DisplayNoneIFrameCreatesNoLayoutObjectEnabled feature had a
LazyReattachIfAttached() call in Document::WillChangeFrameOwnerProperties that
was accidentally not wrapped in a RuntimeEnabledFeatures conditional.

Bug:  749737 ,  758130 ,  650433 
Change-Id: I0482ca470bc0f78aec4ea1f7b82134e565aaa43b
Reviewed-on: https://chromium-review.googlesource.com/639362
Reviewed-by: Stefan Zager <szager@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498149}
[modify] https://crrev.com/1b7a82bab5c46e2cd2ab53c8d2e91f27a6d0ac37/third_party/WebKit/Source/core/dom/Document.cpp

I looked into this a bit.  The root cause is that detaching an <iframe> element calls Dispose() on its LocalFrameView, but leaves it alive in the frame tree.  The stack looks like this:

  blink::LocalFrameView::Dispose()
  blink::HTMLFrameOwnerElement::SetEmbeddedContentView()
  blink::LayoutEmbeddedContent::WillBeDestroyed()
  blink::LayoutEmbeddedContent::Destroy()
  blink::LayoutObject::DestroyAndCleanupAnonymousWrappers()
  blink::Node::DetachLayoutTree()
  ...

When the iframe is redisplayed, this LocalFrameView gets reused.  But reusing a LocalFrameView after Dispose()'ing it is bad.

The observed scrollbar behavior occurs because Dispose() destroys the ScrollAnimator object, which stores its own copy of the scroll offset.  We end up creating a new ScrollAnimator with offset == 0 for the existing LocalFrameView (whose offset is not 0).

Should a display: none iframe have a LocalFrameView?  I'm not sure actually.  If yes, we should avoid calling Dispose() until it's removed from the DOM.  If no, then we should clear LocalFrame::view_ at the same time as Dispose(), so that it is truly destroyed.

Thanks for looking into this!

> Should a display: none iframe have a LocalFrameView? 
I have no idea! Who would be appropriate to cc onto this bug?

+dcheng might have thoughts

Tested this issue on Windows 7,Mac 10.12.6 & Ubuntu 14.04 using latest canary #62.0.3200.0 as per the steps mentioned in c#0.

1. Navigate to  https://codepen.io/wojtekw92/pen/xLzgMJ/  --scrollbar is visible
2. Click hide button twice
3. Observed that the scrollbar is not visible

Hence unable to check this issue.Please find the attached screencast for reference & let us know your observations on the same.

Thanks..!

Deleted: 758130.mp4 641 KB

	758130.mp4 641 KB View Download

@jmukthavaram for what it's worth: I can confirm that Canary has a new regression (toolbar is not even visible anymore) that makes testing this issue impossible.

Verified on the exact same version, although using Windows10:
Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/62.0.3200.0 Safari/537.36

+joelhockey, who did a bunch of refactoring in this area.

- We probably shouldn't leave the LocalFrameView lying around if it's been disposed, which implies we should clear various pointers to it.
- That implies that we'd have to change Document::Shutdown() to handle a potentially null FrameView, which I'd rather not do.
- Given that, it's probably better to try to leave the LocalFrameView live?

Tested with Windows 7, Version 61.0.3163.91 (Official Build) (64-bit).
Tested with Windows 10, Version 61.0.3163.91 (Official Build) (64-bit).
Issue is there.

I have also tried the below steps to reproduce the issue.
Steps I have tried to reproduce the problem:

	1. Go to this https://codepen.io/wojtekw92/pen/xLzgMJ/
	2. Click hide button to hide the iFrame
	3. Click hide button again to show the iFrame
	4. Scroll down using mouse wheel
	Result : It jumps right at top.

Second attempt with different steps:

	1. Go to this https://codepen.io/wojtekw92/pen/xLzgMJ/
	2. Click hide button to hide the iFrame
	3. Click hide button again to show the iFrame
	4. Click the iFrame content area
	5. Scroll down using mouse wheel
	Result : It jumps right at top.

Third attempt with different steps:

	1. Go to this https://codepen.io/wojtekw92/pen/xLzgMJ/
	2. Click hide button to hide the iFrame
	3. Click hide button again to show the iFrame
	4. Click the iFrame scroll and try to move up or down
	5. Scroll down using mouse wheel
	Result : It DOESN'T jumps right at top, it holds the position fine.

I just came through this thread looking for solution of an issue I was having. I have posted it at : https://productforums.google.com/d/msg/chrome/00KLAItQnrU/l_B4tHzwAgAJ

If we find solution for this issue, that will resolve my issue exactly.

Tested with Windows 7, Version 61.0.3163.100 (Official Build) (64-bit).
Tested with Windows 10, Version 61.0.3163.100 (Official Build) (64-bit).
Issue Persist.

Unassigning myself, so that an appropriate owner is found.

Is there going to be any solution available soon for this issue ?
I need to know otherwise I will have to move my project to different browser. This issue is annoying my users currently in Chrome and they are not able to use my tool in Chrome anymore. Please help as soon as possible.

Note that you can set "visibility: hidden" instead of "display: none" as a workaround.

I'll see how hard it is to prevent disposing until we remove it from the DOM.

Will try and get a fix in for M63.

Hi @bokan,

Does it mean we will see fix possibly on Version 63.x.xxxx.xxx (Official Build)?

Thanks.

I'll have to take a look ASAP and we'll see how complex the fix is as M63 has already branched so if it's a big change it might have to wait until 64.

@bokan,

Thanks for the quick response first of all.
If its going to be fixed early or late, that should be okay as long as I know and I can hope for.

My question was more of if M63 means Version 63.x.xxxx.xxx (Official Build)?
I do not know how M63 or M64 is being used and what does that mean.

Thanks.

Sorry, yeah, M means milestone which is the first number in the Chrome version number. So yes, that means 63.x.xxxx.xxx

I'm trying to write a test for this and while it easily reproduces in content_shell, I can't seem to get the same effect in an actual layout test. After much tebugging, it seems like in a LayoutTest, after toggling display: none, the child Document doesn't have a ComputedStyle so we set needs style recalc which eventually creates a new scrollbar. When running in a real content_shell, toggling `display: none` and printing out the ComputedStyle shows it's still the same object and so we don't recalc style.

skobes@, any idea what's going on or what the difference between content_shell and a layout test might be?

DisplayNoneIFrameCreatesNoLayoutObject is status: "experimental", so it's on for layout tests.

It looks like that feature actually fixes this bug.  When it's on, the layout tree under the child Document gets destroyed (triggering scroll offset clamp to 0) and reconstructed (triggering scrollbar creation).  We still reuse a Dispose'd LocalFrameView, which is problematic, but all the scroll logic ends up in sync.

@erikchen, should we just ship DisplayNoneIFrameCreatesNoLayoutObject? :)

woot!

I've set aside a block of time tomorrow to look into DisplayNoneIFrameCreatesNoLayoutObject and figure out what the blockers are to shipping it.

BTW this isn't really a regression, although the bisect in #1 identified a change in the symptom.  Before r460646 (M59) the scrollbar is missing, because LocalFrameView::Dispose calls DetachScrollbars.

Between r460646 and r498149, the scrollbar is shown, but out of sync with the ScrollAnimator.  It's probably shown because calling LazyReattachIfAttached from Document::WillChangeFrameOwnerProperties triggers relayout, restoring the scrollbar.  It's out of sync for the reasons described in #6.

After r498149 (M62) the scrollbar is just missing again.

Issue 771757 tracks fixing reuse-after-Detach and gives some more context on DisplayNoneIFrameCreatesNoLayoutObject.  A quicker fix for this bug may actually be to revert r498149 (thus forcing layout on frame owner changes) and make ScrollAnimatorBase ctor initialize current_offset_ by reading from the ScrollableArea, so that it's reconstructed with the correct offset.

 Issue 658230  has been merged into this issue.

Assigning to Eric since the fix is shipping DisplayNoneIFrameCreatesNoLayoutObject

Inverting dependency between this bug and 650433.  The latter is close to shipping.

Hi all,

I have just installed Dev version of google chrome to test the issue.
Version 65.0.3325.31 (Official Build) dev (64-bit)

I have noticed the change in behavior though the issue is not resolved.
Here are the steps I have followed to test.

	1. Go to this https://codepen.io/wojtekw92/pen/xLzgMJ/
	2. Click hide button to hide the iFrame
	3. Click hide button again to show the iFrame
	
	Result : The scrollbar is visible however not holding the previous position. The focus is on the top of the page. Notice the scrollbar is right on the top.

Deleted: Scroll position lost and moved to the top.png 3.1 KB

	Scroll position lost and moved to the top.png 3.1 KB View Download

The iframe has no scrollable content while it is hidden, so the scroll position is clamped to 0.  It's the same as if you had set display: none on the <html> element inside the iframe.

This issue is resolved in that the scrollbar is rendered in sync with the iframe's scroll offset.  If your page needs the scroll offset to be restored to its original value you should do so explicitly in Javascript.

Hi @skobes

First of all thanks for looking into this.
This is still not resolved.

Instead of the above example in codepen, I have created one with iFrame which has external url to show content.

https://codepen.io/pl47er/pen/mXJNJB

1 - Open the link
2 - Scroll the content down to ensure its not on top
3 - Click "Hide / Show" button to hide the iframe
4 - Click "Hide / Show" button to show the iframe

Now notice that the scroll position is lost and the page displayed is positioned on top of the page.

Expected behavior is it should still hold the previous scroll position which is happening with any other browser I tested e.g. IE 11, Edge 15, Firefox 57.

I would request to get this fixed as I would expect the normal behavior from Google Chrome too. It should hold the scroll position when we hide the iframe and show it again.

Filed issue 808072 to track the issue of not restoring the scroll position.