chrome/browser/ui/libgtk2ui/print_dialog_gtk2.cc - Issue 2165083002: Linux: Refactor X11DesktopHandler

Unified Diff: chrome/browser/ui/libgtk2ui/print_dialog_gtk2.cc

Issue 2165083002: Linux: Refactor X11DesktopHandler (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: Fix tests Created 4 years, 4 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Download patch

Index: chrome/browser/ui/libgtk2ui/print_dialog_gtk2.cc

diff --git a/chrome/browser/ui/libgtk2ui/print_dialog_gtk2.cc b/chrome/browser/ui/libgtk2ui/print_dialog_gtk2.cc

index 59157a41793d9fad72ebdc88fee7a6c23ef05bda..219fe7ef8f774325b443e5f3720d9fff16fb9b3d 100644

--- a/chrome/browser/ui/libgtk2ui/print_dialog_gtk2.cc

+++ b/chrome/browser/ui/libgtk2ui/print_dialog_gtk2.cc

@@ -25,7 +25,6 @@

#include "printing/print_job_constants.h"

#include "printing/print_settings.h"

#include "ui/aura/window.h"

-#include "ui/views/widget/desktop_aura/x11_desktop_handler.h"

using content::BrowserThread;

using printing::PageRanges;

@@ -366,8 +365,7 @@ void PrintDialogGtk2::ShowDialog(

// We need to call gtk_window_present after making the widgets visible to make

// sure window gets correctly raised and gets focus.

- int time = views::X11DesktopHandler::get()->wm_user_time_ms();

danakj 2016/08/11 21:48:19 Using a time is better if we can. Not being famili

Tom (Use chromium acct) 2016/08/15 18:42:45 :( ../../chrome/browser/ui/libgtk2ui/print_dialog

:( ../../chrome/browser/ui/libgtk2ui/print_dialog_gtk2.cc:370: error: undefined reference to 'ui::X11EventSource::GetInstance()' ../../chrome/browser/ui/libgtk2ui/print_dialog_gtk2.cc:370: error: undefined reference to 'ui::X11EventSource::GetTimestamp()' ../../chrome/browser/ui/libgtk2ui/select_file_dialog_impl_gtk2.cc:173: error: undefined reference to 'ui::X11EventSource::GetInstance()' ../../chrome/browser/ui/libgtk2ui/select_file_dialog_impl_gtk2.cc:173: error: undefined reference to 'ui::X11EventSource::GetTimestamp()' I agree this sounds like the right thing to do. Maybe we could split off x11_events_platform into it's own .so, or provide a proxy in views? Here's a list of possible places we could put it thomasanderson@thomasanderson:~/Developer/chromium_6/src$ ldd out/Debug/libgtk2ui.so linux-vdso.so.1 => (0x00007ffe5f9d5000) libbase.so => not found libbase_i18n.so => not found libprinting.so => not found libskia.so => not found libaura.so => not found libui_base.so => not found libui_base_ime.so => not found libdisplay.so => not found libevents.so => not found libevents_base.so => not found libgfx.so => not found libgeometry.so => not found libgfx_x11.so => not found libnative_theme.so => not found libshell_dialogs.so => not found libviews.so => not found libicui18n.so => not found libicuuc.so => not found libcontent.so => not found libgpu.so => not found libgles2_utils.so => not found librange.so => not found libgl_wrapper.so => not found libplatform.so => not found libgl_init.so => not found libipc.so => not found libmojo_public_system.so => not found libkeycodes_x11.so => not found libui_base_x.so => not found liburl.so => not found libmojo_common_lib.so => not found libcc.so => not found libaccessibility.so => not found liburl_ipc.so => not found libcompositor.so => not found

Tom (Use chromium acct) 2016/08/19 00:26:31 Turns out the necessary dependency was added in th

On 2016/08/15 18:42:45, Tom Anderson wrote: > On 2016/08/11 21:48:19, danakj wrote: > > Using a time is better if we can. Not being familiar with our X11 event > handlers > > and all, would it be possible to do something like > > > > int time = ui::X11EventSource::GetInstance()->SomeServerTimeFunction() which > > does > > > > if (we're reacting to an X event handler with a time stamp like keypress or > > whatever) > > return the X event's timestamp; > > else > > return UpdateLastSeenServerTime(); > > > > > > I see you added DesktopWindowTreeHostX11::GetTimestamp() which is basically > > this. I think it'd make sense to put that function on X11EventSource and use > it > > for these presents. What do you think? > > :( > > ../../chrome/browser/ui/libgtk2ui/print_dialog_gtk2.cc:370: error: undefined > reference to 'ui::X11EventSource::GetInstance()' > ../../chrome/browser/ui/libgtk2ui/print_dialog_gtk2.cc:370: error: undefined > reference to 'ui::X11EventSource::GetTimestamp()' > ../../chrome/browser/ui/libgtk2ui/select_file_dialog_impl_gtk2.cc:173: error: > undefined reference to 'ui::X11EventSource::GetInstance()' > ../../chrome/browser/ui/libgtk2ui/select_file_dialog_impl_gtk2.cc:173: error: > undefined reference to 'ui::X11EventSource::GetTimestamp()' > > I agree this sounds like the right thing to do. Maybe we could split off > x11_events_platform into it's own .so, or provide a proxy in views? > > Here's a list of possible places we could put it > > thomasanderson@thomasanderson:~/Developer/chromium_6/src$ ldd > out/Debug/libgtk2ui.so > linux-vdso.so.1 => (0x00007ffe5f9d5000) > libbase.so => not found > libbase_i18n.so => not found > libprinting.so => not found > libskia.so => not found > libaura.so => not found > libui_base.so => not found > libui_base_ime.so => not found > libdisplay.so => not found > libevents.so => not found > libevents_base.so => not found > libgfx.so => not found > libgeometry.so => not found > libgfx_x11.so => not found > libnative_theme.so => not found > libshell_dialogs.so => not found > libviews.so => not found > libicui18n.so => not found > libicuuc.so => not found > libcontent.so => not found > libgpu.so => not found > libgles2_utils.so => not found > librange.so => not found > libgl_wrapper.so => not found > libplatform.so => not found > libgl_init.so => not found > libipc.so => not found > libmojo_public_system.so => not found > libkeycodes_x11.so => not found > libui_base_x.so => not found > liburl.so => not found > libmojo_common_lib.so => not found > libcc.so => not found > libaccessibility.so => not found > liburl_ipc.so => not found > libcompositor.so => not found

Turns out the necessary dependency was added in the past few days, so added GetTimestamp() here.

- gtk_window_present_with_time(GTK_WINDOW(dialog_), time);

+ gtk_window_present(GTK_WINDOW(dialog_));

}

void PrintDialogGtk2::PrintDocument(const printing::MetafilePlayer& metafile,

« no previous file with comments | « no previous file | chrome/browser/ui/libgtk2ui/select_file_dialog_impl_gtk2.cc » ('j') | chrome/browser/ui/startup/startup_browser_creator.cc » ('J')