Fix leak PaintLayerReflectionInfo

Fix leak PaintLayerReflectionInfo

Previously PaintLayerReflectionInfo required destroy() to be called
before destruction, but we missed the call in
PaintLayer::updateReflectionInfo(), causing leak of the reflection
layout object and the layer for the reflection.

Delete PaintLayerReflectionInfo::destroy(). Use destructor instead.

BUG=582717
TEST=PaintLayerTest.ReflectionLeak

Committed: https://crrev.com/8718e5b803192df3cd4cec8907330443104063ee
Cr-Commit-Position: refs/heads/master@{#372985}

[Xianzhu, 2016-02-01 20:55:27]

wangxianzhu@chromium.org changed reviewers:
+ pdr@chromium.org

[pdr., 2016-02-01 21:07:44]

I think this is correct and wasn't able to find a bug in doing this, but this
kind of change can have tough destructor-order side effects.

LGTM, lets give it a shot.

[Xianzhu, 2016-02-01 21:46:33]

The CQ bit was checked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2016-02-01 21:48:37]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1657883002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1657883002/1

[commit-bot: I haz the power, 2016-02-01 22:43:02]

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

[commit-bot: I haz the power, 2016-02-01 22:43:03]

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

[Xianzhu, 2016-02-02 01:05:19]

The CQ bit was checked by wangxianzhu@chromium.org

[Xianzhu, 2016-02-02 01:05:20]

The patchset sent to the CQ was uploaded after l-g-t-m from pdr@chromium.org
Link to the patchset: https://codereview.chromium.org/1657883002/#ps20001
(title: "Update test to avoid leak of ImageResource")

[Xianzhu, 2016-02-02 01:06:42]

The CQ bit was checked by wangxianzhu@chromium.org

[Xianzhu, 2016-02-02 01:06:42]

The patchset sent to the CQ was uploaded after l-g-t-m from pdr@chromium.org
Link to the patchset: https://codereview.chromium.org/1657883002/#ps40001
(title: " ")

[commit-bot: I haz the power, 2016-02-02 01:07:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1657883002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1657883002/20001

[commit-bot: I haz the power, 2016-02-02 01:13:38]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_chromium_gn_compile_dbg on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_chro...)
  android_chromium_gn_compile_rel on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_chro...)
  android_clang_dbg_recipe on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  cast_shell_android on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win8_chromium_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...)
  win_chromium_compile_dbg_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2016-02-02 01:17:58]

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

[commit-bot: I haz the power, 2016-02-02 01:17:59]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_chromium_gn_compile_dbg on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_chro...)
  android_chromium_gn_compile_rel on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_chro...)
  android_clang_dbg_recipe on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  cast_shell_android on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
  win8_chromium_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...)
  win_chromium_compile_dbg_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Xianzhu, 2016-02-02 16:43:34]

The CQ bit was checked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2016-02-02 16:44:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1657883002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1657883002/40001

[commit-bot: I haz the power, 2016-02-02 18:04:15]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-02-02 18:05:05]

Description was changed from

==========
Fix leak PaintLayerReflectionInfo

Previously PaintLayerReflectionInfo required destroy() to be called
before destruction, but we missed the call in
PaintLayer::updateReflectionInfo(), causing leak of the reflection
layout object and the layer for the reflection.

Delete PaintLayerReflectionInfo::destroy(). Use destructor instead.

BUG=582717
TEST=PaintLayerTest.ReflectionLeak
==========

to

==========
Fix leak PaintLayerReflectionInfo

Previously PaintLayerReflectionInfo required destroy() to be called
before destruction, but we missed the call in
PaintLayer::updateReflectionInfo(), causing leak of the reflection
layout object and the layer for the reflection.

Delete PaintLayerReflectionInfo::destroy(). Use destructor instead.

BUG=582717
TEST=PaintLayerTest.ReflectionLeak

