diff options
author | scottmg@chromium.org <scottmg@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-02-13 17:04:31 +0000 |
---|---|---|
committer | scottmg@chromium.org <scottmg@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-02-13 17:04:31 +0000 |
commit | f487ac255f512041852d1569272a4ad2bff2a78a (patch) | |
tree | 34bfe0692f6ec04402e27a7d98a7bee00e873390 /printing | |
parent | 04af2789c7c7691211d629255dbdd63e2c50c678 (diff) | |
download | chromium_src-f487ac255f512041852d1569272a4ad2bff2a78a.zip chromium_src-f487ac255f512041852d1569272a4ad2bff2a78a.tar.gz chromium_src-f487ac255f512041852d1569272a4ad2bff2a78a.tar.bz2 |
Revert 230235 "Use BaseShellDialog for print dialog on Windows"
Per discussion with ananta, opening a system modal dialog from a
background thread is a bad idea, so reverting this (from long ago).
Windows doesn't preserve the order correctly when the window is not on
the main UI thread, and in the bug here, it interacts very badly with
the IME toolbar window, causing the modal print dialog to go behind the
browser window in Z order making chrome appear to be hung.
The original goal was to fix tasks not being dispatched while the dialog
was open. We can instead use the ScopedNestedTaskAllower as we've done
for other system modal dialogs which I'll do in a separate CL.
BUG=342697,180997
> Use BaseShellDialog for print dialog on Windows
>
> This puts the print dialog on a background thread which is necessary so other
> top level windows can keep painting as Aura does the compositor swaps on the
> UI thread.
>
> R=sky@chromium.org,vitalybuka@chromium.org
>
>
> Review URL: https://codereview.chromium.org/27441003
TBR=scottmg@chromium.org,sky@chromium.org,vitalybuka@chromium.org,ananta@chromium.org
Review URL: https://codereview.chromium.org/164013002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@251066 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'printing')
-rw-r--r-- | printing/DEPS | 1 | ||||
-rw-r--r-- | printing/printing.gyp | 1 | ||||
-rw-r--r-- | printing/printing_context_win.cc | 58 | ||||
-rw-r--r-- | printing/printing_context_win.h | 15 |
4 files changed, 23 insertions, 52 deletions
diff --git a/printing/DEPS b/printing/DEPS index bc43b41..97c498b 100644 --- a/printing/DEPS +++ b/printing/DEPS @@ -8,6 +8,5 @@ include_rules = [ "+ui/base/resource", "+ui/base/text", "+ui/gfx", - "+ui/shell_dialogs", "+win8/util", ] diff --git a/printing/printing.gyp b/printing/printing.gyp index 4b4646b..adcab4b 100644 --- a/printing/printing.gyp +++ b/printing/printing.gyp @@ -19,7 +19,6 @@ '../third_party/icu/icu.gyp:icuuc', '../ui/gfx/gfx.gyp:gfx', '../ui/gfx/gfx.gyp:gfx_geometry', - '../ui/shell_dialogs/shell_dialogs.gyp:shell_dialogs', '../url/url.gyp:url_lib', ], 'defines': [ diff --git a/printing/printing_context_win.cc b/printing/printing_context_win.cc index 1c1e3af..a2c3ada 100644 --- a/printing/printing_context_win.cc +++ b/printing/printing_context_win.cc @@ -175,11 +175,11 @@ PrintingContextWin::~PrintingContextWin() { ReleaseContext(); } +// TODO(vitalybuka): Implement as ui::BaseShellDialog crbug.com/180997. void PrintingContextWin::AskUserForSettings( gfx::NativeView view, int max_pages, bool has_selection, const PrintSettingsCallback& callback) { DCHECK(!in_print_job_); - // TODO(scottmg): Possibly this has to move into the threaded runner too? if (win8::IsSingleWindowMetroMode()) { // The system dialog can not be opened while running in Metro. // But we can programatically launch the Metro print device charm though. @@ -212,40 +212,40 @@ void PrintingContextWin::AskUserForSettings( // - Cancel, the settings are not changed, the previous setting, if it was // initialized before, are kept. CANCEL is returned. // On failure, the settings are reset and FAILED is returned. - PRINTDLGEX* dialog_options = - reinterpret_cast<PRINTDLGEX*>(malloc(sizeof(PRINTDLGEX))); - ZeroMemory(dialog_options, sizeof(PRINTDLGEX)); - dialog_options->lStructSize = sizeof(PRINTDLGEX); - dialog_options->hwndOwner = window; + PRINTDLGEX dialog_options = { sizeof(PRINTDLGEX) }; + dialog_options.hwndOwner = window; // Disable options we don't support currently. // TODO(maruel): Reuse the previously loaded settings! - dialog_options->Flags = PD_RETURNDC | PD_USEDEVMODECOPIESANDCOLLATE | - PD_NOCURRENTPAGE | PD_HIDEPRINTTOFILE; + dialog_options.Flags = PD_RETURNDC | PD_USEDEVMODECOPIESANDCOLLATE | + PD_NOCURRENTPAGE | PD_HIDEPRINTTOFILE; if (!has_selection) - dialog_options->Flags |= PD_NOSELECTION; + dialog_options.Flags |= PD_NOSELECTION; - const size_t max_page_ranges = 32; - PRINTPAGERANGE* ranges = new PRINTPAGERANGE[max_page_ranges]; - dialog_options->lpPageRanges = ranges; - dialog_options->nStartPage = START_PAGE_GENERAL; + PRINTPAGERANGE ranges[32]; + dialog_options.nStartPage = START_PAGE_GENERAL; if (max_pages) { // Default initialize to print all the pages. memset(ranges, 0, sizeof(ranges)); ranges[0].nFromPage = 1; ranges[0].nToPage = max_pages; - dialog_options->nPageRanges = 1; - dialog_options->nMaxPageRanges = max_page_ranges; - dialog_options->nMinPage = 1; - dialog_options->nMaxPage = max_pages; + dialog_options.nPageRanges = 1; + dialog_options.nMaxPageRanges = arraysize(ranges); + dialog_options.nMinPage = 1; + dialog_options.nMaxPage = max_pages; + dialog_options.lpPageRanges = ranges; } else { // No need to bother, we don't know how many pages are available. - dialog_options->Flags |= PD_NOPAGENUMS; + dialog_options.Flags |= PD_NOPAGENUMS; } - callback_ = callback; - print_settings_dialog_ = new ui::PrintSettingsDialogWin(this); - print_settings_dialog_->GetPrintSettings( - print_dialog_func_, window, dialog_options); + HRESULT hr = (*print_dialog_func_)(&dialog_options); + if (hr != S_OK) { + ResetSettings(); + callback.Run(FAILED); + } + + // TODO(maruel): Support PD_PRINTTOFILE. + callback.Run(ParseDialogResultEx(dialog_options)); } PrintingContext::Result PrintingContextWin::UseDefaultSettings() { @@ -527,20 +527,6 @@ gfx::NativeDrawingContext PrintingContextWin::context() const { return context_; } -void PrintingContextWin::PrintSettingsConfirmed(PRINTDLGEX* dialog_options) { - // TODO(maruel): Support PD_PRINTTOFILE. - callback_.Run(ParseDialogResultEx(*dialog_options)); - delete [] dialog_options->lpPageRanges; - free(dialog_options); -} - -void PrintingContextWin::PrintSettingsCancelled(PRINTDLGEX* dialog_options) { - ResetSettings(); - callback_.Run(FAILED); - delete [] dialog_options->lpPageRanges; - free(dialog_options); -} - // static BOOL PrintingContextWin::AbortProc(HDC hdc, int nCode) { if (nCode) { diff --git a/printing/printing_context_win.h b/printing/printing_context_win.h index 3be4917..d0cd2fb 100644 --- a/printing/printing_context_win.h +++ b/printing/printing_context_win.h @@ -14,13 +14,10 @@ #include "build/build_config.h" #include "printing/printing_context.h" #include "ui/gfx/native_widget_types.h" -#include "ui/shell_dialogs/print_settings_dialog_win.h" namespace printing { -class PRINTING_EXPORT PrintingContextWin - : public PrintingContext, - public ui::PrintSettingsDialogWin::Observer { +class PRINTING_EXPORT PrintingContextWin : public PrintingContext { public: explicit PrintingContextWin(const std::string& app_locale); ~PrintingContextWin(); @@ -43,10 +40,6 @@ class PRINTING_EXPORT PrintingContextWin virtual void ReleaseContext() OVERRIDE; virtual gfx::NativeDrawingContext context() const OVERRIDE; - // PrintSettingsDialogWin::Observer implementation: - virtual void PrintSettingsConfirmed(PRINTDLGEX* dialog_options) OVERRIDE; - virtual void PrintSettingsCancelled(PRINTDLGEX* dialog_options) OVERRIDE; - #if defined(UNIT_TEST) || defined(PRINTING_IMPLEMENTATION) // Sets a fake PrintDlgEx function pointer in tests. void SetPrintDialog(HRESULT (__stdcall *print_dialog_func)(LPPRINTDLGEX)) { @@ -94,12 +87,6 @@ class PRINTING_EXPORT PrintingContextWin // SetPrintDialog() in tests. HRESULT (__stdcall *print_dialog_func_)(LPPRINTDLGEX); - // Where to notify when the dialog is closed. - PrintSettingsCallback callback_; - - // Wrapper around native print dialog that runs it on a background thread. - scoped_refptr<ui::PrintSettingsDialogWin> print_settings_dialog_; - DISALLOW_COPY_AND_ASSIGN(PrintingContextWin); }; |