Eliminate PS printing edge cases

Eliminate PS printing edge cases

There were a couple possible edge cases with the new postscript language
level identification that could lead to incorrect behavior.
(1) Printer is stuck in GDI centric mode. In this case the change to
POSTSCRIPT_PASSTHROUGH will cause a failure. Check for POSTSCRIPT_
PASSTHROUGH support before running it.
(2) Printer gets set to PS mode but the postscript level detection
returns something weird. This would result in driver trying to send
Gdi commands to a printer in PS mode.
These issues are resolved in this patch.

Finally, this fixes the problem of postscript printers not printing
if the postscript feature is disabled by using GDI mode instead of
PS mode to query the postscript level.

BUG=614988
Review-Url: https://codereview.chromium.org/2714073002
Cr-Commit-Position: refs/heads/master@{#453312}
Committed: https://chromium.googlesource.com/chromium/src/+/d8fd468f11a46afd3544885930b71744aa09acd6

[rbpotter, 2017-02-24 02:35:05]

Description was changed from

==========
Eliminate PS printing edge cases

There were a couple possible edge cases with the new postscript language
level identification that could lead to incorrect behavior.
(1) Printer is stuck in GDI centric mode. In this case the change to
POSTSCRIPT_PASSTHROUGH will cause a failure. Check for POSTSCRIPT_
PASSTHROUGH support before running it.
(2) Printer gets set to PS mode but the postscript level detection
returns something weird. This would result in driver trying to send
Gdi commands to a printer in PS mode.

This patch fixes these edge cases.
BUG=614988
==========

to

==========
Eliminate PS printing edge cases

There were a couple possible edge cases with the new postscript language
level identification that could lead to incorrect behavior.
(1) Printer is stuck in GDI centric mode. In this case the change to
POSTSCRIPT_PASSTHROUGH will cause a failure. Check for POSTSCRIPT_
PASSTHROUGH support before running it.
(2) Printer gets set to PS mode but the postscript level detection
returns something weird. This would result in driver trying to send
Gdi commands to a printer in PS mode.

This patch fixes these edge cases.
BUG=614988
==========

[rbpotter, 2017-02-24 02:35:05]

rbpotter@chromium.org changed reviewers:
+ thestig@chromium.org

[rbpotter, 2017-02-24 02:35:11]

The CQ bit was checked by rbpotter@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-24 02:35:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-24 03:19:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-24 03:19:10]

Dry run: This issue passed the CQ dry run.

[rbpotter, 2017-02-24 21:40:23]

FYI, need to add a check before calling IsPrinterPostScript
to ensure that the postscript flag is actually enabled.
Otherwise we switch the printer to PS mode and then can't
write to it normally. Working on a way to do this now.

[rbpotter, 2017-02-24 22:28:58]

The CQ bit was checked by rbpotter@chromium.org to run a CQ dry run

[rbpotter, 2017-02-24 22:29:29]

Added a check for postscript enabled so that PS printers
don't get set to PS mode unless the feature is enabled.

[commit-bot: I haz the power, 2017-02-24 22:29:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-24 22:40:40]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-24 22:40:40]

Dry run: Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[rbpotter, 2017-02-24 22:58:50]

The CQ bit was checked by rbpotter@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-24 22:59:22]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-25 00:32:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-25 00:32:23]

Dry run: This issue passed the CQ dry run.

[Lei Zhang, 2017-02-25 03:03:36]

Still reading through print_settings_initializer_win.cc...

https://codereview.chromium.org/2714073002/diff/60001/chrome/browser/printing...
File chrome/browser/printing/pdf_to_emf_converter.cc (right):

https://codereview.chromium.org/2714073002/diff/60001/chrome/browser/printing...
chrome/browser/printing/pdf_to_emf_converter.cc:372: bool postscript_passthrough
=
int passthrough = ExtEscape(hdc, QUERYESCSUPPORT, sizeof(escape), ptr, 0,
nullptr) > 0 ?  POSTSCRIPT_PASSTHROUGH : PASSTHROUGH;

Then below:

int ret = ExtEscape(hdc, passthrough, 2 + *ptr, data, 0, nullptr);

https://codereview.chromium.org/2714073002/diff/60001/printing/print_settings...
File printing/print_settings_initializer_win.cc (right):

https://codereview.chromium.org/2714073002/diff/60001/printing/print_settings...
printing/print_settings_initializer_win.cc:24: // If postscript, try to query
Postscript Identify and then set to
Do we need to move this comment?

