Move PreFinalizer to WebPluginContainerImpl.

Move PreFinalizer to WebPluginContainerImpl.

Based on feedback in https://codereview.chromium.org/2886113002/ this moves the
finalizer to the concrete class WebPluginContainerImpl.

I tested using  editing/selection/selection-plugin-clear-crash.html
--enable-leak-detection that failed previously when finalizer was not being
called, and it passed.

BUG=712963
Review-Url: https://codereview.chromium.org/2901453002
Cr-Commit-Position: refs/heads/master@{#473522}
Committed: https://chromium.googlesource.com/chromium/src/+/c9f9768b2d76c8f09ffb715f5ba6eaa67ee243a7

[slangley, 2017-05-22 04:50:15]

Description was changed from

==========
Move PreFinalizer to WebPluginContainerImpl.

I tested using  editing/selection/selection-plugin-clear-crash.html
--enable-leak-detection that failed previously when finalizer was not being
called, and it passed.

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

to

==========
Move PreFinalizer to WebPluginContainerImpl.

Based on feedback in https://codereview.chromium.org/2886113002/ this moves the
finalizer to the concrete class WebPluginContainerImpl.

I tested using  editing/selection/selection-plugin-clear-crash.html
--enable-leak-detection that failed previously when finalizer was not being
called, and it passed.

BUG=712963
==========

[slangley, 2017-05-22 04:50:15]

slangley@chromium.org changed reviewers:
+ haraken@chromium.org

[haraken, 2017-05-22 04:54:56]

LGTM

https://codereview.chromium.org/2901453002/diff/1/third_party/WebKit/Source/w...
File third_party/WebKit/Source/web/WebPluginContainerImpl.h (right):

https://codereview.chromium.org/2901453002/diff/1/third_party/WebKit/Source/w...
third_party/WebKit/Source/web/WebPluginContainerImpl.h:180: void PreFinalize() {
Dispose(); }

Would it be clearer if:

- Introduce DisposePlugin() as a non-virtual method.
- Make both PreFinalize() and Dispose() call DisposePlugin().

?

[slangley, 2017-05-22 05:36:17]

On 2017/05/22 at 04:54:56, haraken wrote:
> LGTM
> 
>
https://codereview.chromium.org/2901453002/diff/1/third_party/WebKit/Source/w...
> File third_party/WebKit/Source/web/WebPluginContainerImpl.h (right):
> 
>
https://codereview.chromium.org/2901453002/diff/1/third_party/WebKit/Source/w...
> third_party/WebKit/Source/web/WebPluginContainerImpl.h:180: void PreFinalize()
{ Dispose(); }
> 
> Would it be clearer if:
> 
> - Introduce DisposePlugin() as a non-virtual method.
> - Make both PreFinalize() and Dispose() call DisposePlugin().
> 
> ?

I don't think it adds additional clarity over this, I figure the public API
should be calling Dispose() and having another way to achieve the same thing
would be confusing.

[haraken, 2017-05-22 05:37:55]

On 2017/05/22 05:36:17, slangley wrote:
> On 2017/05/22 at 04:54:56, haraken wrote:
> > LGTM
> > 
> >
>
https://codereview.chromium.org/2901453002/diff/1/third_party/WebKit/Source/w...
> > File third_party/WebKit/Source/web/WebPluginContainerImpl.h (right):
> > 
> >
>
https://codereview.chromium.org/2901453002/diff/1/third_party/WebKit/Source/w...
> > third_party/WebKit/Source/web/WebPluginContainerImpl.h:180: void
PreFinalize()
> { Dispose(); }
> > 
> > Would it be clearer if:
> > 
> > - Introduce DisposePlugin() as a non-virtual method.
> > - Make both PreFinalize() and Dispose() call DisposePlugin().
> > 
> > ?
> 
> I don't think it adds additional clarity over this, I figure the public API
> should be calling Dispose() and having another way to achieve the same thing
> would be confusing.

ok :)

[slangley, 2017-05-22 05:41:39]

The CQ bit was checked by slangley@chromium.org

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

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-05-22 07:47:02]

CQ is committing da patch.

Bot data: {"patchset_id": 1, "attempt_start_ts": 1495431699267910, "parent_rev":
"00bd27b320be81e0cb51d5c9a017d97449c5a5fa", "commit_rev":
"c9f9768b2d76c8f09ffb715f5ba6eaa67ee243a7"}

[commit-bot: I haz the power, 2017-05-22 07:47:12]

Description was changed from

==========
Move PreFinalizer to WebPluginContainerImpl.

Based on feedback in https://codereview.chromium.org/2886113002/ this moves the
finalizer to the concrete class WebPluginContainerImpl.

I tested using  editing/selection/selection-plugin-clear-crash.html
--enable-leak-detection that failed previously when finalizer was not being
called, and it passed.

BUG=712963
==========

to

==========
Move PreFinalizer to WebPluginContainerImpl.

Based on feedback in https://codereview.chromium.org/2886113002/ this moves the
finalizer to the concrete class WebPluginContainerImpl.

I tested using  editing/selection/selection-plugin-clear-crash.html
--enable-leak-detection that failed previously when finalizer was not being
called, and it passed.

BUG=712963

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

[commit-bot: I haz the power, 2017-05-22 07:47:13]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/c9f9768b2d76c8f09ffb715f5ba6...