[LayoutNG] Convert physical fragments to being RefCounted.

[LayoutNG] Convert physical fragments to being RefCounted.

A few changes in this patch:
NGPhysicalFragment becomes RefCounted.
NGFragment "views" becomes stack allocated.
  (also remove an unused param).
NGFragmentBuilder moves off oil-pan to used being unique_ptr
  (this one was done because the "build" step would have required
   conversion from oilpan to non-oilpan code).

BUG=635619
Review-Url: https://codereview.chromium.org/2676533003
Cr-Commit-Position: refs/heads/master@{#448211}
Committed: https://chromium.googlesource.com/chromium/src/+/c4ef48dc4fb378c5d83ceb81b4417d74317596ae

[ikilpatrick, 2017-02-03 04:34:42]

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

[commit-bot: I haz the power, 2017-02-03 04:35:15]

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

[ikilpatrick, 2017-02-03 04:39:07]

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

[commit-bot: I haz the power, 2017-02-03 04:39:15]

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

[ikilpatrick, 2017-02-03 04:43:23]

Description was changed from

==========
[LayoutNG] Move fragments to being RefCounted.

BUG=
==========

to

==========
[LayoutNG] Convert physical fragments to being RefCounted.

A few changes in this patch:
NGPhysicalFragment becomes RefCounted.
NGFragment "views" become stack allocated.
  (also remove an unused param).
NGFragmentBuilder move off oil-pan to used being unique_ptr
  (this one was done because the "build" step would have required conversion
   from oilpan to non-oilpan code).

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

[ikilpatrick, 2017-02-03 04:43:51]

Description was changed from

==========
[LayoutNG] Convert physical fragments to being RefCounted.

A few changes in this patch:
NGPhysicalFragment becomes RefCounted.
NGFragment "views" become stack allocated.
  (also remove an unused param).
NGFragmentBuilder move off oil-pan to used being unique_ptr
  (this one was done because the "build" step would have required conversion
   from oilpan to non-oilpan code).

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

to

==========
[LayoutNG] Convert physical fragments to being RefCounted.

A few changes in this patch:
NGPhysicalFragment becomes RefCounted.
NGFragment "views" becomes stack allocated.
  (also remove an unused param).
NGFragmentBuilder moves off oil-pan to used being unique_ptr
  (this one was done because the "build" step would have required
   conversion from oilpan to non-oilpan code).

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

[ikilpatrick, 2017-02-03 05:00:00]

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

[commit-bot: I haz the power, 2017-02-03 06:47:44]

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

[commit-bot: I haz the power, 2017-02-03 06:47:45]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[ikilpatrick, 2017-02-03 08:52:37]

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

[commit-bot: I haz the power, 2017-02-03 08:52:47]

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, 2017-02-03 10:44:12]

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

[commit-bot: I haz the power, 2017-02-03 10:44:13]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[ikilpatrick, 2017-02-03 11:53:46]

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

[commit-bot: I haz the power, 2017-02-03 11:53:54]

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, 2017-02-03 13:33:19]

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

[commit-bot: I haz the power, 2017-02-03 13:33:20]

Dry run: This issue passed the CQ dry run.

[eae, 2017-02-03 18:50:38]

You're on fire this week!

LGTM

[cbiesinger, 2017-02-03 19:43:40]

lgtm

https://codereview.chromium.org/2676533003/diff/180001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/ng/ng_fragment.h (left):

https://codereview.chromium.org/2676533003/diff/180001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/ng/ng_fragment.h:21: TextDirection
Direction() const {
Don't you need the direction for the inline offset calculation? Or am I
misunderstanding something?

https://codereview.chromium.org/2676533003/diff/180001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/ng/ng_line_builder.cc (right):

https://codereview.chromium.org/2676533003/diff/180001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/ng/ng_line_builder.cc:131: for (unsigned i
= 0; i < fragments_.size(); i++)
Please add brackets now that the body has multiple lines

https://codereview.chromium.org/2676533003/diff/180001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/tests/NGInlineLayoutTest.cpp (right):

https://codereview.chromium.org/2676533003/diff/180001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/tests/NGInlineLayoutTest.cpp:89: // Delete the
line box tree to avoid leaks in the test.
seems unrelated?

[ikilpatrick, 2017-02-04 07:52:14]

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

[commit-bot: I haz the power, 2017-02-04 07:52:25]

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

[ikilpatrick, 2017-02-04 09:45:57]

https://codereview.chromium.org/2676533003/diff/180001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/ng/ng_fragment.h (left):

https://codereview.chromium.org/2676533003/diff/180001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/ng/ng_fragment.h:21: TextDirection
Direction() const {
On 2017/02/03 19:43:40, cbiesinger wrote:
> Don't you need the direction for the inline offset calculation? Or am I
> misunderstanding something?

No you're right, I've just removed this for now as we weren't using it.
Typically just set the offset for a fragment through the builder and never read
it (off this object) again, if that makes sense.

Can add back, but figured it'd be easy to do later if needed.

https://codereview.chromium.org/2676533003/diff/180001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/layout/ng/ng_line_builder.cc (right):

https://codereview.chromium.org/2676533003/diff/180001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/layout/ng/ng_line_builder.cc:131: for (unsigned i
= 0; i < fragments_.size(); i++)
On 2017/02/03 19:43:40, cbiesinger wrote:
> Please add brackets now that the body has multiple lines

Done.

https://codereview.chromium.org/2676533003/diff/180001/third_party/WebKit/Sou...
File third_party/WebKit/Source/web/tests/NGInlineLayoutTest.cpp (right):

