Commit c4d26a44 authored by thakis's avatar thakis Committed by Commit bot

Revert of Fix maximize on some window managers (patchset #1 id:1 of...

Revert of Fix maximize on some window managers (patchset #1 id:1 of https://codereview.chromium.org/543663003/)

Reason for revert:
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/builds/104/ (see http://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20MSan%20Tests/builds/104/ 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/desktop_aura/desktop_window_tree_host_x11.cc

Original issue's description:
> 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://chromium.googlesource.com/chromium/src/+/334fca491b7e798d75e76a56c57074d3187602be

TBR=erg@chromium.org,arjanl@opera.com
NOTREECHECKS=true
NOTRY=true
BUG=

Review URL: https://codereview.chromium.org/554843002

Cr-Commit-Position: refs/heads/master@{#293786}
parent 9156c041
...@@ -138,7 +138,6 @@ DesktopWindowTreeHostX11::DesktopWindowTreeHostX11( ...@@ -138,7 +138,6 @@ DesktopWindowTreeHostX11::DesktopWindowTreeHostX11(
is_fullscreen_(false), is_fullscreen_(false),
is_always_on_top_(false), is_always_on_top_(false),
use_native_frame_(false), use_native_frame_(false),
should_maximize_after_map_(false),
use_argb_visual_(false), use_argb_visual_(false),
drag_drop_client_(NULL), drag_drop_client_(NULL),
native_widget_delegate_(native_widget_delegate), native_widget_delegate_(native_widget_delegate),
...@@ -367,6 +366,9 @@ void DesktopWindowTreeHostX11::ShowWindowWithState( ...@@ -367,6 +366,9 @@ void DesktopWindowTreeHostX11::ShowWindowWithState(
if (show_state == ui::SHOW_STATE_NORMAL || if (show_state == ui::SHOW_STATE_NORMAL ||
show_state == ui::SHOW_STATE_MAXIMIZED) { show_state == ui::SHOW_STATE_MAXIMIZED) {
// Note: XFCE ignores a maximize hint given before mapping the window.
if (show_state == ui::SHOW_STATE_MAXIMIZED)
Maximize();
Activate(); Activate();
} }
...@@ -539,10 +541,6 @@ void DesktopWindowTreeHostX11::Maximize() { ...@@ -539,10 +541,6 @@ void DesktopWindowTreeHostX11::Maximize() {
SetBounds(adjusted_bounds); SetBounds(adjusted_bounds);
} }
// Some WMs do not respect maximization hints on unmapped windows, so we
// save this one for later too.
should_maximize_after_map_ = !window_mapped_;
// When we are in the process of requesting to maximize a window, we can // When we are in the process of requesting to maximize a window, we can
// accurately keep track of our restored bounds instead of relying on the // accurately keep track of our restored bounds instead of relying on the
// heuristics that are in the PropertyNotify and ConfigureNotify handlers. // heuristics that are in the PropertyNotify and ConfigureNotify handlers.
...@@ -561,7 +559,6 @@ void DesktopWindowTreeHostX11::Minimize() { ...@@ -561,7 +559,6 @@ void DesktopWindowTreeHostX11::Minimize() {
} }
void DesktopWindowTreeHostX11::Restore() { void DesktopWindowTreeHostX11::Restore() {
should_maximize_after_map_ = false;
SetWMSpecState(false, SetWMSpecState(false,
atom_cache_.GetAtom("_NET_WM_STATE_MAXIMIZED_VERT"), atom_cache_.GetAtom("_NET_WM_STATE_MAXIMIZED_VERT"),
atom_cache_.GetAtom("_NET_WM_STATE_MAXIMIZED_HORZ")); atom_cache_.GetAtom("_NET_WM_STATE_MAXIMIZED_HORZ"));
...@@ -1588,13 +1585,6 @@ void DesktopWindowTreeHostX11::MapWindow(ui::WindowShowState show_state) { ...@@ -1588,13 +1585,6 @@ void DesktopWindowTreeHostX11::MapWindow(ui::WindowShowState show_state) {
if (ui::X11EventSource::GetInstance()) if (ui::X11EventSource::GetInstance())
ui::X11EventSource::GetInstance()->BlockUntilWindowMapped(xwindow_); ui::X11EventSource::GetInstance()->BlockUntilWindowMapped(xwindow_);
window_mapped_ = true; window_mapped_ = true;
// Some WMs only respect maximize hints after the window has been mapped.
// Check whether we need to re-do a maximization.
if (should_maximize_after_map_) {
Maximize();
should_maximize_after_map_ = false;
}
} }
void DesktopWindowTreeHostX11::SetWindowTransparency() { void DesktopWindowTreeHostX11::SetWindowTransparency() {
......
...@@ -286,9 +286,6 @@ class VIEWS_EXPORT DesktopWindowTreeHostX11 ...@@ -286,9 +286,6 @@ class VIEWS_EXPORT DesktopWindowTreeHostX11
// True if the window has title-bar / borders provided by the window manager. // True if the window has title-bar / borders provided by the window manager.
bool use_native_frame_; bool use_native_frame_;
// True if a Maximize() call should be done after mapping the window.
bool should_maximize_after_map_;
// Whether we used an ARGB visual for our window. // Whether we used an ARGB visual for our window.
bool use_argb_visual_; bool use_argb_visual_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment