Make NGFragment to own NGPhysicalFragment

Make NGFragment to own NGPhysicalFragment

Remove Oilpan from NGPhysicalFragment and make NGFragment to own it through unique_ptr.
This will fix the issue with limited number(it's about ~150 today) of supported physical
fragment children in LayoutNG code.

The problem can be reproduced by creating a tree with depth > 150 and running it through LayoutNG code.
Oilpan will crash with visitor->heap().stackFrameDepth().isAcceptableStackUse() once kSafeStackFrameSize > 32 * 1024 while trying to trace a vector

BUG=635619

[Gleb Lanbin, 2016-09-23 23:57:46]

Patchset #1 (id:1) has been deleted

[Gleb Lanbin, 2016-09-23 23:58:39]

glebl@chromium.org changed reviewers:
+ cbiesinger@chromium.org, ikilpatrick@chromium.org

[Gleb Lanbin, 2016-09-23 23:58:47]

The CQ bit was checked by glebl@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-23 23:59:06]

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

[commit-bot: I haz the power, 2016-09-24 00:42:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-24 00:42:56]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Gleb Lanbin, 2016-09-24 19:44:28]

Patchset #1 (id:20001) has been deleted

[Gleb Lanbin, 2016-09-25 18:44:00]

Patchset #2 (id:60001) has been deleted

[Gleb Lanbin, 2016-09-25 18:44:08]

Patchset #1 (id:40001) has been deleted

[Gleb Lanbin, 2016-09-25 22:19:45]

On 2016/09/23 23:58:39, glebl wrote:

I think it's probably better to discuss the HeapVector limitation issue offline.

[Gleb Lanbin, 2016-09-25 22:19:46]

On 2016/09/23 23:58:39, glebl wrote:

I think it's probably better to discuss the HeapVector limitation issue offline.

[Gleb Lanbin, 2016-09-25 22:19:59]

Description was changed from

==========
Make NGFragment to own NGPhysicalFragment

Remove Oilpan from NGPhysicalFragment and make NGFragment to own it through
unique_ptr.
This will fix the issue with limited number(it's about ~150 today) of supported
physical
fragment children in LayoutNG code.

The problem can be reproduced by creating a tree with depth > 150 and running it
through LayoutNG code.
Oilpan will crash with visitor->heap().stackFrameDepth().isAcceptableStackUse()
once kSafeStackFrameSize > 32 * 1024 while trying to trace a vector

BUG=635619
==========

to

==========
Make NGFragment to own NGPhysicalFragment

Remove Oilpan from NGPhysicalFragment and make NGFragment to own it through
unique_ptr.
This will fix the issue with limited number(it's about ~150 today) of supported
physical
fragment children in LayoutNG code.

The problem can be reproduced by creating a tree with depth > 150 and running it
through LayoutNG code.
Oilpan will crash with visitor->heap().stackFrameDepth().isAcceptableStackUse()
once kSafeStackFrameSize > 32 * 1024 while trying to trace a vector

BUG=635619
==========

[Gleb Lanbin, 2016-09-25 22:19:59]

glebl@chromium.org changed reviewers:
- cbiesinger@chromium.org, ikilpatrick@chromium.org

[cbiesinger, 2016-09-26 17:17:54]

cbiesinger@chromium.org changed reviewers:
+ cbiesinger@chromium.org

[cbiesinger, 2016-09-26 17:17:55]

disappointing that oilpan can't handle this :(

https://codereview.chromium.org/2365083002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc
(right):

https://codereview.chromium.org/2365083002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/ng/ng_block_layout_algorithm.cc:76: if
(!current_child_->Layout(constraint_space_for_children_.get()))
Why did you remove the out param?

https://codereview.chromium.org/2365083002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/ng/ng_box.cc (right):

https://codereview.chromium.org/2365083002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/ng/ng_box.cc:143: if (layout_box_ &&
fragment_->PhysicalFragment()) {
Can PhysicalFragment() ever be null?

https://codereview.chromium.org/2365083002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/ng/ng_constraint_space.h (right):

https://codereview.chromium.org/2365083002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/ng/ng_constraint_space.h:125:
DEFINE_INLINE_TRACE(){};
no semicolon here. shouldn't there be a space before {} ?

https://codereview.chromium.org/2365083002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/ng/ng_physical_constraint_space.h
(left):

https://codereview.chromium.org/2365083002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/ng/ng_physical_constraint_space.h:57:
class CORE_EXPORT NGPhysicalConstraintSpace final
Why remove the final?

https://codereview.chromium.org/2365083002/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/ng/ng_physical_fragment.h (right):

https://codereview.chromium.org/2365083002/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/ng/ng_physical_fragment.h:28: return
children_;
I really think you should document in the builder & in the header of this file
that the child fragments need to be kept alive by the caller as this class just
keeps a pointer to them. that's pretty subtle...