https://codereview.chromium.org/2676533003/diff/180001/third_party/WebKit/Sou...
third_party/WebKit/Source/web/tests/NGInlineLayoutTest.cpp:89: // Delete the
line box tree to avoid leaks in the test.
On 2017/02/03 19:43:40, cbiesinger wrote:
> seems unrelated?

Yeah unfortunately not, it was a memory leak caused by:

Direct leak of 64 byte(s) in 1 object(s) allocated from:
    #0 0x63dd7c in __interceptor_malloc
(/usr/local/google/home/ikilpatrick/repo/chromium/src/out/lsan/webkit_unit_tests+0x63dd7c)
    #1 0xbc2c900 in PartitionAlloc
base/allocator/partition_allocator/partition_alloc.h:702:18
    #2 0xbc2c900 in blink::InlineBox::operator new(unsigned long)
third_party/WebKit/Source/core/layout/line/InlineBox.cpp:81
    #3 0xb8dfd96 in blink::LayoutBox::createInlineBox()
third_party/WebKit/Source/core/layout/LayoutBox.cpp:2028:10
    #4 0xb8905ba in createInlineBox
third_party/WebKit/Source/core/layout/api/LineLayoutBox.h:97:50
    #5 0xb8905ba in createInlineBoxForLayoutObject
third_party/WebKit/Source/core/layout/LayoutBlockFlowLine.cpp:134
    #6 0xb8905ba in
blink::LayoutBlockFlow::constructLine(blink::BidiRunList<blink::BidiRun>&,
blink::LineInfo const&)
third_party/WebKit/Source/core/layout/LayoutBlockFlowLine.cpp:296
    #7 0xbd00079 in blink::NGLineBuilder::CopyFragmentDataToLayoutBlockFlow()
third_party/WebKit/Source/core/layout/ng/ng_line_builder.cc:191:38
    #8 0xbce3f93 in blink::NGInlineLayoutAlgorithm::Layout()
third_party/WebKit/Source/core/layout/ng/ng_inline_layout_algorithm.cc:53:17
    #9 0x87213f in
blink::NGInlineLayoutTest_BlockWithTextAndAtomicInline_Test::TestBody()
third_party/WebKit/Source/web/tests/NGInlineLayoutTest.cpp:81:12

This should probably be a rendering test instead which handles the appropriate
document tearDown.

[commit-bot: I haz the power, 2017-02-04 09:53:10]

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

[commit-bot: I haz the power, 2017-02-04 09:53:11]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)
  linux_chromium_tsan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_TIMED_OUT,
no build URL)
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_TIMED_OUT, no build
URL)
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_TIMED_OUT, no
build URL)
  win_clang on master.tryserver.chromium.win (JOB_TIMED_OUT, no build URL)

[ikilpatrick, 2017-02-04 22:40:57]

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

[commit-bot: I haz the power, 2017-02-04 22:41:05]

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, 2017-02-05 01:24:34]

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

[commit-bot: I haz the power, 2017-02-05 01:24:36]

Dry run: This issue passed the CQ dry run.

[ikilpatrick, 2017-02-06 00:45:36]

The CQ bit was checked by ikilpatrick@chromium.org

[ikilpatrick, 2017-02-06 00:45:36]

The patchset sent to the CQ was uploaded after l-g-t-m from
cbiesinger@chromium.org, eae@chromium.org
Link to the patchset: https://codereview.chromium.org/2676533003/#ps200001
(title: "address comments.")

[commit-bot: I haz the power, 2017-02-06 00:45:50]

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

[ikilpatrick, 2017-02-06 00:46:23]

The CQ bit was unchecked by ikilpatrick@chromium.org

[ikilpatrick, 2017-02-06 00:46:51]

The CQ bit was checked by ikilpatrick@chromium.org

[commit-bot: I haz the power, 2017-02-06 00:47:05]

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, 2017-02-06 05:37:56]

CQ is committing da patch.

Bot data: {"patchset_id": 200001, "attempt_start_ts": 1486342011173080,
"parent_rev": "96e23868c7e9441a3d1d3b0784bfc32dc388ebea", "commit_rev":
"c4ef48dc4fb378c5d83ceb81b4417d74317596ae"}

[commit-bot: I haz the power, 2017-02-06 05:38:57]

Description was changed from

==========
[LayoutNG] Convert physical fragments to being RefCounted.

A few changes in this patch:
NGPhysicalFragment becomes RefCounted.
NGFragment "views" becomes stack allocated.
  (also remove an unused param).
NGFragmentBuilder moves off oil-pan to used being unique_ptr
  (this one was done because the "build" step would have required
   conversion from oilpan to non-oilpan code).

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

to

==========
[LayoutNG] Convert physical fragments to being RefCounted.

A few changes in this patch:
NGPhysicalFragment becomes RefCounted.
NGFragment "views" becomes stack allocated.
  (also remove an unused param).
NGFragmentBuilder moves off oil-pan to used being unique_ptr
  (this one was done because the "build" step would have required
   conversion from oilpan to non-oilpan code).

BUG=635619

Review-Url: https://codereview.chromium.org/2676533003
Cr-Commit-Position: refs/heads/master@{#448211}
Committed:
https://chromium.googlesource.com/chromium/src/+/c4ef48dc4fb378c5d83ceb81b441...
==========

[commit-bot: I haz the power, 2017-02-06 05:38:58]

Committed patchset #11 (id:200001) as
https://chromium.googlesource.com/chromium/src/+/c4ef48dc4fb378c5d83ceb81b441...