RenderWidgetHost::WasHidden is called twice on each tab switch on Aura.

RenderWidgetHost::WasHidden is called twice on each tab switch on Aura.


TabContentsContainer::ChangeWebContent was calling native_container_->DetachContents then eventually calls RWH->WasHidden(), so there is no need to call it explicitly a second time.


BUG=110645
TEST=Manual

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=128248

[mmocny, 2012-01-17 18:27:29]

John, you wrote this code so I defer to you to know if I am correct to fix it
this way, but I do not think we should call WasHidden() twice.

Otherwise, perhaps RWH::WasHidden should check its is_hidden_ state and exit
early?

[jam, 2012-01-17 19:32:48]

I didn't write this code, but i did change it from tab_contents to web_contents
:) can you go through the svn history further back to see who wrote it?

[mmocny, 2012-01-17 19:58:26]

On 2012/01/17 19:32:48, John Abd-El-Malek wrote:
> I didn't write this code, but i did change it from tab_contents to
web_contents
> :) can you go through the svn history further back to see who wrote it?

Sorry about that!  My git blame was a but too hasty.

[mmocny, 2012-01-17 20:01:48]

Oshima, you wrote this code so I defer to you to know if I am correct to fix it
this way, but I do not think we should call WasHidden() twice.

Otherwise, perhaps RWH::WasHidden should check its is_hidden_ state and exit
early?

[oshima, 2012-01-17 20:53:54]

On 2012/01/17 20:01:48, mmocny wrote:
> Oshima, you wrote this code so I defer to you to know if I am correct to fix
it
> this way, but I do not think we should call WasHidden() twice.
> 
> Otherwise, perhaps RWH::WasHidden should check its is_hidden_ state and exit
> early?

"WasHidden" was in the original code I merged. If it calls WasHidden for, then
this may be ok, but you want to make sure this works for windows as well. Do you
have the stack when WasHidden is called through DetachContents?

[oshima, 2012-01-17 20:54:25]

On 2012/01/17 20:53:54, oshima wrote:
> On 2012/01/17 20:01:48, mmocny wrote:
> > Oshima, you wrote this code so I defer to you to know if I am correct to fix
> it
> > this way, but I do not think we should call WasHidden() twice.
> > 
> > Otherwise, perhaps RWH::WasHidden should check its is_hidden_ state and exit
> > early?
> 
> "WasHidden" was in the original code I merged. If it calls WasHidden for, then

I meant "for sure"

> this may be ok, but you want to make sure this works for windows as well. Do
you
> have the stack when WasHidden is called through DetachContents?

[mmocny, 2012-01-18 15:07:44]

I haven't tested on windows, I'll have to find an available machine.

Here are the stack traces:

TWO Calls to WasHidden on Aura, first one through DetachContents:

#0  RenderWidgetHost::WasHidden (this=0x2aaacb3d2400) at
../../content/browser/renderer_host/render_widget_host.cc:254
#1  0x00002aaaaef916cf in RenderWidgetHostViewAura::WasHidden
(this=0x2aaacf3e1800)
    at ../../content/browser/renderer_host/render_widget_host_view_aura.cc:178
#2  0x00002aaaaeffb8f4 in TabContents::WasHidden (this=0x2aaacc0c7040)
    at ../../content/browser/tab_contents/tab_contents.cc:610
#3  0x00002aaaaeffb9cd in TabContents::HideContents (this=0x2aaacc0c7040)
    at ../../content/browser/tab_contents/tab_contents.cc:632
#4  0x00002aaaab4d4567 in TabContentsViewViews::OnNativeWidgetVisibilityChanged
(this=0x2aaacb3c3600, visible=false)
    at ../../chrome/browser/ui/views/tab_contents/tab_contents_view_views.cc:469
#5  0x00002aaaae2142a4 in views::NativeWidgetAura::OnWindowVisibilityChanged
(this=0x2aaad027fd20, visible=false)
    at ../../ui/views/widget/native_widget_aura.cc:706
