Remove frame flattening support as Chromium has no intention of using it

Remove frame flattening support as Chromium has no intention of using it

Chromium does not use the frame flattening code on any platform, though it used
to use it for frameset flattening on Android, but that is no longer the case.

As there is no intention of ever using it, it should be removed.

Committed on behalf of Kenneth Rohde Christiansen <kenneth.r.christiansen@intel.com>;. 

BUG=231898
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=148647

[jamesr, 2013-04-16 19:34:03]

awesome! lgtm

[ojan, 2013-04-16 19:37:10]

Looks like there was some duplicate effort here. Interestingly, the two patches
are a bit different (on the C++ side). I'll let you and Mike work out what the
right patch is. Both LGTM. :)

https://codereview.chromium.org/13877010/

[ojan, 2013-04-16 19:41:09]

I spotted two functional differences between the patches. 

This patch deletes more code because it assumes the Chromium side patch has
already landed (which is fine). Also, this patch is missing the RenderFrameBase
changes.

https://codereview.chromium.org/13963006/diff/1/Source/WebCore/page/FrameView...
File Source/WebCore/page/FrameView.cpp (left):

https://codereview.chromium.org/13963006/diff/1/Source/WebCore/page/FrameView...
Source/WebCore/page/FrameView.cpp:2035: if (m_frame->ownerRenderer() &&
isInChildFrameWithFrameFlattening())
This looks to be the only functional difference between this patch and Mike's.

https://codereview.chromium.org/13963006/diff/1/Source/WebCore/page/FrameView...
Source/WebCore/page/FrameView.cpp:3221: // When frame flattening is on, child
frame can mark parent frame dirty. In such case, child frame
Also, Mike's patch leaves in this assert for seamless.

[Mike West, 2013-04-16 20:34:38]

On 2013/04/16 19:41:09, ojan wrote:
> I spotted two functional differences between the patches. 
> 
> This patch deletes more code because it assumes the Chromium side patch has
> already landed (which is fine). Also, this patch is missing the
RenderFrameBase
> changes.
> 
>
https://codereview.chromium.org/13963006/diff/1/Source/WebCore/page/FrameView...
> File Source/WebCore/page/FrameView.cpp (left):
> 
>
https://codereview.chromium.org/13963006/diff/1/Source/WebCore/page/FrameView...
> Source/WebCore/page/FrameView.cpp:2035: if (m_frame->ownerRenderer() &&
> isInChildFrameWithFrameFlattening())
> This looks to be the only functional difference between this patch and Mike's.
> 
>
https://codereview.chromium.org/13963006/diff/1/Source/WebCore/page/FrameView...
> Source/WebCore/page/FrameView.cpp:3221: // When frame flattening is on, child
> frame can mark parent frame dirty. In such case, child frame
> Also, Mike's patch leaves in this assert for seamless.

I'm perfectly happy for you to land this patch as long as you grab the bits from
my patch that Ojan noted. :) I'm pretty sure this patch breaks seamless without
those edits.

[kenneth.christiansen, 2013-04-16 21:25:32]

> I'm perfectly happy for you to land this patch as long as you grab the bits
from
> my patch that Ojan noted. :) I'm pretty sure this patch breaks seamless
without
> those edits.

You are right, I must invalidate the parents for seamless as well.

I already did the changes, let me upload.

[kenneth.christiansen, 2013-04-16 21:32:11]

New patchset uploaded, thanks for the comments and your time!

[Use mkwst_at_chromium.org plz., 2013-04-16 21:37:58]

LGTM if you address the (small) points below, and unskip the tests you're
deleting by removing them from LayoutTests/TestExpectations.

I think you can safely carry over Ojan's LGTM to land this once you take care of
those bits.

https://codereview.chromium.org/13963006/diff/7001/Source/WebCore/page/FrameV...
File Source/WebCore/page/FrameView.cpp (right):

https://codereview.chromium.org/13963006/diff/7001/Source/WebCore/page/FrameV...
Source/WebCore/page/FrameView.cpp:2007: bool isSeamlessDocument =
m_frame->document()->shouldDisplaySeamlesslyWithParent();
As Ojan noted on the other patch, I don't think the intermediate variable is
necessary if you're dropping the null checks. You can just inline this into the
if clause below.

https://codereview.chromium.org/13963006/diff/7001/Source/WebCore/rendering/R...
File Source/WebCore/rendering/RenderFrameBase.cpp (right):

https://codereview.chromium.org/13963006/diff/7001/Source/WebCore/rendering/R...
Source/WebCore/rendering/RenderFrameBase.cpp:41: inline bool
shouldExpandFrame(LayoutUnit width, LayoutUnit height, bool hasFixedWidth, bool
hasFixedHeight)
This method can go as well.

Please also add a FIXME noting that this class should be removed entirely
(probably in the .h file). I'll take care of that once you land this patch. :)

[kenneth.christiansen, 2013-04-16 22:06:43]

Done

> Please also add a FIXME noting that this class should be removed entirely
> (probably in the .h file). I'll take care of that once you land this patch. :)

I just got rid of the class instead.

I had to remove the below line as LayoutTests/proximity doesn't exist anymore.
Was probably removed before moving to Blink.

LayoutTests/TestExpectations:235 Path does not exist. proximity

[ojan, 2013-04-16 22:11:02]

lgtm

Yay! The patch seems to think you're only deleting the contents of the
RenderFrameBase.*. It shows an "M" instead of a "D" next to the file. Maybe
that's just the tooling being confused though.

[kenneth.christiansen, 2013-04-16 22:14:02]

> Yay! The patch seems to think you're only deleting the contents of the
> RenderFrameBase.*. It shows an "M" instead of a "D" next to the file. Maybe
> that's just the tooling being confused though.

I did do git rm filename, so it should be fine.

[ojan, 2013-04-18 01:49:47]

The WebCore->core rename makes it hard to apply this patch to tip of tree. Mind
uploading a new one?

[kenneth.christiansen, 2013-04-18 09:00:39]

On 2013/04/18 01:49:47, ojan wrote:
> The WebCore->core rename makes it hard to apply this patch to tip of tree.
Mind
> uploading a new one?

No, I am on it!

[Mike West, 2013-04-18 13:00:48]

Committed patchset #5 manually as r148647 (presubmit successful).

[kenneth.christiansen, 2013-04-18 16:44:31]

On 2013/04/18 13:00:48, Mike West (chromium) wrote:
> Committed patchset #5 manually as r148647 (presubmit successful).

This was reverted as there was an issue with scrollbars not being shown. That
has been fixed in the last revision of the patch.