aura: Fix segfault when launching aura_demo on Linux.

aura: Fix segfault when launching aura_demo on Linux.

BUG=117255
R=ben@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=126202

[Ben Goodger (Google), 2012-03-11 23:48:37]

why is p NULL?

On Sun, Mar 11, 2012 at 11:12 AM, <tfarina@chromium.org> wrote:

> Reviewers: Ben Goodger (Google),
>
> Description:
> aura: Fix segfault when launching aura_demo on Linux.
>
> BUG=117255
> R=ben@chromium.org
>
>
> Please review this at
http://codereview.chromium.**org/9688003/<http://codereview.chromium.org/9688...
>
> SVN Base:
svn://svn.chromium.org/chrome/**trunk/src<http://svn.chromium.org/chrome/trunk/src>
>
> Affected files:
>  M ui/aura/window.cc
>
>
> Index: ui/aura/window.cc
> diff --git a/ui/aura/window.cc b/ui/aura/window.cc
> index 71fedda1c5c0367c6816fba0db2dcc**91494e74f4..**
> 1d30d572ba6c0d229a2f564b84e19d**b1e0390985 100644
> --- a/ui/aura/window.cc
> +++ b/ui/aura/window.cc
> @@ -245,7 +245,9 @@ void Window::SetExternalTexture(ui:**:Texture*
> texture) {
>  }
>
>  void Window::SetParent(Window* parent) {
> -  GetParentForWindow(this, parent)->AddChild(this);
> +  Window* p = GetParentForWindow(this, parent);
> +  if (p)
> +   p->AddChild(this);
>  }
>
>  void Window::StackChildAtTop(**Window* child) {
>
>
>

[tfarina, 2012-03-11 23:53:02]

On Sun, Mar 11, 2012 at 8:48 PM, Ben Goodger (Google) <ben@chromium.org> wrote:
> why is p NULL?
>
Because:

- |parent| is NULL
- and GetStackingClient() is NULL
- and thus GetParentForWindow() returns NULL.

So it seems we are missing a call to SetStackingClient() somewhere.

>
> On Sun, Mar 11, 2012 at 11:12 AM, <tfarina@chromium.org> wrote:
>>
>> Reviewers: Ben Goodger (Google),
>>
>> Description:
>> aura: Fix segfault when launching aura_demo on Linux.
>>
>> BUG=117255
>> R=ben@chromium.org
>>
>>
>> Please review this at http://codereview.chromium.org/9688003/
>>
>> SVN Base: svn://svn.chromium.org/chrome/trunk/src
>>
>> Affected files:
>>  M ui/aura/window.cc
>>
>>
>> Index: ui/aura/window.cc
>> diff --git a/ui/aura/window.cc b/ui/aura/window.cc
>> index
>>
71fedda1c5c0367c6816fba0db2dcc91494e74f4..1d30d572ba6c0d229a2f564b84e19db1e0390985
>> 100644
>> --- a/ui/aura/window.cc
>> +++ b/ui/aura/window.cc
>> @@ -245,7 +245,9 @@ void Window::SetExternalTexture(ui::Texture* texture)
>> {
>>  }
>>
>>  void Window::SetParent(Window* parent) {
>> -  GetParentForWindow(this, parent)->AddChild(this);
>> +  Window* p = GetParentForWindow(this, parent);
>> +  if (p)
>> +   p->AddChild(this);
>>  }
>>
>>  void Window::StackChildAtTop(Window* child) {
>>
>>
>



-- 
Thiago

[Ben Goodger (Google), 2012-03-11 23:54:07]

can we do that in the demo instead?

-Ben

On Sun, Mar 11, 2012 at 4:52 PM, Thiago Farina <tfarina@chromium.org> wrote:

> On Sun, Mar 11, 2012 at 8:48 PM, Ben Goodger (Google) <ben@chromium.org>
> wrote:
> > why is p NULL?
> >
> Because:
>
> - |parent| is NULL
> - and GetStackingClient() is NULL
> - and thus GetParentForWindow() returns NULL.
>
> So it seems we are missing a call to SetStackingClient() somewhere.
>
> >
> > On Sun, Mar 11, 2012 at 11:12 AM, <tfarina@chromium.org> wrote:
> >>
> >> Reviewers: Ben Goodger (Google),
> >>
> >> Description:
> >> aura: Fix segfault when launching aura_demo on Linux.
> >>
> >> BUG=117255
> >> R=ben@chromium.org
> >>
> >>
> >> Please review this at http://codereview.chromium.org/9688003/
> >>
> >> SVN Base: svn://svn.chromium.org/chrome/trunk/src
> >>
> >> Affected files:
> >>  M ui/aura/window.cc
> >>
> >>
> >> Index: ui/aura/window.cc
> >> diff --git a/ui/aura/window.cc b/ui/aura/window.cc
> >> index
> >>
>
71fedda1c5c0367c6816fba0db2dcc91494e74f4..1d30d572ba6c0d229a2f564b84e19db1e0390985
> >> 100644
> >> --- a/ui/aura/window.cc
> >> +++ b/ui/aura/window.cc
> >> @@ -245,7 +245,9 @@ void Window::SetExternalTexture(ui::Texture*
> texture)
> >> {
> >>  }
> >>
> >>  void Window::SetParent(Window* parent) {
> >> -  GetParentForWindow(this, parent)->AddChild(this);
> >> +  Window* p = GetParentForWindow(this, parent);
> >> +  if (p)
> >> +   p->AddChild(this);
> >>  }
> >>
> >>  void Window::StackChildAtTop(Window* child) {
> >>
> >>
> >
>
>
>
> --
> Thiago
>

[tfarina, 2012-03-12 00:08:02]

On 2012/03/11 23:54:07, Ben Goodger (Google) wrote:
> can we do that in the demo instead?
>
Sure. Done. Please, take another look.

[Ben Goodger (Google), 2012-03-12 16:35:33]

lgtm

http://codereview.chromium.org/9688003/diff/2004/ui/aura/demo/demo_main.cc
File ui/aura/demo/demo_main.cc (right):

http://codereview.chromium.org/9688003/diff/2004/ui/aura/demo/demo_main.cc#ne...
ui/aura/demo/demo_main.cc:75: class StackingClientImpl : public
aura::client::StackingClient {
DemoStackingClient (a la DemoWindowDelegate, above)

[tfarina, 2012-03-12 17:36:04]

http://codereview.chromium.org/9688003/diff/2004/ui/aura/demo/demo_main.cc
File ui/aura/demo/demo_main.cc (right):

http://codereview.chromium.org/9688003/diff/2004/ui/aura/demo/demo_main.cc#ne...
ui/aura/demo/demo_main.cc:75: class StackingClientImpl : public
aura::client::StackingClient {
On 2012/03/12 16:35:33, Ben Goodger (Google) wrote:
> DemoStackingClient (a la DemoWindowDelegate, above)

Done.