Remove extraneous frameView->setNeedsLayout() in LayoutPart

Remove extraneous frameView->setNeedsLayout() in LayoutPart

This CL is one step towards having saner pixel snapping for sub-frames.
The goal is to remove dependencies to roundedIntRect, which is conceptually
incorrect and is inconsistent with other part of our code base.

The setNeedsLayout is extraneous because LayoutPart is not supposed to know
whether the sub-frame actually changed its layout size, and the FrameView is
responsible for scheduling layout if it decided to mutate layout size.

Committed: https://crrev.com/6f72ce705eea7d49483b1cc75439260ebde643c4
Cr-Commit-Position: refs/heads/master@{#408529}

[trchen, 2016-07-27 22:58:03]

trchen@chromium.org changed reviewers:
+ bokan@chromium.org

[trchen, 2016-07-28 00:04:36]

https://codereview.chromium.org/2188483004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/FrameSelectionTest.cpp (left):

https://codereview.chromium.org/2188483004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/FrameSelectionTest.cpp:129:
EXPECT_EQ(startCount, layoutCount());
This is bogus. Resizing the frame would invalidate layout, and painting requires
a clean layout. It is wrong to expect layoutCount() not increased.

The original CL that introduced this test to make sure caret painting code is
impossible to invoke layout, but I think the method in that CL was wrong. We
should have figured out why layout could be dirty during paint.

[trchen, 2016-07-28 00:05:16]

Description was changed from

==========
Remove extraneous frameView->setNeedsLayout in LayoutPart

This CL is one step towards having saner pixel snapping for sub-frames.
The goal is to remove dependencies to roundedIntRect, which is conceptually
incorrect and is inconsistent with other part of our code base.

The setNeedsLayout is extraneous because LayoutPart is not supposed to know
whether the sub-frame actually changed its layout size, and the FrameView is
responsible for scheduling layout if it decided to mutate layout size.
==========

to

==========
Remove extraneous frameView->setNeedsLayout() in LayoutPart

This CL is one step towards having saner pixel snapping for sub-frames.
The goal is to remove dependencies to roundedIntRect, which is conceptually
incorrect and is inconsistent with other part of our code base.

The setNeedsLayout is extraneous because LayoutPart is not supposed to know
whether the sub-frame actually changed its layout size, and the FrameView is
responsible for scheduling layout if it decided to mutate layout size.
==========

[bokan, 2016-07-28 13:24:56]

love it. lgtm.

https://codereview.chromium.org/2188483004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/FrameSelectionTest.cpp (left):

https://codereview.chromium.org/2188483004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/FrameSelectionTest.cpp:129:
EXPECT_EQ(startCount, layoutCount());
On 2016/07/28 00:04:36, trchen wrote:
> This is bogus. Resizing the frame would invalidate layout, and painting
requires
> a clean layout. It is wrong to expect layoutCount() not increased.
> 
> The original CL that introduced this test to make sure caret painting code is
> impossible to invoke layout, but I think the method in that CL was wrong. We
> should have figured out why layout could be dirty during paint.

Agree, this is sketchy. Could you file a bug to follow up with your suggestion
of figuring out why layout can be dirty during paint?

[trchen, 2016-07-28 22:45:54]

https://codereview.chromium.org/2188483004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/FrameSelectionTest.cpp (left):

https://codereview.chromium.org/2188483004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/FrameSelectionTest.cpp:129:
EXPECT_EQ(startCount, layoutCount());
On 2016/07/28 13:24:56, bokan wrote:
> On 2016/07/28 00:04:36, trchen wrote:
> > This is bogus. Resizing the frame would invalidate layout, and painting
> requires
> > a clean layout. It is wrong to expect layoutCount() not increased.
> > 
> > The original CL that introduced this test to make sure caret painting code
is
> > impossible to invoke layout, but I think the method in that CL was wrong. We
> > should have figured out why layout could be dirty during paint.
> 
> Agree, this is sketchy. Could you file a bug to follow up with your suggestion
> of figuring out why layout can be dirty during paint?

I think the original bug is already solved. Xianzhu recently invested a lot of
time to investigate layout lifecycle bugs. We now have many more DCHECK and
fixed them. Here is the meta bug: crbug.com/590856

[trchen, 2016-07-28 22:46:00]

The CQ bit was checked by trchen@chromium.org

[commit-bot: I haz the power, 2016-07-28 22:46:22]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[bokan, 2016-07-28 22:48:41]

https://codereview.chromium.org/2188483004/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/editing/FrameSelectionTest.cpp (left):

https://codereview.chromium.org/2188483004/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/editing/FrameSelectionTest.cpp:129:
EXPECT_EQ(startCount, layoutCount());
On 2016/07/28 22:45:54, trchen wrote:
> On 2016/07/28 13:24:56, bokan wrote:
> > On 2016/07/28 00:04:36, trchen wrote:
> > > This is bogus. Resizing the frame would invalidate layout, and painting
> > requires
> > > a clean layout. It is wrong to expect layoutCount() not increased.
> > > 
> > > The original CL that introduced this test to make sure caret painting code
> is
> > > impossible to invoke layout, but I think the method in that CL was wrong.
We
> > > should have figured out why layout could be dirty during paint.
> > 
> > Agree, this is sketchy. Could you file a bug to follow up with your
suggestion
> > of figuring out why layout can be dirty during paint?
> 
> I think the original bug is already solved. Xianzhu recently invested a lot of
> time to investigate layout lifecycle bugs. We now have many more DCHECK and
> fixed them. Here is the meta bug: crbug.com/590856

ok, sgtm, thanks.

[commit-bot: I haz the power, 2016-07-29 00:20:27]

Description was changed from

==========
Remove extraneous frameView->setNeedsLayout() in LayoutPart

This CL is one step towards having saner pixel snapping for sub-frames.
The goal is to remove dependencies to roundedIntRect, which is conceptually
incorrect and is inconsistent with other part of our code base.

The setNeedsLayout is extraneous because LayoutPart is not supposed to know
whether the sub-frame actually changed its layout size, and the FrameView is
responsible for scheduling layout if it decided to mutate layout size.
==========

to

==========
Remove extraneous frameView->setNeedsLayout() in LayoutPart

This CL is one step towards having saner pixel snapping for sub-frames.
The goal is to remove dependencies to roundedIntRect, which is conceptually
incorrect and is inconsistent with other part of our code base.

The setNeedsLayout is extraneous because LayoutPart is not supposed to know
whether the sub-frame actually changed its layout size, and the FrameView is
responsible for scheduling layout if it decided to mutate layout size.
==========

[commit-bot: I haz the power, 2016-07-29 00:20:28]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-07-29 00:25:24]