#6  0x00002aaab6fafc09 in aura::Window::SetVisible (this=0x2aaad2472c00,
visible=false) at ../../ui/aura/window.cc:484
#7  0x00002aaab6fadc79 in aura::Window::Hide (this=0x2aaad2472c00) at
../../ui/aura/window.cc:115
#8  0x00002aaaae1be414 in views::NativeViewHostAura::NativeViewDetaching
(this=0x2aaacb36c8a0, destroyed=false)
    at ../../ui/views/controls/native/native_view_host_aura.cc:37
#9  0x00002aaaae1bdfde in views::NativeViewHost::Detach (this=0x2aaacb3bd008,
destroyed=false)
    at ../../ui/views/controls/native/native_view_host.cc:183
#10 0x00002aaaae1bd975 in views::NativeViewHost::Detach (this=0x2aaacb3bd008)
    at ../../ui/views/controls/native/native_view_host.cc:51
#11 0x00002aaaabd0daa2 in NativeTabContentsContainerAura::DetachContents
(this=0x2aaacb3bd000, contents=0x2aaacc0c7040)
    at
../../chrome/browser/ui/views/tab_contents/native_tab_contents_container_aura.cc:48
#12 0x00002aaaab4d1c47 in TabContentsContainer::ChangeWebContents
(this=0x2aaacb38b780, contents=0x0)
    at ../../chrome/browser/ui/views/tab_contents/tab_contents_container.cc:39
#13 0x00002aaaab49ea09 in BrowserView::ProcessTabSelected (this=0x2aaacb3952c0,
new_contents=0x2aaacb3929a0)
    at ../../chrome/browser/ui/views/frame/browser_view.cc:2477
#14 0x00002aaaab49b74e in BrowserView::ActiveTabChanged (this=0x2aaacb3952c0,
old_contents=0x2aaacf84ab00, 
    new_contents=0x2aaacb3929a0, index=0, user_gesture=true)
(...)

#0  RenderWidgetHost::WasHidden (this=0x2aaacb3d2400) at
../../content/browser/renderer_host/render_widget_host.cc:254
#1  0x00002aaaaef916cf in RenderWidgetHostViewAura::WasHidden
(this=0x2aaacf3e1800)
    at ../../content/browser/renderer_host/render_widget_host_view_aura.cc:178
#2  0x00002aaaaeffb8f4 in TabContents::WasHidden (this=0x2aaacc0c7040)
    at ../../content/browser/tab_contents/tab_contents.cc:610
#3  0x00002aaaab4d1c6e in TabContentsContainer::ChangeWebContents
(this=0x2aaacb38b780, contents=0x0)
    at ../../chrome/browser/ui/views/tab_contents/tab_contents_container.cc:40
#4  0x00002aaaab49ea09 in BrowserView::ProcessTabSelected (this=0x2aaacb3952c0,
new_contents=0x2aaacb3929a0)
    at ../../chrome/browser/ui/views/frame/browser_view.cc:2477
#5  0x00002aaaab49b74e in BrowserView::ActiveTabChanged (this=0x2aaacb3952c0,
old_contents=0x2aaacf84ab00, 
    new_contents=0x2aaacb3929a0, index=0, user_gesture=true)
    at ../../chrome/browser/ui/views/frame/browser_view.cc:1399
(...)



ONE call on Linux desktop

#0  RenderWidgetHost::WasHidden (this=0x2aaac53f6400) at
../../content/browser/renderer_host/render_widget_host.cc:254
#1  0x00002aaaae89eed1 in RenderWidgetHostViewGtk::WasHidden
(this=0x2aaac58f0000)
    at ../../content/browser/renderer_host/render_widget_host_view_gtk.cc:689
#2  0x00002aaaae907324 in TabContents::WasHidden (this=0x2aaac2c4c680)
    at ../../content/browser/tab_contents/tab_contents.cc:610
