Rewrite how RemoteFrameOwners retain life.

Rewrite how RemoteFrameOwners retain life.

Before out-of-process iframes, frame owners (i.e. HTMLFrameOwnerElement)
were retained by the DOM tree of the parent frame. However, remote
frames don't have a DOM tree to keep the frame owners alive. Instead,
remote frames used a map to keep the frame owners live while the
corresponding child frame was still in the frame tree.

However, this map was never updated when frames were swapped, so frame
owners would trivially leak after a frame swap. To simplify the code,
just eliminate the map and have remote frame owners keep themselves
live. With Oilpan, there's actually no need to do anything special:
Frame already has a strong reference to the FrameOwner.

Without Oilpan, it is a bit more complicated: a remote frame owner
manually refs and unrefs itself when attached to/detached from a content
frame.

BUG=none

Committed: https://crrev.com/916fd306fb79285ec5535793bc72f334dd35595e
Cr-Commit-Position: refs/heads/master@{#381717}

[dcheng, 2016-03-16 21:07:21]

Description was changed from

==========
Paragraph comments111111111111111111111111111111111111111111111111111111


Moo


Compiles


Refcount things.


Moo

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

to

==========
Paragraph comments111111111111111111111111111111111111111111111111111111
Rewrite how RemoteFrameOwners retain life.


Moo


Compiles


Refcount things.


Moo

BUG=none
==========

[dcheng, 2016-03-16 21:07:21]

dcheng@chromium.org changed reviewers:
+ alexmos@chromium.org

[dcheng, 2016-03-16 21:08:10]

Description was changed from

==========
Paragraph comments111111111111111111111111111111111111111111111111111111
Rewrite how RemoteFrameOwners retain life.


Moo


Compiles


Refcount things.


Moo

BUG=none
==========

to

==========
Rewrite how RemoteFrameOwners retain life.

Paragraph comments111111111111111111111111111111111111111111111111111111
Moo


Compiles


Refcount things.


Moo

BUG=none
==========

[dcheng, 2016-03-16 21:09:09]

Description was changed from

==========
Rewrite how RemoteFrameOwners retain life.

Paragraph comments111111111111111111111111111111111111111111111111111111
Moo


Compiles


Refcount things.


Moo

BUG=none
==========

to

==========
Rewrite how RemoteFrameOwners retain life.

Paragraph comments111111111111111111111111111111111111111111111111111111
Pre out-of-process iframes, frame owners (i.e. HTMLFrameOwnerElement)
were retained by the DOM tree of the parent frame. However, remote
frames don't have a DOM tree to keep the frame owners alive, so instead,
remote frames used a map to keep the frame owners live while the
corresponding child frame was still in the frame tree.
Moo


Compiles


Refcount things.


Moo

BUG=none
==========

[dcheng, 2016-03-16 21:13:34]

Description was changed from

==========
Rewrite how RemoteFrameOwners retain life.

Paragraph comments111111111111111111111111111111111111111111111111111111
Pre out-of-process iframes, frame owners (i.e. HTMLFrameOwnerElement)
were retained by the DOM tree of the parent frame. However, remote
frames don't have a DOM tree to keep the frame owners alive, so instead,
remote frames used a map to keep the frame owners live while the
corresponding child frame was still in the frame tree.
Moo


Compiles


Refcount things.


Moo

BUG=none
==========

to

==========
Rewrite how RemoteFrameOwners retain life.

Before out-of-process iframes, frame owners (i.e. HTMLFrameOwnerElement)
were retained by the DOM tree of the parent frame. However, remote
frames don't have a DOM tree to keep the frame owners alive. Instead,
remote frames used a map to keep the frame owners live while the
corresponding child frame was still in the frame tree.

Paragraph comments111111111111111111111111111111111111111111111111111111
However, this map was never updated when frames were swapped, so frame
owners would trivially leak after a frame swap. To simplify the code,
just eliminate the map and have remote frame owners keep themselves
live. With Oilpan, there's actually no need to do anything special:
Frame already has a strong reference to the FrameOwner.

Without Oilpan, it is a bit more complicated: a remote frame owner
manually refs and unrefs itself when attached to/detached from a content
frame.

BUG=none
==========

[dcheng, 2016-03-16 21:13:48]

Description was changed from

==========
Rewrite how RemoteFrameOwners retain life.

Before out-of-process iframes, frame owners (i.e. HTMLFrameOwnerElement)
were retained by the DOM tree of the parent frame. However, remote
frames don't have a DOM tree to keep the frame owners alive. Instead,
remote frames used a map to keep the frame owners live while the
corresponding child frame was still in the frame tree.

Paragraph comments111111111111111111111111111111111111111111111111111111
However, this map was never updated when frames were swapped, so frame
owners would trivially leak after a frame swap. To simplify the code,
just eliminate the map and have remote frame owners keep themselves
live. With Oilpan, there's actually no need to do anything special:
Frame already has a strong reference to the FrameOwner.

Without Oilpan, it is a bit more complicated: a remote frame owner
manually refs and unrefs itself when attached to/detached from a content
frame.

BUG=none
==========

to

==========
Rewrite how RemoteFrameOwners retain life.

Before out-of-process iframes, frame owners (i.e. HTMLFrameOwnerElement)
were retained by the DOM tree of the parent frame. However, remote
frames don't have a DOM tree to keep the frame owners alive. Instead,
remote frames used a map to keep the frame owners live while the
corresponding child frame was still in the frame tree.

However, this map was never updated when frames were swapped, so frame
owners would trivially leak after a frame swap. To simplify the code,
just eliminate the map and have remote frame owners keep themselves
live. With Oilpan, there's actually no need to do anything special:
Frame already has a strong reference to the FrameOwner.

Without Oilpan, it is a bit more complicated: a remote frame owner
manually refs and unrefs itself when attached to/detached from a content
frame.

BUG=none
==========

[dcheng, 2016-03-16 21:15:04]

This is a bit ugly, but the simplest solution I could find. I believe it's
likely that non-Oilpan support will be removed completely soon, and we can
remove a lot of this complexity.

https://codereview.chromium.org/1807033003/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/frame/FrameOwner.h (right):

https://codereview.chromium.org/1807033003/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/frame/FrameOwner.h:26: virtual bool isRemote()
const = 0;
I went through and audited all existing calls (that I could find) to isLocal()
and flipped them to use isRemote() if appropriate.

[haraken, 2016-03-17 10:41:46]

On 2016/03/16 21:15:04, dcheng wrote:
> This is a bit ugly, but the simplest solution I could find. I believe it's
> likely that non-Oilpan support will be removed completely soon, and we can
> remove a lot of this complexity.
> 
>
https://codereview.chromium.org/1807033003/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/frame/FrameOwner.h (right):
> 
>
https://codereview.chromium.org/1807033003/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/frame/FrameOwner.h:26: virtual bool isRemote()
> const = 0;
> I went through and audited all existing calls (that I could find) to isLocal()
> and flipped them to use isRemote() if appropriate.

Makes sense. LGTM.

[dcheng, 2016-03-17 14:30:24]

The CQ bit was checked by dcheng@chromium.org

[commit-bot: I haz the power, 2016-03-17 14:30:40]

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

[commit-bot: I haz the power, 2016-03-17 15:39:53]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-03-17 15:41:00]

