Update Ash CustomFrame to not directly access Shell

Update Ash CustomFrame to not directly access Shell

The ash::Shell is not available when runnig in Mash. Update the frame code to exit early in this environment.

Committed: https://crrev.com/151a30547ebed34badf6ebf14f60701e2123cf52
Cr-Commit-Position: refs/heads/master@{#397264}

[jonross, 2016-05-30 17:18:32]

jonross@chromium.org changed reviewers:
+ sadrul@chromium.org

[jonross, 2016-05-30 17:18:32]

Hey,

Small change that allows extensions/apps to boot within mash. However if would
be good to discuss a formal approach to replacing all of the services.

[sadrul, 2016-05-31 15:53:57]

sadrul@chromium.org changed reviewers:
+ sky@chromium.org

[sadrul, 2016-05-31 15:53:57]

+sky@ for his thoughts as well.

I think this fix is needed because it's the chrome code that's trying to use
this code from ash? In a mash world, this should ideally be done by the WM, and
chrome itself wouldn't need to do this (and instead, maybe some prop on a window
to notify the WM).

[sky, 2016-05-31 19:12:44]

It seems like we shouldn't be creating  CustomFrameViewAsh. What code is
triggering that? Might it be the apps code?

[jonross, 2016-05-31 19:59:01]

On 2016/05/31 19:12:44, sky wrote:
> It seems like we shouldn't be creating  CustomFrameViewAsh. What code is
> triggering that? Might it be the apps code?

In this particular case I had previously closed a cros session where the files
app was launched. I launched using mash, but the same user dir. So it attempts
to relaunch the app upon startup:

[FATAL:shell.cc(224)] Check failed: instance_. 
base::debug::StackTrace::StackTrace()
logging::LogMessage::~LogMessage()
ash::Shell::GetInstance()
ash::FrameCaptionButtonContainerView::ShouldSizeButtonBeVisible()
ash::FrameCaptionButtonContainerView::FrameCaptionButtonContainerView()
ash::CustomFrameViewAsh::HeaderView::HeaderView()
ash::CustomFrameViewAsh::CustomFrameViewAsh()
ChromeNativeAppWindowViewsAuraAsh::CreateNonClientFrameView()
views::Widget::CreateNonClientFrameView()
views::Widget::Init()
ChromeNativeAppWindowViews::InitializeDefaultWindow()
ChromeNativeAppWindowViews::InitializeWindow()
ChromeNativeAppWindowViewsAuraAsh::InitializeWindow()
native_app_window::NativeAppWindowViews::Init()
ChromeAppWindowClient::CreateNativeAppWindowImpl()
ChromeAppWindowClient::CreateNativeAppWindow()
extensions::AppWindow::Init()
extensions::AppWindowCreateFunction::RunAsync()
AsyncExtensionFunction::Run()
extensions::ExtensionFunctionDispatcher::DispatchWithCallbackInternal()
extensions::ExtensionFunctionDispatcher::Dispatch()
extensions::ExtensionWebContentsObserver::OnRequest()
[...]
extensions::ExtensionWebContentsObserver::OnMessageReceived()
extensions::ChromeExtensionWebContentsObserver::OnMessageReceived()
content::WebContentsImpl::OnMessageReceived()
content::WebContentsImpl::OnMessageReceived()
content::RenderFrameHostImpl::OnMessageReceived()
content::RenderProcessHostImpl::OnMessageReceived()
IPC::ChannelProxy::Context::OnDispatchMessage()
[....MessageLoop....]

[sky, 2016-05-31 21:02:13]

It's the extension/app code triggering the creation. It should look do
something different when running in mash as chrome does. Basically
something like:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...

  -Scott

On Tue, May 31, 2016 at 12:59 PM,  <jonross@chromium.org> wrote:
> On 2016/05/31 19:12:44, sky wrote:
>> It seems like we shouldn't be creating CustomFrameViewAsh. What code is
>> triggering that? Might it be the apps code?
>
> In this particular case I had previously closed a cros session where the
> files
> app was launched. I launched using mash, but the same user dir. So it
> attempts
> to relaunch the app upon startup:
>
> [FATAL:shell.cc(224)] Check failed: instance_.
> base::debug::StackTrace::StackTrace()
> logging::LogMessage::~LogMessage()
> ash::Shell::GetInstance()
> ash::FrameCaptionButtonContainerView::ShouldSizeButtonBeVisible()
> ash::FrameCaptionButtonContainerView::FrameCaptionButtonContainerView()
> ash::CustomFrameViewAsh::HeaderView::HeaderView()
> ash::CustomFrameViewAsh::CustomFrameViewAsh()
> ChromeNativeAppWindowViewsAuraAsh::CreateNonClientFrameView()
> views::Widget::CreateNonClientFrameView()
> views::Widget::Init()
> ChromeNativeAppWindowViews::InitializeDefaultWindow()
> ChromeNativeAppWindowViews::InitializeWindow()
> ChromeNativeAppWindowViewsAuraAsh::InitializeWindow()
> native_app_window::NativeAppWindowViews::Init()
> ChromeAppWindowClient::CreateNativeAppWindowImpl()
> ChromeAppWindowClient::CreateNativeAppWindow()
> extensions::AppWindow::Init()
> extensions::AppWindowCreateFunction::RunAsync()
> AsyncExtensionFunction::Run()
> extensions::ExtensionFunctionDispatcher::DispatchWithCallbackInternal()
> extensions::ExtensionFunctionDispatcher::Dispatch()
> extensions::ExtensionWebContentsObserver::OnRequest()
> [...]
> extensions::ExtensionWebContentsObserver::OnMessageReceived()
> extensions::ChromeExtensionWebContentsObserver::OnMessageReceived()
> content::WebContentsImpl::OnMessageReceived()
> content::WebContentsImpl::OnMessageReceived()
> content::RenderFrameHostImpl::OnMessageReceived()
> content::RenderProcessHostImpl::OnMessageReceived()
> IPC::ChannelProxy::Context::OnDispatchMessage()
> [....MessageLoop....]
>
>
> https://codereview.chromium.org/2023883002/

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[jonross, 2016-06-01 15:43:47]