#3  0x00002aaaab2a74b6 in TabContentsContainerGtk::HideTab (this=0x2aaac5583a20,
tab=0x2aaac2c1b160)
    at ../../chrome/browser/ui/gtk/tab_contents_container_gtk.cc:160
#4  0x00002aaaab2a7018 in TabContentsContainerGtk::SetTab (this=0x2aaac5583a20,
tab=0x2aaac65792c0)
    at ../../chrome/browser/ui/gtk/tab_contents_container_gtk.cc:68
#5  0x00002aaaab23d06a in BrowserWindowGtk::ActiveTabChanged
(this=0x2aaac2c5b180, old_contents=0x2aaac2c1b160, 
    new_contents=0x2aaac65792c0, index=1, user_gesture=false) at
../../chrome/browser/ui/gtk/browser_window_gtk.cc:1291
#6  0x00002aaaab1c2e74 in TabStripModel::NotifyIfActiveTabChanged
(this=0x2aaac5946a50, old_contents=0x2aaac2c1b160, 
    user_gesture=false) at ../../chrome/browser/tabs/tab_strip_model.cc:1236
#7  0x00002aaaab1c2f01 in TabStripModel::NotifyIfActiveOrSelectionChanged
(this=0x2aaac5946a50, 
    old_contents=0x2aaac2c1b160, user_gesture=false, old_model=...)
    at ../../chrome/browser/tabs/tab_strip_model.cc:1248
(...)

On 2012/01/17 20:54:25, oshima wrote:
> On 2012/01/17 20:53:54, oshima wrote:
> > On 2012/01/17 20:01:48, mmocny wrote:
> > > Oshima, you wrote this code so I defer to you to know if I am correct to
fix
> > it
> > > this way, but I do not think we should call WasHidden() twice.
> > > 
> > > Otherwise, perhaps RWH::WasHidden should check its is_hidden_ state and
exit
> > > early?
> > 
> > "WasHidden" was in the original code I merged. If it calls WasHidden for,
then
> 
> I meant "for sure"
> 
> > this may be ok, but you want to make sure this works for windows as well. Do
> you
> > have the stack when WasHidden is called through DetachContents?

[oshima, 2012-01-18 21:49:55]

Can you test it on existing chromeos as well? It uses views/gtk and should have
slightly different path as well.


On 2012/01/18 15:07:44, mmocny wrote:
> I haven't tested on windows, I'll have to find an available machine.
> 
> Here are the stack traces:
> 
> TWO Calls to WasHidden on Aura, first one through DetachContents:
> 
> #0  RenderWidgetHost::WasHidden (this=0x2aaacb3d2400) at
> ../../content/browser/renderer_host/render_widget_host.cc:254
> #1  0x00002aaaaef916cf in RenderWidgetHostViewAura::WasHidden
> (this=0x2aaacf3e1800)
>     at ../../content/browser/renderer_host/render_widget_host_view_aura.cc:178
> #2  0x00002aaaaeffb8f4 in TabContents::WasHidden (this=0x2aaacc0c7040)
>     at ../../content/browser/tab_contents/tab_contents.cc:610
> #3  0x00002aaaaeffb9cd in TabContents::HideContents (this=0x2aaacc0c7040)
>     at ../../content/browser/tab_contents/tab_contents.cc:632
> #4  0x00002aaaab4d4567 in
TabContentsViewViews::OnNativeWidgetVisibilityChanged
> (this=0x2aaacb3c3600, visible=false)
>     at
../../chrome/browser/ui/views/tab_contents/tab_contents_view_views.cc:469
> #5  0x00002aaaae2142a4 in views::NativeWidgetAura::OnWindowVisibilityChanged
> (this=0x2aaad027fd20, visible=false)
>     at ../../ui/views/widget/native_widget_aura.cc:706
> #6  0x00002aaab6fafc09 in aura::Window::SetVisible (this=0x2aaad2472c00,
> visible=false) at ../../ui/aura/window.cc:484
> #7  0x00002aaab6fadc79 in aura::Window::Hide (this=0x2aaad2472c00) at
> ../../ui/aura/window.cc:115
> #8  0x00002aaaae1be414 in views::NativeViewHostAura::NativeViewDetaching
> (this=0x2aaacb36c8a0, destroyed=false)
>     at ../../ui/views/controls/native/native_view_host_aura.cc:37
> #9  0x00002aaaae1bdfde in views::NativeViewHost::Detach (this=0x2aaacb3bd008,
> destroyed=false)
>     at ../../ui/views/controls/native/native_view_host.cc:183
> #10 0x00002aaaae1bd975 in views::NativeViewHost::Detach (this=0x2aaacb3bd008)
>     at ../../ui/views/controls/native/native_view_host.cc:51
> #11 0x00002aaaabd0daa2 in NativeTabContentsContainerAura::DetachContents
> (this=0x2aaacb3bd000, contents=0x2aaacc0c7040)
>     at
>
../../chrome/browser/ui/views/tab_contents/native_tab_contents_container_aura.cc:48
> #12 0x00002aaaab4d1c47 in TabContentsContainer::ChangeWebContents
> (this=0x2aaacb38b780, contents=0x0)
>     at ../../chrome/browser/ui/views/tab_contents/tab_contents_container.cc:39
> #13 0x00002aaaab49ea09 in BrowserView::ProcessTabSelected
(this=0x2aaacb3952c0,
> new_contents=0x2aaacb3929a0)
>     at ../../chrome/browser/ui/views/frame/browser_view.cc:2477
> #14 0x00002aaaab49b74e in BrowserView::ActiveTabChanged (this=0x2aaacb3952c0,
> old_contents=0x2aaacf84ab00, 
>     new_contents=0x2aaacb3929a0, index=0, user_gesture=true)
> (...)
> 
> #0  RenderWidgetHost::WasHidden (this=0x2aaacb3d2400) at
> ../../content/browser/renderer_host/render_widget_host.cc:254
> #1  0x00002aaaaef916cf in RenderWidgetHostViewAura::WasHidden
> (this=0x2aaacf3e1800)
>     at ../../content/browser/renderer_host/render_widget_host_view_aura.cc:178
> #2  0x00002aaaaeffb8f4 in TabContents::WasHidden (this=0x2aaacc0c7040)
>     at ../../content/browser/tab_contents/tab_contents.cc:610
> #3  0x00002aaaab4d1c6e in TabContentsContainer::ChangeWebContents
> (this=0x2aaacb38b780, contents=0x0)
>     at ../../chrome/browser/ui/views/tab_contents/tab_contents_container.cc:40
> #4  0x00002aaaab49ea09 in BrowserView::ProcessTabSelected
(this=0x2aaacb3952c0,
> new_contents=0x2aaacb3929a0)
>     at ../../chrome/browser/ui/views/frame/browser_view.cc:2477
> #5  0x00002aaaab49b74e in BrowserView::ActiveTabChanged (this=0x2aaacb3952c0,
> old_contents=0x2aaacf84ab00, 
>     new_contents=0x2aaacb3929a0, index=0, user_gesture=true)
>     at ../../chrome/browser/ui/views/frame/browser_view.cc:1399
> (...)
> 
> 
> 
> ONE call on Linux desktop
> 
> #0  RenderWidgetHost::WasHidden (this=0x2aaac53f6400) at
> ../../content/browser/renderer_host/render_widget_host.cc:254
> #1  0x00002aaaae89eed1 in RenderWidgetHostViewGtk::WasHidden
> (this=0x2aaac58f0000)
>     at ../../content/browser/renderer_host/render_widget_host_view_gtk.cc:689
> #2  0x00002aaaae907324 in TabContents::WasHidden (this=0x2aaac2c4c680)
>     at ../../content/browser/tab_contents/tab_contents.cc:610
> #3  0x00002aaaab2a74b6 in TabContentsContainerGtk::HideTab
(this=0x2aaac5583a20,
> tab=0x2aaac2c1b160)
>     at ../../chrome/browser/ui/gtk/tab_contents_container_gtk.cc:160
> #4  0x00002aaaab2a7018 in TabContentsContainerGtk::SetTab
(this=0x2aaac5583a20,
> tab=0x2aaac65792c0)
>     at ../../chrome/browser/ui/gtk/tab_contents_container_gtk.cc:68
> #5  0x00002aaaab23d06a in BrowserWindowGtk::ActiveTabChanged
> (this=0x2aaac2c5b180, old_contents=0x2aaac2c1b160, 
>     new_contents=0x2aaac65792c0, index=1, user_gesture=false) at
> ../../chrome/browser/ui/gtk/browser_window_gtk.cc:1291
> #6  0x00002aaaab1c2e74 in TabStripModel::NotifyIfActiveTabChanged
> (this=0x2aaac5946a50, old_contents=0x2aaac2c1b160, 
>     user_gesture=false) at ../../chrome/browser/tabs/tab_strip_model.cc:1236
> #7  0x00002aaaab1c2f01 in TabStripModel::NotifyIfActiveOrSelectionChanged
> (this=0x2aaac5946a50, 
>     old_contents=0x2aaac2c1b160, user_gesture=false, old_model=...)
>     at ../../chrome/browser/tabs/tab_strip_model.cc:1248
> (...)
> 
> On 2012/01/17 20:54:25, oshima wrote:
> > On 2012/01/17 20:53:54, oshima wrote:
> > > On 2012/01/17 20:01:48, mmocny wrote:
> > > > Oshima, you wrote this code so I defer to you to know if I am correct to
> fix
> > > it
> > > > this way, but I do not think we should call WasHidden() twice.
> > > > 
> > > > Otherwise, perhaps RWH::WasHidden should check its is_hidden_ state and
> exit
> > > > early?
> > > 
> > > "WasHidden" was in the original code I merged. If it calls WasHidden for,
> then
> > 
> > I meant "for sure"
> > 
> > > this may be ok, but you want to make sure this works for windows as well.
Do
> > you
> > > have the stack when WasHidden is called through DetachContents?

