Sky: When an iframe element is removed, delete its mojo View.

Sky: When an iframe element is removed, delete its mojo View.

R=esprehn@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/45991a3752e6a6298901eba36e7bf4cdec4d263c

[Matt Perry, 2014-11-17 22:50:44]

mpcomplete@chromium.org changed reviewers:
+ esprehn@chromium.org

[esprehn, 2014-11-17 23:19:41]

https://codereview.chromium.org/733063007/diff/1/sky/engine/core/html/HTMLIFr...
File sky/engine/core/html/HTMLIFrameElement.cpp (right):

https://codereview.chromium.org/733063007/diff/1/sky/engine/core/html/HTMLIFr...
sky/engine/core/html/HTMLIFrameElement.cpp:72: m_contentView->AddObserver(this);
Why do we need to be an observer?

[Matt Perry, 2014-11-18 16:28:45]

https://codereview.chromium.org/733063007/diff/1/sky/engine/core/html/HTMLIFr...
File sky/engine/core/html/HTMLIFrameElement.cpp (right):

https://codereview.chromium.org/733063007/diff/1/sky/engine/core/html/HTMLIFr...
sky/engine/core/html/HTMLIFrameElement.cpp:72: m_contentView->AddObserver(this);
On 2014/11/17 23:19:41, esprehn wrote:
> Why do we need to be an observer?

I'm just guarding against the possibility that the mojo::View is deleted before
the HTMLIFrameElement. This lets us NULL it out so we don't have a dangling
pointer (which could be used by RenderIFrame).

[esprehn, 2014-11-18 17:53:42]

https://codereview.chromium.org/733063007/diff/1/sky/engine/core/html/HTMLIFr...
File sky/engine/core/html/HTMLIFrameElement.cpp (right):

https://codereview.chromium.org/733063007/diff/1/sky/engine/core/html/HTMLIFr...
sky/engine/core/html/HTMLIFrameElement.cpp:72: m_contentView->AddObserver(this);
On 2014/11/18 16:28:45, Matt Perry wrote:
> On 2014/11/17 23:19:41, esprehn wrote:
> > Why do we need to be an observer?
> 
> I'm just guarding against the possibility that the mojo::View is deleted
before
> the HTMLIFrameElement. This lets us NULL it out so we don't have a dangling
> pointer (which could be used by RenderIFrame).

I guess what I mean is, don't we need to implement some method to be an
observer? This change only has .cpp file changes and no new interfaces or
methods.

[Matt Perry, 2014-11-18 18:09:17]

https://codereview.chromium.org/733063007/diff/1/sky/engine/core/html/HTMLIFr...
File sky/engine/core/html/HTMLIFrameElement.cpp (right):

https://codereview.chromium.org/733063007/diff/1/sky/engine/core/html/HTMLIFr...
sky/engine/core/html/HTMLIFrameElement.cpp:72: m_contentView->AddObserver(this);
On 2014/11/18 17:53:42, esprehn wrote:
> On 2014/11/18 16:28:45, Matt Perry wrote:
> > On 2014/11/17 23:19:41, esprehn wrote:
> > > Why do we need to be an observer?
> > 
> > I'm just guarding against the possibility that the mojo::View is deleted
> before
> > the HTMLIFrameElement. This lets us NULL it out so we don't have a dangling
> > pointer (which could be used by RenderIFrame).
> 
> I guess what I mean is, don't we need to implement some method to be an
> observer? This change only has .cpp file changes and no new interfaces or
> methods.

Ah, yes. OnViewDestroyed, line 53. I had implemented it already but forgot to
actually register this as an observer.

[esprehn, 2014-11-18 18:59:06]

lgtm

[Matt Perry, 2014-11-18 19:00:10]

Committed patchset #1 (id:1) manually as
45991a3752e6a6298901eba36e7bf4cdec4d263c (presubmit successful).