Description was changed from

==========
Remove extraneous frameView->setNeedsLayout() in LayoutPart

This CL is one step towards having saner pixel snapping for sub-frames.
The goal is to remove dependencies to roundedIntRect, which is conceptually
incorrect and is inconsistent with other part of our code base.

The setNeedsLayout is extraneous because LayoutPart is not supposed to know
whether the sub-frame actually changed its layout size, and the FrameView is
responsible for scheduling layout if it decided to mutate layout size.
==========

to

==========
Remove extraneous frameView->setNeedsLayout() in LayoutPart

This CL is one step towards having saner pixel snapping for sub-frames.
The goal is to remove dependencies to roundedIntRect, which is conceptually
incorrect and is inconsistent with other part of our code base.

The setNeedsLayout is extraneous because LayoutPart is not supposed to know
whether the sub-frame actually changed its layout size, and the FrameView is
responsible for scheduling layout if it decided to mutate layout size.

Committed: https://crrev.com/6f72ce705eea7d49483b1cc75439260ebde643c4
Cr-Commit-Position: refs/heads/master@{#408529}
==========

[commit-bot: I haz the power, 2016-07-29 00:25:26]

Patchset 2 (id:??) landed as
https://crrev.com/6f72ce705eea7d49483b1cc75439260ebde643c4
Cr-Commit-Position: refs/heads/master@{#408529}