Chromium Code Reviews

Issue 551123002: Revert of Revert of Fix maximize on some window managers (Closed)

Created:
6 years, 3 months ago by Nico
Modified:
6 years, 3 months ago
Reviewers:
Elliot Glaysher, Arjan van Leeuwen
CC:
chromium-reviews, tfarina, ben+views_chromium.org, tdanderson+views_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Revert of Revert of Fix maximize on some window managers (patchset #1 id:1 of https://codereview.chromium.org/554843002/) Reason for revert: I unchecked cq, not sure why this revert was landed. (I want to revert https://codereview.chromium.org/469993003/ instead) Original issue's description: > 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= > > Committed: https://chromium.googlesource.com/chromium/src/+/013b6d4051e8e2c8960faa12b92e273925aaee5b TBR=erg@chromium.org,arjanl@opera.com NOTREECHECKS=true NOTRY=true BUG= Committed: https://crrev.com/8c59383c8663fc04f6f15f45ac5ff44fef215995 Cr-Commit-Position: refs/heads/master@{#293790}

Patch Set 1 #

Unified diffs Side-by-side diffs Stats (+16 lines, -3 lines)
M ui/views/widget/desktop_aura/desktop_window_tree_host_x11.h View 1 chunk +3 lines, -0 lines 0 comments
M ui/views/widget/desktop_aura/desktop_window_tree_host_x11.cc View 5 chunks +13 lines, -3 lines 0 comments

Messages

Total messages: 4 (0 generated)
Nico
Created Revert of Revert of Fix maximize on some window managers
6 years, 3 months ago (2014-09-08 21:17:10 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/thakis@chromium.org/551123002/1
6 years, 3 months ago (2014-09-08 21:24:29 UTC) #2
commit-bot: I haz the power
Committed patchset #1 (id:1) as c79e7c876e4cf1d14f8526bbba6949bc6120f434
6 years, 3 months ago (2014-09-08 21:39:55 UTC) #3
commit-bot: I haz the power
6 years, 3 months ago (2014-09-10 03:48:18 UTC) #4
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/8c59383c8663fc04f6f15f45ac5ff44fef215995
Cr-Commit-Position: refs/heads/master@{#293790}

Powered by Google App Engine