https://codereview.chromium.org/2714073002/diff/60001/printing/print_settings...
printing/print_settings_initializer_win.cc:37: bool SetPrinterPostScript(HDC
hdc) {
Maybe SetPrinterToPostScriptMode() ?

https://codereview.chromium.org/2714073002/diff/60001/printing/printing_conte...
File printing/printing_context.h (right):

https://codereview.chromium.org/2714073002/diff/60001/printing/printing_conte...
printing/printing_context.h:114: // Sets postscript printing enabled
"Sets if the postscript printing feature is enabled."

https://codereview.chromium.org/2714073002/diff/60001/printing/printing_conte...
printing/printing_context.h:115: virtual void SetPostScriptEnabled(bool
is_enabled) = 0;
Maybe we should just ifdef this so we don't need dummy implementations on other
platforms.

https://codereview.chromium.org/2714073002/diff/60001/printing/printing_conte...
File printing/printing_context_win.h (right):

https://codereview.chromium.org/2714073002/diff/60001/printing/printing_conte...
printing/printing_context_win.h:57: bool postscript_enabled() { return
is_postscript_enabled_; }
const method

https://codereview.chromium.org/2714073002/diff/60001/printing/printing_conte...
printing/printing_context_win.h:66: // Is postscript printing enabled
"Is the postscript printing feature enabled"

[Lei Zhang, 2017-02-25 03:24:53]

https://codereview.chromium.org/2714073002/diff/60001/printing/print_settings...
File printing/print_settings_initializer_win.cc (right):

https://codereview.chromium.org/2714073002/diff/60001/printing/print_settings...
printing/print_settings_initializer_win.cc:45: int GetPrinterPostScriptLevel(HDC
hdc) {
Does this require PS centric mode? Or does it just work better in PS centric
mode? A comment about the pre-conditions before calling will be helpful.

My notes from several months ago says FEATURESETTING_PSLEVEL requires GDI or PS
centric mode. Does not work in Compatibility mode.

https://codereview.chromium.org/2714073002/diff/60001/printing/print_settings...
printing/print_settings_initializer_win.cc:48: int param_out = -1;
Can we just use 0 or do we need to distinguish between 0 and -1?

https://codereview.chromium.org/2714073002/diff/60001/printing/print_settings...
printing/print_settings_initializer_win.cc:62: if (HasEscapeSupprt(hdc,
POSTSCRIPT_IDENTIFY)) {
Have you found printers that support POSTSCRIPT_IDENTIFY? I only found 1 with
one set of Windows 7 printer drivers.

https://codereview.chromium.org/2714073002/diff/60001/printing/print_settings...
printing/print_settings_initializer_win.cc:144: // Check for postscript first so
that we can change the mode with the
This is a leftover question from the last CL. Why can't we do this first and
return early if the printer is XPS? If it's not, then fiddle with the modes. Is
there a limitation where POSTSCRIPT_IDENTIFY has to be called first before
calling GETTECHNOLOGY?

[rbpotter, 2017-02-25 04:40:28]

Patchset #5 (id:80001) has been deleted

[rbpotter, 2017-02-25 04:40:34]

The CQ bit was checked by rbpotter@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-25 04:40:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[rbpotter, 2017-02-25 04:50:40]

Checked Win7 and 10, both PS levels, feature on and off.
GDI seems to work.

[commit-bot: I haz the power, 2017-02-25 05:26:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-25 05:26:26]

Dry run: This issue passed the CQ dry run.

[Lei Zhang, 2017-02-25 09:09:35]

https://codereview.chromium.org/2714073002/diff/100001/printing/print_setting...
File printing/print_settings_initializer_win.cc (right):

https://codereview.chromium.org/2714073002/diff/100001/printing/print_setting...
printing/print_settings_initializer_win.cc:15: bool HasEscapeSupprt(HDC hdc,
DWORD escape) {
Looks like we didn't have the $250 and couldn't buy a vowel here.

https://codereview.chromium.org/2714073002/diff/100001/printing/print_setting...
printing/print_settings_initializer_win.cc:51: return -1;
0 here as well?

https://codereview.chromium.org/2714073002/diff/100001/printing/print_setting...
printing/print_settings_initializer_win.cc:56: // Try to set to Gdi mode so that
language level detection works (not
s/Gdi/GDI/

https://codereview.chromium.org/2714073002/diff/100001/printing/print_setting...
printing/print_settings_initializer_win.cc:63: SetPrinterToGdiMode(hdc);
This patch set no longer checks the return value. Does that mean its return type
can be void?

https://codereview.chromium.org/2714073002/diff/100001/printing/print_setting...
printing/print_settings_initializer_win.cc:73: if (!HasEscapeSupprt(hdc,
GET_PS_FEATURESETTING)) {
If a printer does not support POSTSCRIPT_IDENTIFY, we can still reach here by
passing the two else-if checks. However, in that case we are still in
compatibility mode and this block will always execute. Can we structure the
logic as follows?

if (!HasEscapeSupport(hdc, POSTSCRIPT_IDENTIFY)) {
  if (!IsTechnology(hdc, kPostScriptDriver))
    return false;
  if (!HasEscapeSupport(hdc, POSTSCRIPT_PASSTHROUGH) ||
      !HasEscapeSupport(hdc, POSTSCRIPT_DATA)) {
    return false;
  }
  *level = 2;
  return true;
}

SetPrinterToGdiMode(hdc);
if (!HasEscapeSupport(hdc, GET_PS_FEATURESETTING)) {
  *level = 2;
  return true;
}

*level = GetPrinterPostScriptLevel(hdc);
return *level == 2 || *level == 3;

[Lei Zhang, 2017-02-25 10:17:11]

On 2017/02/25 04:50:40, rbpotter wrote:
> Checked Win7 and 10, both PS levels, feature on and off.
> GDI seems to work.

Seems to work for me too based on limited testing. I also verified for
GET_PS_FEATURESETTING to work, we indeed have to switch out of compatibility
mode. Looks like we have to do this if we ever want to set PS level 3.

I also noticed at least some (if not all) Win 10 built-in printer drivers say
they are XPS in response to GETTECHNOLOGY. I believe that is because they use
the V4 driver model (introduced with Win 8) which is XPS-based.

[rbpotter, 2017-02-27 16:40:50]

Yes, I noticed that when I first started working on this
on Windows 10. You have to manually install the Universal
PS drivers, seems most if not all built-ins report as XPS.

https://codereview.chromium.org/2714073002/diff/100001/printing/print_setting...
File printing/print_settings_initializer_win.cc (right):

https://codereview.chromium.org/2714073002/diff/100001/printing/print_setting...
printing/print_settings_initializer_win.cc:15: bool HasEscapeSupprt(HDC hdc,
DWORD escape) {
On 2017/02/25 09:09:35, Lei Zhang (super slow) wrote:
> Looks like we didn't have the $250 and couldn't buy a vowel here.

Done.

https://codereview.chromium.org/2714073002/diff/100001/printing/print_setting...
printing/print_settings_initializer_win.cc:51: return -1;
On 2017/02/25 09:09:35, Lei Zhang (super slow) wrote:
> 0 here as well?

Done.

https://codereview.chromium.org/2714073002/diff/100001/printing/print_setting...
printing/print_settings_initializer_win.cc:56: // Try to set to Gdi mode so that
language level detection works (not
On 2017/02/25 09:09:35, Lei Zhang (super slow) wrote:
> s/Gdi/GDI/

Done.

https://codereview.chromium.org/2714073002/diff/100001/printing/print_setting...
printing/print_settings_initializer_win.cc:63: SetPrinterToGdiMode(hdc);
On 2017/02/25 09:09:35, Lei Zhang (super slow) wrote:
> This patch set no longer checks the return value. Does that mean its return
type
> can be void?

Yes- don't need to check since GDI mode doesn't cause problems for normal
printing. Done.

https://codereview.chromium.org/2714073002/diff/100001/printing/print_setting...
printing/print_settings_initializer_win.cc:73: if (!HasEscapeSupprt(hdc,
GET_PS_FEATURESETTING)) {
On 2017/02/25 09:09:35, Lei Zhang (super slow) wrote:
> If a printer does not support POSTSCRIPT_IDENTIFY, we can still reach here by
> passing the two else-if checks. However, in that case we are still in
> compatibility mode and this block will always execute. Can we structure the
> logic as follows?
> 
> if (!HasEscapeSupport(hdc, POSTSCRIPT_IDENTIFY)) {
>   if (!IsTechnology(hdc, kPostScriptDriver))
>     return false;
>   if (!HasEscapeSupport(hdc, POSTSCRIPT_PASSTHROUGH) ||
>       !HasEscapeSupport(hdc, POSTSCRIPT_DATA)) {
>     return false;
>   }
>   *level = 2;
>   return true;
> }
> 
> SetPrinterToGdiMode(hdc);
> if (!HasEscapeSupport(hdc, GET_PS_FEATURESETTING)) {
>   *level = 2;
>   return true;
> }
> 
> *level = GetPrinterPostScriptLevel(hdc);
> return *level == 2 || *level == 3;

Done.

[rbpotter, 2017-02-27 16:51:06]

The CQ bit was checked by rbpotter@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-27 16:51:26]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[rbpotter, 2017-02-27 16:51:44]

Description was changed from

==========
Eliminate PS printing edge cases

There were a couple possible edge cases with the new postscript language
level identification that could lead to incorrect behavior.
(1) Printer is stuck in GDI centric mode. In this case the change to
POSTSCRIPT_PASSTHROUGH will cause a failure. Check for POSTSCRIPT_
PASSTHROUGH support before running it.
(2) Printer gets set to PS mode but the postscript level detection
returns something weird. This would result in driver trying to send
Gdi commands to a printer in PS mode.

This patch fixes these edge cases.
BUG=614988
==========

to

==========
Eliminate PS printing edge cases

There were a couple possible edge cases with the new postscript language
level identification that could lead to incorrect behavior.
(1) Printer is stuck in GDI centric mode. In this case the change to
POSTSCRIPT_PASSTHROUGH will cause a failure. Check for POSTSCRIPT_
PASSTHROUGH support before running it.
(2) Printer gets set to PS mode but the postscript level detection
returns something weird. This would result in driver trying to send
Gdi commands to a printer in PS mode.
Finally, this fixes the problem of postscript printers not printing
if the postscript feature is disabled by using GDI mode instead of
PS mode to query the postscript level.

This patch fixes these edge cases.
BUG=614988
==========

[rbpotter, 2017-02-27 16:52:10]

Description was changed from

==========
Eliminate PS printing edge cases

There were a couple possible edge cases with the new postscript language
level identification that could lead to incorrect behavior.
(1) Printer is stuck in GDI centric mode. In this case the change to
POSTSCRIPT_PASSTHROUGH will cause a failure. Check for POSTSCRIPT_
PASSTHROUGH support before running it.
(2) Printer gets set to PS mode but the postscript level detection
returns something weird. This would result in driver trying to send
Gdi commands to a printer in PS mode.
Finally, this fixes the problem of postscript printers not printing
if the postscript feature is disabled by using GDI mode instead of
PS mode to query the postscript level.

This patch fixes these edge cases.
BUG=614988
==========

to

==========
Eliminate PS printing edge cases

There were a couple possible edge cases with the new postscript language
level identification that could lead to incorrect behavior.
(1) Printer is stuck in GDI centric mode. In this case the change to
POSTSCRIPT_PASSTHROUGH will cause a failure. Check for POSTSCRIPT_
PASSTHROUGH support before running it.
(2) Printer gets set to PS mode but the postscript level detection
returns something weird. This would result in driver trying to send
Gdi commands to a printer in PS mode.
These issues are resolved in this patch.

Finally, this fixes the problem of postscript printers not printing
if the postscript feature is disabled by using GDI mode instead of
PS mode to query the postscript level.

BUG=614988
==========

[commit-bot: I haz the power, 2017-02-27 17:46:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-27 17:46:03]

Dry run: This issue passed the CQ dry run.

[Lei Zhang, 2017-02-27 19:46:47]

lgtm

[rbpotter, 2017-02-27 19:54:37]

The CQ bit was checked by rbpotter@chromium.org

[commit-bot: I haz the power, 2017-02-27 19:55:15]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-27 20:06:44]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1488225277120300,
"parent_rev": "98f81ced0431b88f99df691fa495deaacb0d095d", "commit_rev":
"d8fd468f11a46afd3544885930b71744aa09acd6"}

[commit-bot: I haz the power, 2017-02-27 20:07:23]

Description was changed from

==========
Eliminate PS printing edge cases

There were a couple possible edge cases with the new postscript language
level identification that could lead to incorrect behavior.
(1) Printer is stuck in GDI centric mode. In this case the change to
POSTSCRIPT_PASSTHROUGH will cause a failure. Check for POSTSCRIPT_
PASSTHROUGH support before running it.
(2) Printer gets set to PS mode but the postscript level detection
returns something weird. This would result in driver trying to send
Gdi commands to a printer in PS mode.
These issues are resolved in this patch.

Finally, this fixes the problem of postscript printers not printing
if the postscript feature is disabled by using GDI mode instead of
PS mode to query the postscript level.

BUG=614988
==========

to

==========
Eliminate PS printing edge cases

There were a couple possible edge cases with the new postscript language
level identification that could lead to incorrect behavior.
(1) Printer is stuck in GDI centric mode. In this case the change to
POSTSCRIPT_PASSTHROUGH will cause a failure. Check for POSTSCRIPT_
PASSTHROUGH support before running it.
(2) Printer gets set to PS mode but the postscript level detection
returns something weird. This would result in driver trying to send
Gdi commands to a printer in PS mode.
These issues are resolved in this patch.

Finally, this fixes the problem of postscript printers not printing
if the postscript feature is disabled by using GDI mode instead of
PS mode to query the postscript level.

BUG=614988

Review-Url: https://codereview.chromium.org/2714073002
Cr-Commit-Position: refs/heads/master@{#453312}
Committed:
https://chromium.googlesource.com/chromium/src/+/d8fd468f11a46afd3544885930b7...
==========

[commit-bot: I haz the power, 2017-02-27 20:07:24]

Committed patchset #6 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/d8fd468f11a46afd3544885930b7...