On 2016/05/31 21:02:13, sky wrote:
> It's the extension/app code triggering the creation. It should look do
> something different when running in mash as chrome does. Basically
> something like:
>
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...
> 
>   -Scott
> 
> On Tue, May 31, 2016 at 12:59 PM,  <mailto:jonross@chromium.org> wrote:
> > On 2016/05/31 19:12:44, sky wrote:
> >> It seems like we shouldn't be creating CustomFrameViewAsh. What code is
> >> triggering that? Might it be the apps code?
> >
> > In this particular case I had previously closed a cros session where the
> > files
> > app was launched. I launched using mash, but the same user dir. So it
> > attempts
> > to relaunch the app upon startup:
> >
> > [FATAL:shell.cc(224)] Check failed: instance_.
> > base::debug::StackTrace::StackTrace()
> > logging::LogMessage::~LogMessage()
> > ash::Shell::GetInstance()
> > ash::FrameCaptionButtonContainerView::ShouldSizeButtonBeVisible()
> > ash::FrameCaptionButtonContainerView::FrameCaptionButtonContainerView()
> > ash::CustomFrameViewAsh::HeaderView::HeaderView()
> > ash::CustomFrameViewAsh::CustomFrameViewAsh()
> > ChromeNativeAppWindowViewsAuraAsh::CreateNonClientFrameView()
> > views::Widget::CreateNonClientFrameView()
> > views::Widget::Init()
> > ChromeNativeAppWindowViews::InitializeDefaultWindow()
> > ChromeNativeAppWindowViews::InitializeWindow()
> > ChromeNativeAppWindowViewsAuraAsh::InitializeWindow()
> > native_app_window::NativeAppWindowViews::Init()
> > ChromeAppWindowClient::CreateNativeAppWindowImpl()
> > ChromeAppWindowClient::CreateNativeAppWindow()
> > extensions::AppWindow::Init()
> > extensions::AppWindowCreateFunction::RunAsync()
> > AsyncExtensionFunction::Run()
> > extensions::ExtensionFunctionDispatcher::DispatchWithCallbackInternal()
> > extensions::ExtensionFunctionDispatcher::Dispatch()
> > extensions::ExtensionWebContentsObserver::OnRequest()
> > [...]
> > extensions::ExtensionWebContentsObserver::OnMessageReceived()
> > extensions::ChromeExtensionWebContentsObserver::OnMessageReceived()
> > content::WebContentsImpl::OnMessageReceived()
> > content::WebContentsImpl::OnMessageReceived()
> > content::RenderFrameHostImpl::OnMessageReceived()
> > content::RenderProcessHostImpl::OnMessageReceived()
> > IPC::ChannelProxy::Context::OnDispatchMessage()
> > [....MessageLoop....]
> >
> >
> > https://codereview.chromium.org/2023883002/
> 
> -- 
> You received this message because you are subscribed to the Google Groups
> "Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.
> 

I've switched the frame creation to use the defaults instead of the ash custom
one.

[sky, 2016-06-01 17:06:16]

https://codereview.chromium.org/2023883002/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc
(right):

https://codereview.chromium.org/2023883002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc:276: if
(chrome::IsRunningInMash()) {
nit: no {}

I'm not familiar with ChromeNativeAppWindowViews, does
ChromeNativeAppWindowViews set the non-client insets so that the wm
moves/resizes these windows?

[jonross, 2016-06-01 20:47:44]

https://codereview.chromium.org/2023883002/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc
(right):

https://codereview.chromium.org/2023883002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc:276: if
(chrome::IsRunningInMash()) {
On 2016/06/01 17:06:16, sky wrote:
> nit: no {}
> 
> I'm not familiar with ChromeNativeAppWindowViews, does
> ChromeNativeAppWindowViews set the non-client insets so that the wm
> moves/resizes these windows?

Yes it does. It also chooses between some default frames based on app
requirements.

In the case of the crashing files app, it is able to be resized by the mus ws

[sky, 2016-06-01 20:54:22]

Great, LGTM

[jonross, 2016-06-01 20:54:56]

The CQ bit was checked by jonross@chromium.org

[commit-bot: I haz the power, 2016-06-01 20:55:40]

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

[commit-bot: I haz the power, 2016-06-01 22:05:02]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-06-01 22:08:09]

Description was changed from

==========
Update Ash CustomFrame to not directly access Shell

The ash::Shell is not available when runnig in Mash. Update the frame code to
exit early in this environment.
==========

to

==========
Update Ash CustomFrame to not directly access Shell

The ash::Shell is not available when runnig in Mash. Update the frame code to
exit early in this environment.

Committed: https://crrev.com/151a30547ebed34badf6ebf14f60701e2123cf52
Cr-Commit-Position: refs/heads/master@{#397264}
==========

[commit-bot: I haz the power, 2016-06-01 22:08:10]

Patchset 3 (id:??) landed as
https://crrev.com/151a30547ebed34badf6ebf14f60701e2123cf52
Cr-Commit-Position: refs/heads/master@{#397264}