Add virtual destructors to IntersectionObserverCallback

Add virtual destructors to IntersectionObserverCallback

BUG=577478, 576797, 577028
R=haraken@chromium.org,dcheng@chromium.org

Committed: https://crrev.com/9993cddc7c8fc611fc561cfca45494e8ebd18be7
Cr-Commit-Position: refs/heads/master@{#369375}

[haraken, 2016-01-14 07:05:37]

LGTM

[dcheng, 2016-01-14 07:06:10]

https://codereview.chromium.org/1583173002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/dom/IntersectionObservation.h (right):

https://codereview.chromium.org/1583173002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/IntersectionObservation.h:20: virtual
~IntersectionObservation();
No virtual needed here and on IntersectionObserver: if you're worried someone
will subclass it, you can mark these two classes final instead.

[sof, 2016-01-14 07:13:08]

sigbjornf@opera.com changed reviewers:
+ sigbjornf@opera.com

[sof, 2016-01-14 07:13:09]

https://codereview.chromium.org/1583173002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/dom/IntersectionObservation.h (right):

https://codereview.chromium.org/1583173002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/IntersectionObservation.h:20: virtual
~IntersectionObservation();
On 2016/01/14 07:06:09, dcheng wrote:
> No virtual needed here and on IntersectionObserver: if you're worried someone
> will subclass it, you can mark these two classes final instead.

Adding a TODO above this class decl, like

 TODO(Oilpan): switch to GarbageCollected<> when removing transition types.

would be worth doing, so that we don't forget.

[haraken, 2016-01-14 07:23:45]

https://codereview.chromium.org/1583173002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/dom/IntersectionObservation.h (right):

https://codereview.chromium.org/1583173002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/IntersectionObservation.h:20: virtual
~IntersectionObservation();
On 2016/01/14 07:13:09, sof wrote:
> On 2016/01/14 07:06:09, dcheng wrote:
> > No virtual needed here and on IntersectionObserver: if you're worried
someone
> > will subclass it, you can mark these two classes final instead.
> 
> Adding a TODO above this class decl, like
> 
>  TODO(Oilpan): switch to GarbageCollected<> when removing transition types.
> 
> would be worth doing, so that we don't forget.

Yeah, makes sense. We considered to introduce
GarbageCollectedFinalizedWillBeGarbageCollected before but rejected the idea
because it's a bit too crazy :)

[szager1, 2016-01-14 07:24:18]

Description was changed from

==========
Add virtual destructors to garbage-collected classes.

BUG=577478,576797,577028
R=haraken@chromium.org,dcheng@chromium.org
==========

to

==========
Add virtual destructors to IntersectionObserverCallback

BUG=577478,576797,577028
R=haraken@chromium.org,dcheng@chromium.org
==========

[szager1, 2016-01-14 07:24:37]

https://codereview.chromium.org/1583173002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/dom/IntersectionObservation.h (right):

https://codereview.chromium.org/1583173002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/IntersectionObservation.h:20: virtual
~IntersectionObservation();
On 2016/01/14 07:06:09, dcheng wrote:
> No virtual needed here and on IntersectionObserver: if you're worried someone
> will subclass it, you can mark these two classes final instead.

Done.

https://codereview.chromium.org/1583173002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/dom/IntersectionObservation.h:20: virtual
~IntersectionObservation();
On 2016/01/14 07:13:09, sof wrote:
> On 2016/01/14 07:06:09, dcheng wrote:
> > No virtual needed here and on IntersectionObserver: if you're worried
someone
> > will subclass it, you can mark these two classes final instead.
> 
> Adding a TODO above this class decl, like
> 
>  TODO(Oilpan): switch to GarbageCollected<> when removing transition types.
> 
> would be worth doing, so that we don't forget.

Done.

[szager1, 2016-01-14 07:25:48]

The CQ bit was checked by szager@chromium.org

[szager1, 2016-01-14 07:25:48]

The patchset sent to the CQ was uploaded after l-g-t-m from haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/1583173002/#ps20001
(title: "Remove unnecessary destructors")

[commit-bot: I haz the power, 2016-01-14 07:25:54]

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

[commit-bot: I haz the power, 2016-01-14 09:08:39]

Description was changed from

==========
Add virtual destructors to IntersectionObserverCallback

BUG=577478,576797,577028
R=haraken@chromium.org,dcheng@chromium.org
==========

to

==========
Add virtual destructors to IntersectionObserverCallback

BUG=577478,576797,577028
R=haraken@chromium.org,dcheng@chromium.org
==========

[commit-bot: I haz the power, 2016-01-14 09:08:42]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-01-14 09:10:17]

Description was changed from

==========
Add virtual destructors to IntersectionObserverCallback

BUG=577478,576797,577028
R=haraken@chromium.org,dcheng@chromium.org
==========

to

==========
Add virtual destructors to IntersectionObserverCallback

BUG=577478,576797,577028
R=haraken@chromium.org,dcheng@chromium.org

Committed: https://crrev.com/9993cddc7c8fc611fc561cfca45494e8ebd18be7
Cr-Commit-Position: refs/heads/master@{#369375}
==========

[commit-bot: I haz the power, 2016-01-14 09:10:18]

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

[Nico, 2016-01-14 20:39:06]

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

[Nico, 2016-01-14 20:39:07]

(ta-da: https://llvm.org/bugs/show_bug.cgi?id=26137#c5)