Description was changed from

==========
Rewrite how RemoteFrameOwners retain life.

Before out-of-process iframes, frame owners (i.e. HTMLFrameOwnerElement)
were retained by the DOM tree of the parent frame. However, remote
frames don't have a DOM tree to keep the frame owners alive. Instead,
remote frames used a map to keep the frame owners live while the
corresponding child frame was still in the frame tree.

However, this map was never updated when frames were swapped, so frame
owners would trivially leak after a frame swap. To simplify the code,
just eliminate the map and have remote frame owners keep themselves
live. With Oilpan, there's actually no need to do anything special:
Frame already has a strong reference to the FrameOwner.

Without Oilpan, it is a bit more complicated: a remote frame owner
manually refs and unrefs itself when attached to/detached from a content
frame.

BUG=none
==========

to

==========
Rewrite how RemoteFrameOwners retain life.

Before out-of-process iframes, frame owners (i.e. HTMLFrameOwnerElement)
were retained by the DOM tree of the parent frame. However, remote
frames don't have a DOM tree to keep the frame owners alive. Instead,
remote frames used a map to keep the frame owners live while the
corresponding child frame was still in the frame tree.

However, this map was never updated when frames were swapped, so frame
owners would trivially leak after a frame swap. To simplify the code,
just eliminate the map and have remote frame owners keep themselves
live. With Oilpan, there's actually no need to do anything special:
Frame already has a strong reference to the FrameOwner.

Without Oilpan, it is a bit more complicated: a remote frame owner
manually refs and unrefs itself when attached to/detached from a content
frame.

BUG=none

Committed: https://crrev.com/916fd306fb79285ec5535793bc72f334dd35595e
Cr-Commit-Position: refs/heads/master@{#381717}
==========

[commit-bot: I haz the power, 2016-03-17 15:41:03]

Patchset 1 (id:??) landed as
https://crrev.com/916fd306fb79285ec5535793bc72f334dd35595e
Cr-Commit-Position: refs/heads/master@{#381717}

[alexmos, 2016-03-17 16:32:29]

LGTM, thanks for cleaning this up!