Committed: https://crrev.com/8718e5b803192df3cd4cec8907330443104063ee
Cr-Commit-Position: refs/heads/master@{#372985}
==========

[commit-bot: I haz the power, 2016-02-02 18:05:06]

Patchset 3 (id:??) landed as
https://crrev.com/8718e5b803192df3cd4cec8907330443104063ee
Cr-Commit-Position: refs/heads/master@{#372985}

[Reid Kleckner, 2016-02-04 00:33:00]

On 2016/02/02 18:05:06, commit-bot: I haz the power wrote:
> Patchset 3 (id:??) landed as
> https://crrev.com/8718e5b803192df3cd4cec8907330443104063ee
> Cr-Commit-Position: refs/heads/master@{#372985}

This broke webkit_unit_tests on WinClang:
https://build.chromium.org/p/chromium.fyi/builders/CrWinClang%20tester/builds...

[Reid Kleckner, 2016-02-04 00:33:22]

Description was changed from

==========
Fix leak PaintLayerReflectionInfo

Previously PaintLayerReflectionInfo required destroy() to be called
before destruction, but we missed the call in
PaintLayer::updateReflectionInfo(), causing leak of the reflection
layout object and the layer for the reflection.

Delete PaintLayerReflectionInfo::destroy(). Use destructor instead.

BUG=582717
TEST=PaintLayerTest.ReflectionLeak

Committed: https://crrev.com/8718e5b803192df3cd4cec8907330443104063ee
Cr-Commit-Position: refs/heads/master@{#372985}
==========

to

==========
Fix leak PaintLayerReflectionInfo

Previously PaintLayerReflectionInfo required destroy() to be called
before destruction, but we missed the call in
PaintLayer::updateReflectionInfo(), causing leak of the reflection
layout object and the layer for the reflection.

Delete PaintLayerReflectionInfo::destroy(). Use destructor instead.

BUG=582717
TEST=PaintLayerTest.ReflectionLeak

Committed: https://crrev.com/8718e5b803192df3cd4cec8907330443104063ee
Cr-Commit-Position: refs/heads/master@{#372985}
==========

[pdr., 2016-02-04 02:34:04]

On 2016/02/04 at 00:33:00, rnk wrote:
> On 2016/02/02 18:05:06, commit-bot: I haz the power wrote:
> > Patchset 3 (id:??) landed as
> > https://crrev.com/8718e5b803192df3cd4cec8907330443104063ee
> > Cr-Commit-Position: refs/heads/master@{#372985}
> 
> This broke webkit_unit_tests on WinClang:
>
https://build.chromium.org/p/chromium.fyi/builders/CrWinClang%20tester/builds...

Lets roll this out.

WangXianzhu is on vacation so I'll go ahead and do the rollout.

[pdr., 2016-02-04 02:35:12]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/1661233003/ by pdr@chromium.org.

The reason for reverting is: Caused crashes on CrWinClang:
https://build.chromium.org/p/chromium.fyi/builders/CrWinClang%20tester/builds....

[Stephen Chennney, 2016-02-05 19:51:32]

On 2016/02/04 02:35:12, pdr wrote:
> A revert of this CL (patchset #3 id:40001) has been created in
> https://codereview.chromium.org/1661233003/ by mailto:pdr@chromium.org.
> 
> The reason for reverting is: Caused crashes on CrWinClang:
>
https://build.chromium.org/p/chromium.fyi/builders/CrWinClang%20tester/builds....

I don't recall: do we create new codereview issues for relands of rolled-out
patches?

[chrishtr, 2016-02-05 21:30:32]

On 2016/02/05 at 19:51:32, schenney wrote:
> On 2016/02/04 02:35:12, pdr wrote:
> > A revert of this CL (patchset #3 id:40001) has been created in
> > https://codereview.chromium.org/1661233003/ by mailto:pdr@chromium.org.
> > 
> > The reason for reverting is: Caused crashes on CrWinClang:
> >
https://build.chromium.org/p/chromium.fyi/builders/CrWinClang%20tester/builds....
> 
> I don't recall: do we create new codereview issues for relands of rolled-out
patches?

I think it's better practice to do so, to make things less confusing.