Fix maximize on some window managers

Fix maximize on some window managers

On some window managers the Maximize hints are not respected when they
are set on an unmapped window. There was an existing workaround for this
in the code, but the workaround failed whenever ShowWindowWithState
didn't happen to be called with the parameters needed to activate the
workaround. This meant that if a window was first made visible and later
activated, it would not get maximized at all.

This fix saves whether the maximize hints need to be sent to the window,
and makes sure it gets done after mapping the window.

BUG=

Committed: https://crrev.com/79cd33a2ae10b9424fc7bde5b577ec732bfa4a04
Cr-Commit-Position: refs/heads/master@{#293698}

[Arjan van Leeuwen, 2014-09-04 09:28:17]

arjanl@opera.com changed reviewers:
+ erg@chromium.org

[Arjan van Leeuwen, 2014-09-04 09:29:03]

This is a fix for maximizing on some WMs that doesn't depend so much on the
order of calls (as with the previous workaround).

[Elliot Glaysher, 2014-09-04 17:37:04]

https://codereview.chromium.org/543663003/diff/1/ui/views/widget/desktop_aura...
File ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc (right):

https://codereview.chromium.org/543663003/diff/1/ui/views/widget/desktop_aura...
ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:564:
should_maximize_after_map_ = false;
This seems fragile.

Could you please test in the case of Maximizing through code before the window
is mapped, restoring through the window manager controls (not chrome's buttons),
and then doing something that would unmap and then remap the window?

And if that's broken, you'll also have to add more should_maximize_after_map_ =
false statements to the _net_wm_state property notify.

[Arjan van Leeuwen, 2014-09-05 11:23:26]

https://codereview.chromium.org/543663003/diff/1/ui/views/widget/desktop_aura...
File ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc (right):

https://codereview.chromium.org/543663003/diff/1/ui/views/widget/desktop_aura...
ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:564:
should_maximize_after_map_ = false;
On 2014/09/04 17:37:04, Elliot Glaysher wrote:
> This seems fragile.
> 
> Could you please test in the case of Maximizing through code before the window
> is mapped, restoring through the window manager controls (not chrome's
buttons),

You can't restore the window through the window manager before the window is
mapped. And when the window is mapped (via DesktopWindowTreeHostX11::MapWindow),
we always reset should_maximize_after_map. Is there another way for the window
to get mapped besides using that function?

[Elliot Glaysher, 2014-09-05 17:22:57]

On 2014/09/05 11:23:26, arjanl wrote:
>
https://codereview.chromium.org/543663003/diff/1/ui/views/widget/desktop_aura...
> File ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc (right):
> 
>
https://codereview.chromium.org/543663003/diff/1/ui/views/widget/desktop_aura...
> ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:564:
> should_maximize_after_map_ = false;
> On 2014/09/04 17:37:04, Elliot Glaysher wrote:
> > This seems fragile.
> > 
> > Could you please test in the case of Maximizing through code before the
window
> > is mapped, restoring through the window manager controls (not chrome's
> buttons),
> 
> You can't restore the window through the window manager before the window is
> mapped. And when the window is mapped (via
DesktopWindowTreeHostX11::MapWindow),
> we always reset should_maximize_after_map. Is there another way for the window
> to get mapped besides using that function?

I guess not. LGTM.

[Arjan van Leeuwen, 2014-09-08 07:55:00]

The CQ bit was checked by arjanl@opera.com

[commit-bot: I haz the power, 2014-09-08 07:56:09]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/arjanl@opera.com/543663003/1

[commit-bot: I haz the power, 2014-09-08 09:48:05]

Committed patchset #1 (id:1) as 334fca491b7e798d75e76a56c57074d3187602be

[Nico, 2014-09-08 20:49:43]

A revert of this CL (patchset #1 id:1) has been created in
https://codereview.chromium.org/554843002/ by thakis@chromium.org.

The reason for reverting is: Speculative, to see if it helps with

==12563== WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x7f834414fd2c in
views::DesktopWindowTreeHostX11::InitX11Window(views::Widget::InitParams const&)
ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:1164
    #1 0x7f834414dc6e in views::DesktopWindowTreeHostX11::Init(aura::Window*,
views::Widget::InitParams const&)
ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc:259
    #2 0x7f83441408e9 in
views::DesktopNativeWidgetAura::InitNativeWidget(views::Widget::InitParams
const&) ui/views/widget/desktop_aura/desktop_native_widget_aura.cc:420
    #3 0x7f834411722f in views::Widget::Init(views::Widget::InitParams const&)
ui/views/widget/widget.cc:358
    #4 0x7f834473bc8b in
ChromeNativeAppWindowViews::InitializeDefaultWindow(extensions::AppWindow::CreateParams
const&) chrome/browser/ui/views/apps/chrome_native_app_window_views.cc:231
    #5 0x7f834473f52d in
ChromeNativeAppWindowViews::InitializeWindow(extensions::AppWindow*,
extensions::AppWindow::CreateParams const&)
chrome/browser/ui/views/apps/chrome_native_app_window_views.cc:672
    #6 0x7f834c3d8bbe in
apps::NativeAppWindowViews::Init(extensions::AppWindow*,
extensions::AppWindow::CreateParams const&)
apps/ui/views/native_app_window_views.cc:46
    #7 0x7f834460443c in
ChromeAppsClient::CreateNativeAppWindowImpl(extensions::AppWindow*,
extensions::AppWindow::CreateParams const&)
chrome/browser/ui/views/apps/chrome_apps_client_views.cc:14
    #8 0x7f834be98433 in extensions::AppWindow::Init(GURL const&,
extensions::AppWindowContents*, extensions::AppWindow::CreateParams const&)
extensions/browser/app_window/app_window.cc:281
    #9 0x7f834bd6bbfb in extensions::AppWindowCreateFunction::RunAsync()
extensions/browser/api/app_window/app_window_api.cc:296
    #10 0x7f834bee4510 in AsyncExtensionFunction::Run()
extensions/browser/extension_function.cc:452
    #11 0x7f834bee9771 in
extensions::ExtensionFunctionDispatcher::DispatchWithCallbackInternal(ExtensionHostMsg_Request_Params
const&, content::RenderViewHost*, content::RenderFrameHost*, base::Callback<void
(ExtensionFunction::ResponseType, base::ListValue const&,
std::__1::basic_string<char, std::__1::char_traits<char>,
std::__1::allocator<char> > const&)> const&)
extensions/browser/extension_function_dispatcher.cc:392
    #12 0x7f834bee86b6 in
extensions::ExtensionFunctionDispatcher::Dispatch(ExtensionHostMsg_Request_Params
const&, content::RenderViewHost*)
extensions/browser/extension_function_dispatcher.cc:309
    #13 0x7f834bedac91 in OnRequest extensions/browser/extension_host.cc:342
    #14 0x7f8345add9d2 in
content::WebContentsImpl::OnMessageReceived(content::RenderViewHost*,
content::RenderFrameHost*, IPC::Message const&)
content/browser/web_contents/web_contents_impl.cc:526
    #15 0x7f83459847fe in
content::RenderViewHostImpl::OnMessageReceived(IPC::Message const&)
content/browser/renderer_host/render_view_host_impl.cc:894

Started happening here
http://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20MSan%20Tests...
(see
http://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20MSan%20Tests...
for symbols), and this looks tangentially related (it touched the file the
uninitialized read is in.)

Line 1164 is

  if (params.visible_on_all_workspaces) {
    state_atom_list.push_back(atom_cache_.GetAtom("_NET_WM_STATE_STICKY"));
    ui::SetIntProperty(xwindow_, "_NET_WM_DESKTOP", "CARDINAL", kAllDesktops);
  }

https://chromium.googlesource.com/chromium/src/+blame/master/ui/views/widget/....

[Nico, 2014-09-08 20:56:24]

I aborted the revert, I think before the cq landed it. 
https://codereview.chromium.org/469993003 looks like a more likely cause.

[commit-bot: I haz the power, 2014-09-10 03:45:28]

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