[mmocny1, 2012-01-18 21:53:14]

Sure, but it will have a wait a bit.  Should I file a bug to make sure that
this isn't lost?

On Wed, Jan 18, 2012 at 4:49 PM, <oshima@chromium.org> wrote:

> Can you test it on existing chromeos as well? It uses views/gtk and should
> have
> slightly different path as well.
>
>
>
> On 2012/01/18 15:07:44, mmocny wrote:
>
>> I haven't tested on windows, I'll have to find an available machine.
>>
>
>  Here are the stack traces:
>>
>
>  TWO Calls to WasHidden on Aura, first one through DetachContents:
>>
>
>  #0  RenderWidgetHost::WasHidden (this=0x2aaacb3d2400) at
>> ../../content/browser/**renderer_host/render_widget_**host.cc:254
>> #1  0x00002aaaaef916cf in RenderWidgetHostViewAura::**WasHidden
>> (this=0x2aaacf3e1800)
>>     at ../../content/browser/**renderer_host/render_widget_**
>> host_view_aura.cc:178
>> #2  0x00002aaaaeffb8f4 in TabContents::WasHidden (this=0x2aaacc0c7040)
>>     at ../../content/browser/tab_**contents/tab_contents.cc:610
>> #3  0x00002aaaaeffb9cd in TabContents::HideContents (this=0x2aaacc0c7040)
>>     at ../../content/browser/tab_**contents/tab_contents.cc:632
>> #4  0x00002aaaab4d4567 in
>>
> TabContentsViewViews::**OnNativeWidgetVisibilityChange**d
>
>> (this=0x2aaacb3c3600, visible=false)
>>     at
>>
> ../../chrome/browser/ui/views/**tab_contents/tab_contents_**
> view_views.cc:469
>
>> #5  0x00002aaaae2142a4 in views::NativeWidgetAura::**
>> OnWindowVisibilityChanged
>> (this=0x2aaad027fd20, visible=false)
>>     at ../../ui/views/widget/native_**widget_aura.cc:706
>> #6  0x00002aaab6fafc09 in aura::Window::SetVisible (this=0x2aaad2472c00,
>> visible=false) at ../../ui/aura/window.cc:484
>> #7  0x00002aaab6fadc79 in aura::Window::Hide (this=0x2aaad2472c00) at
>> ../../ui/aura/window.cc:115
>> #8  0x00002aaaae1be414 in views::NativeViewHostAura::**
>> NativeViewDetaching
>> (this=0x2aaacb36c8a0, destroyed=false)
>>     at ../../ui/views/controls/**native/native_view_host_aura.**cc:37
>> #9  0x00002aaaae1bdfde in views::NativeViewHost::Detach
>> (this=0x2aaacb3bd008,
>> destroyed=false)
>>     at ../../ui/views/controls/**native/native_view_host.cc:183
>> #10 0x00002aaaae1bd975 in views::NativeViewHost::Detach
>> (this=0x2aaacb3bd008)
>>     at ../../ui/views/controls/**native/native_view_host.cc:51
>> #11 0x00002aaaabd0daa2 in NativeTabContentsContainerAura**
>> ::DetachContents
>> (this=0x2aaacb3bd000, contents=0x2aaacc0c7040)
>>     at
>>
>
> ../../chrome/browser/ui/views/**tab_contents/native_tab_**
> contents_container_aura.cc:48
>
>> #12 0x00002aaaab4d1c47 in TabContentsContainer::**ChangeWebContents
>> (this=0x2aaacb38b780, contents=0x0)
>>     at ../../chrome/browser/ui/views/**tab_contents/tab_contents_**
>> container.cc:39
>> #13 0x00002aaaab49ea09 in BrowserView::**ProcessTabSelected
>>
> (this=0x2aaacb3952c0,
>
>> new_contents=0x2aaacb3929a0)
>>     at ../../chrome/browser/ui/views/**frame/browser_view.cc:2477
>> #14 0x00002aaaab49b74e in BrowserView::ActiveTabChanged
>> (this=0x2aaacb3952c0,
>> old_contents=0x2aaacf84ab00,
>>     new_contents=0x2aaacb3929a0, index=0, user_gesture=true)
>> (...)
>>
>
>  #0  RenderWidgetHost::WasHidden (this=0x2aaacb3d2400) at
>> ../../content/browser/**renderer_host/render_widget_**host.cc:254
>> #1  0x00002aaaaef916cf in RenderWidgetHostViewAura::**WasHidden
>> (this=0x2aaacf3e1800)
>>     at ../../content/browser/**renderer_host/render_widget_**
>> host_view_aura.cc:178
>> #2  0x00002aaaaeffb8f4 in TabContents::WasHidden (this=0x2aaacc0c7040)
>>     at ../../content/browser/tab_**contents/tab_contents.cc:610
>> #3  0x00002aaaab4d1c6e in TabContentsContainer::**ChangeWebContents
>> (this=0x2aaacb38b780, contents=0x0)
>>     at ../../chrome/browser/ui/views/**tab_contents/tab_contents_**
>> container.cc:40
>> #4  0x00002aaaab49ea09 in BrowserView::**ProcessTabSelected
>>
> (this=0x2aaacb3952c0,
>
>> new_contents=0x2aaacb3929a0)
>>     at ../../chrome/browser/ui/views/**frame/browser_view.cc:2477
>> #5  0x00002aaaab49b74e in BrowserView::ActiveTabChanged
>> (this=0x2aaacb3952c0,
>> old_contents=0x2aaacf84ab00,
>>     new_contents=0x2aaacb3929a0, index=0, user_gesture=true)
>>     at ../../chrome/browser/ui/views/**frame/browser_view.cc:1399
>> (...)
>>
>
>
>
>  ONE call on Linux desktop
>>
>
>  #0  RenderWidgetHost::WasHidden (this=0x2aaac53f6400) at
>> ../../content/browser/**renderer_host/render_widget_**host.cc:254
>> #1  0x00002aaaae89eed1 in RenderWidgetHostViewGtk::**WasHidden
>> (this=0x2aaac58f0000)
>>     at ../../content/browser/**renderer_host/render_widget_**
>> host_view_gtk.cc:689
>> #2  0x00002aaaae907324 in TabContents::WasHidden (this=0x2aaac2c4c680)
>>     at ../../content/browser/tab_**contents/tab_contents.cc:610
>> #3  0x00002aaaab2a74b6 in TabContentsContainerGtk::**HideTab
>>
> (this=0x2aaac5583a20,
>
>> tab=0x2aaac2c1b160)
>>     at ../../chrome/browser/ui/gtk/**tab_contents_container_gtk.cc:**160
>> #4  0x00002aaaab2a7018 in TabContentsContainerGtk::**SetTab
>>
> (this=0x2aaac5583a20,
>
>> tab=0x2aaac65792c0)
>>     at ../../chrome/browser/ui/gtk/**tab_contents_container_gtk.cc:**68
>> #5  0x00002aaaab23d06a in BrowserWindowGtk::**ActiveTabChanged
>> (this=0x2aaac2c5b180, old_contents=0x2aaac2c1b160,
>>     new_contents=0x2aaac65792c0, index=1, user_gesture=false) at
>> ../../chrome/browser/ui/gtk/**browser_window_gtk.cc:1291
>> #6  0x00002aaaab1c2e74 in TabStripModel::**NotifyIfActiveTabChanged
>> (this=0x2aaac5946a50, old_contents=0x2aaac2c1b160,
>>     user_gesture=false) at ../../chrome/browser/tabs/tab_**
>> strip_model.cc:1236
>> #7  0x00002aaaab1c2f01 in TabStripModel::**NotifyIfActiveOrSelectionChang
>> **ed
>> (this=0x2aaac5946a50,
>>     old_contents=0x2aaac2c1b160, user_gesture=false, old_model=...)
>>     at ../../chrome/browser/tabs/tab_**strip_model.cc:1248
>> (...)
>>
>
>  On 2012/01/17 20:54:25, oshima wrote:
>> > On 2012/01/17 20:53:54, oshima wrote:
>> > > On 2012/01/17 20:01:48, mmocny wrote:
>> > > > Oshima, you wrote this code so I defer to you to know if I am
>> correct to
>> fix
>> > > it
>> > > > this way, but I do not think we should call WasHidden() twice.
>> > > >
>> > > > Otherwise, perhaps RWH::WasHidden should check its is_hidden_ state
>> and
>> exit
>> > > > early?
>> > >
>> > > "WasHidden" was in the original code I merged. If it calls WasHidden
>> for,
>> then
>> >
>> > I meant "for sure"
>> >
>> > > this may be ok, but you want to make sure this works for windows as
>> well.
>>
> Do
>
>> > you
>> > > have the stack when WasHidden is called through DetachContents?
>>
>
>
>
>
http://codereview.chromium.**org/9243013/<http://codereview.chromium.org/9243...
>

[mazda, 2012-03-22 15:04:10]

I tested this on Chrome OS (Aura) and it looks it' working file.

[mmocny, 2012-03-22 15:09:10]

There was no hurry before, but seems this is causing grief for browser
thumbnailer (see last messages crbug.com/110645)

[Ben Goodger (Google), 2012-03-22 16:21:06]

lgtm

[commit-bot: I haz the power, 2012-03-22 16:26:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mmocny@chromium.org/9243013/1

[commit-bot: I haz the power, 2012-03-22 18:09:49]

Change committed as 128248