ViewPainter should skip background in print economy mode

ViewPainter should skip background in print economy mode

The logic is applied in BoxPainter::paintFillLayerExtended() but the
ViewPainter does special treatment for base background color we should
apply the same logic there.

BUG=501303
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=197782

[trchen, 2015-06-23 04:44:48]

trchen@chromium.org changed reviewers:
+ chrishtr@chromium.org

[trchen, 2015-06-23 04:44:58]

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

[commit-bot: I haz the power, 2015-06-23 04:45:08]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1196223008/1

[commit-bot: I haz the power, 2015-06-23 05:39:56]

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

[commit-bot: I haz the power, 2015-06-23 05:39:57]

Dry run: This issue passed the CQ dry run.

[chrishtr, 2015-06-23 17:42:58]

https://codereview.chromium.org/1196223008/diff/1/Source/core/paint/ViewPaint...
File Source/core/paint/ViewPainter.cpp (right):

https://codereview.chromium.org/1196223008/diff/1/Source/core/paint/ViewPaint...
Source/core/paint/ViewPainter.cpp:67: if (document.printing() &&
m_layoutView.style()->printColorAdjust() == PrintColorAdjustEconomy
Where did this logic used to be before your refactor?

[chrishtr, 2015-06-23 17:42:58]

https://codereview.chromium.org/1196223008/diff/1/Source/core/paint/ViewPaint...
File Source/core/paint/ViewPainter.cpp (right):

https://codereview.chromium.org/1196223008/diff/1/Source/core/paint/ViewPaint...
Source/core/paint/ViewPainter.cpp:67: if (document.printing() &&
m_layoutView.style()->printColorAdjust() == PrintColorAdjustEconomy
Where did this logic used to be before your refactor?

[trchen, 2015-06-23 20:40:16]

https://codereview.chromium.org/1196223008/diff/1/Source/core/paint/ViewPaint...
File Source/core/paint/ViewPainter.cpp (right):

https://codereview.chromium.org/1196223008/diff/1/Source/core/paint/ViewPaint...
Source/core/paint/ViewPainter.cpp:67: if (document.printing() &&
m_layoutView.style()->printColorAdjust() == PrintColorAdjustEconomy
On 2015/06/23 17:42:57, chrishtr wrote:
> Where did this logic used to be before your refactor?

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

This new code should be equivalent if I didn't make any logical fallacy. :3

[chrishtr, 2015-06-23 21:40:51]

On 2015/06/23 at 20:40:16, trchen wrote:
>
https://codereview.chromium.org/1196223008/diff/1/Source/core/paint/ViewPaint...
> File Source/core/paint/ViewPainter.cpp (right):
> 
>
https://codereview.chromium.org/1196223008/diff/1/Source/core/paint/ViewPaint...
> Source/core/paint/ViewPainter.cpp:67: if (document.printing() &&
m_layoutView.style()->printColorAdjust() == PrintColorAdjustEconomy
> On 2015/06/23 17:42:57, chrishtr wrote:
> > Where did this logic used to be before your refactor?
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
> 
> This new code should be equivalent if I didn't make any logical fallacy. :3

Does it have to be copied? Can we delete the old code?

[trchen, 2015-06-23 21:44:08]

On 2015/06/23 21:40:51, chrishtr wrote:
> On 2015/06/23 at 20:40:16, trchen wrote:
> >
>
https://codereview.chromium.org/1196223008/diff/1/Source/core/paint/ViewPaint...
> > File Source/core/paint/ViewPainter.cpp (right):
> > 
> >
>
https://codereview.chromium.org/1196223008/diff/1/Source/core/paint/ViewPaint...
> > Source/core/paint/ViewPainter.cpp:67: if (document.printing() &&
> m_layoutView.style()->printColorAdjust() == PrintColorAdjustEconomy
> > On 2015/06/23 17:42:57, chrishtr wrote:
> > > Where did this logic used to be before your refactor?
> > 
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
> > 
> > This new code should be equivalent if I didn't make any logical fallacy. :3
> 
> Does it have to be copied? Can we delete the old code?

The economy mode affect normal elements too. IMO
BoxPainter::paintFillLayerExtended() is the wrong place though. It should really
be done in BoxPainter::paintBackground().

[chrishtr, 2015-06-23 21:51:08]

On 2015/06/23 at 21:44:08, trchen wrote:
> On 2015/06/23 21:40:51, chrishtr wrote:
> > On 2015/06/23 at 20:40:16, trchen wrote:
> > >
> >
https://codereview.chromium.org/1196223008/diff/1/Source/core/paint/ViewPaint...
> > > File Source/core/paint/ViewPainter.cpp (right):
> > > 
> > >
> >
https://codereview.chromium.org/1196223008/diff/1/Source/core/paint/ViewPaint...
> > > Source/core/paint/ViewPainter.cpp:67: if (document.printing() &&
> > m_layoutView.style()->printColorAdjust() == PrintColorAdjustEconomy
> > > On 2015/06/23 17:42:57, chrishtr wrote:
> > > > Where did this logic used to be before your refactor?
> > > 
> > >
> >
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
> > > 
> > > This new code should be equivalent if I didn't make any logical fallacy.
:3
> > 
> > Does it have to be copied? Can we delete the old code?
> 
> The economy mode affect normal elements too. IMO
BoxPainter::paintFillLayerExtended() is the wrong place though. It should really
be done in BoxPainter::paintBackground().

It would be nice to not duplicate the code also. Is that straightforward?

[trchen, 2015-06-23 22:13:04]

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

[trchen, 2015-06-23 22:13:45]

On 2015/06/23 21:51:08, chrishtr wrote:
> On 2015/06/23 at 21:44:08, trchen wrote:
> > On 2015/06/23 21:40:51, chrishtr wrote:
> > > On 2015/06/23 at 20:40:16, trchen wrote:
> > > >
> > >
>
https://codereview.chromium.org/1196223008/diff/1/Source/core/paint/ViewPaint...
> > > > File Source/core/paint/ViewPainter.cpp (right):
> > > > 
> > > >
> > >
>
https://codereview.chromium.org/1196223008/diff/1/Source/core/paint/ViewPaint...
> > > > Source/core/paint/ViewPainter.cpp:67: if (document.printing() &&
> > > m_layoutView.style()->printColorAdjust() == PrintColorAdjustEconomy
> > > > On 2015/06/23 17:42:57, chrishtr wrote:
> > > > > Where did this logic used to be before your refactor?
> > > > 
> > > >
> > >
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
> > > > 
> > > > This new code should be equivalent if I didn't make any logical fallacy.
> :3
> > > 
> > > Does it have to be copied? Can we delete the old code?
> > 
> > The economy mode affect normal elements too. IMO
> BoxPainter::paintFillLayerExtended() is the wrong place though. It should
really
> be done in BoxPainter::paintBackground().
> 
> It would be nice to not duplicate the code also. Is that straightforward?

Deduped. :)

[commit-bot: I haz the power, 2015-06-23 22:14:02]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1196223008/20001

[trchen, 2015-06-23 22:14:46]

On 2015/06/23 22:13:45, trchen wrote:
> On 2015/06/23 21:51:08, chrishtr wrote:
> > On 2015/06/23 at 21:44:08, trchen wrote:
> > > On 2015/06/23 21:40:51, chrishtr wrote:
> > > > On 2015/06/23 at 20:40:16, trchen wrote:
> > > > >
> > > >
> >
>
https://codereview.chromium.org/1196223008/diff/1/Source/core/paint/ViewPaint...
> > > > > File Source/core/paint/ViewPainter.cpp (right):
> > > > > 
> > > > >
> > > >
> >
>
https://codereview.chromium.org/1196223008/diff/1/Source/core/paint/ViewPaint...
> > > > > Source/core/paint/ViewPainter.cpp:67: if (document.printing() &&
> > > > m_layoutView.style()->printColorAdjust() == PrintColorAdjustEconomy
> > > > > On 2015/06/23 17:42:57, chrishtr wrote:
> > > > > > Where did this logic used to be before your refactor?
> > > > > 
> > > > >
> > > >
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
> > > > > 
> > > > > This new code should be equivalent if I didn't make any logical
fallacy.
> > :3
> > > > 
> > > > Does it have to be copied? Can we delete the old code?
> > > 
> > > The economy mode affect normal elements too. IMO
> > BoxPainter::paintFillLayerExtended() is the wrong place though. It should
> really
> > be done in BoxPainter::paintBackground().
> > 
> > It would be nice to not duplicate the code also. Is that straightforward?
> 
> Deduped. :)

Oops, realized that I made a typo with the function name. One sec.

[trchen, 2015-06-23 23:54:18]

Patchset #3 (id:40001) has been deleted

[trchen, 2015-06-23 23:54:22]

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

[commit-bot: I haz the power, 2015-06-23 23:54:36]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1196223008/60001

[commit-bot: I haz the power, 2015-06-24 00:12:03]

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

[commit-bot: I haz the power, 2015-06-24 00:12:04]

Dry run: Exceeded global retry quota

[trchen, 2015-06-24 00:24:28]

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

[commit-bot: I haz the power, 2015-06-24 00:25:20]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1196223008/80001

[commit-bot: I haz the power, 2015-06-24 01:16:07]

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

[commit-bot: I haz the power, 2015-06-24 01:16:07]

Dry run: This issue passed the CQ dry run.

[trchen, 2015-06-24 22:10:31]

On 2015/06/24 01:16:07, commit-bot: I haz the power wrote:
> Dry run: This issue passed the CQ dry run.

BTW this CL is ready. :)

[chrishtr, 2015-06-24 22:13:20]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2015-06-24 22:13:21]

lgtm

https://codereview.chromium.org/1196223008/diff/80001/Source/core/paint/BoxPa...
File Source/core/paint/BoxPainter.h (right):

https://codereview.chromium.org/1196223008/diff/80001/Source/core/paint/BoxPa...
Source/core/paint/BoxPainter.h:46: static bool
shouldForceWhiteBackgroundForPrintEconomy(const ComputedStyle&, const
Document&);
Add a comment explaining what this is. Maybe an example?

[commit-bot: I haz the power, 2015-06-24 22:13:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1196223008/80001

[commit-bot: I haz the power, 2015-06-24 22:17:58]

Committed patchset #4 (id:80001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=197782