Remove WebGraphicsContext/WebGraphicsContextImpl

Remove WebGraphicsContext/WebGraphicsContextImpl

We used it to pass GraphicsContext from PageOverlay to
PageOverlay::Delegete implementors. Pass GraphicsContext directly.

Skip cache in InspectorOverlay because view painted for the overlay
may conflict with the view's real painting.

BUG=536999
R=chrishtr@chromium.org
TBR=dpranke@chromium.org (remove an extra include from src/components/test_runner.cc)

Committed: https://crrev.com/bc449085148e77c472250047afce48ca8f0aa46d
Cr-Commit-Position: refs/heads/master@{#357148}

[Xianzhu, 2015-10-30 00:00:17]

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

[chrishtr, 2015-10-30 00:17:41]

The CQ bit was checked by chrishtr@chromium.org

[chrishtr, 2015-10-30 00:17:41]

lgtm

[commit-bot: I haz the power, 2015-10-30 00:18:23]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1412593003/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1412593003/1

[commit-bot: I haz the power, 2015-10-30 00:30:11]

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

[commit-bot: I haz the power, 2015-10-30 00:30:12]

Try jobs failed on following builders:
  linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Xianzhu, 2015-10-30 15:38:53]

The CQ bit was checked by wangxianzhu@chromium.org

[Xianzhu, 2015-10-30 15:38:54]

The patchset sent to the CQ was uploaded after l-g-t-m from
chrishtr@chromium.org
Link to the patchset: https://codereview.chromium.org/1412593003/#ps20001
(title: "Remove another include WebGraphicsContext.h")

[commit-bot: I haz the power, 2015-10-30 15:39:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1412593003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1412593003/20001

[commit-bot: I haz the power, 2015-10-30 15:48:50]

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

[commit-bot: I haz the power, 2015-10-30 15:48:51]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Xianzhu, 2015-10-30 15:49:46]

Description was changed from

==========
Remove WebGraphicsContext/WebGraphicsContextImpl

We used it to pass GraphicsContext from PageOverlay to
PageOverlay::Delegete implementors. Pass GraphicsContext directly.

Skip cache in InspectorOverlay because view painted for the overlay
may conflict with the view's real painting.

BUG=536999
==========

to

==========
Remove WebGraphicsContext/WebGraphicsContextImpl

We used it to pass GraphicsContext from PageOverlay to
PageOverlay::Delegete implementors. Pass GraphicsContext directly.

Skip cache in InspectorOverlay because view painted for the overlay
may conflict with the view's real painting.

BUG=536999
R=chrishtr@chromium.org
TBR=dpranke@chromium.org (remove an extra include from
src/components/test_runner.cc)
==========

[Xianzhu, 2015-10-30 15:49:51]

The CQ bit was checked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2015-10-30 15:50:23]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1412593003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1412593003/20001

[commit-bot: I haz the power, 2015-10-30 18:42:58]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-10-30 18:44:53]

Patchset 2 (id:??) landed as
https://crrev.com/bc449085148e77c472250047afce48ca8f0aa46d
Cr-Commit-Position: refs/heads/master@{#357148}

[chrishtr, 2016-05-05 16:10:44]

https://codereview.chromium.org/1412593003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/InspectorOverlay.cpp (right):

https://codereview.chromium.org/1412593003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/InspectorOverlay.cpp:118: DisplayItemCacheSkipper
cacheSkipper(graphicsContext);
I'm encountering this again in the context of investigation into issue 592915.
This is the only
place other than BoxPainter (for painting theme content) that this is used, so
I'm looking into
getting rid of both.

Why is this necessary again? Why would the PaintController for this overlay be
shared with the
main page?

[Xianzhu, 2016-05-05 16:27:52]

https://codereview.chromium.org/1412593003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/InspectorOverlay.cpp (right):

https://codereview.chromium.org/1412593003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/InspectorOverlay.cpp:118: DisplayItemCacheSkipper
cacheSkipper(graphicsContext);
On 2016/05/05 16:10:43, chrishtr wrote:
> I'm encountering this again in the context of investigation into issue 592915.
> This is the only
> place other than BoxPainter (for painting theme content) that this is used, so
> I'm looking into
> getting rid of both.
> 
> Why is this necessary again? Why would the PaintController for this overlay be
> shared with the
> main page?

The comment seems a bit misleading. The real reason of cache skipper here is
that we never invalidate display items in this paint controller when the display
items are invalidated, so we must assume that all display items are invalid.
Another choice (which seems more correct) is to clear the paint controller
before painting in PageOverlay::paintContents().

[chrishtr, 2016-05-05 16:30:08]

https://codereview.chromium.org/1412593003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/InspectorOverlay.cpp (right):

https://codereview.chromium.org/1412593003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/InspectorOverlay.cpp:118: DisplayItemCacheSkipper
cacheSkipper(graphicsContext);
On 2016/05/05 at 16:27:52, Xianzhu wrote:
> On 2016/05/05 16:10:43, chrishtr wrote:
> > I'm encountering this again in the context of investigation into issue
592915.
> > This is the only
> > place other than BoxPainter (for painting theme content) that this is used,
so
> > I'm looking into
> > getting rid of both.
> > 
> > Why is this necessary again? Why would the PaintController for this overlay
be
> > shared with the
> > main page?
> 
> The comment seems a bit misleading. The real reason of cache skipper here is
that we never invalidate display items in this paint controller when the display
items are invalidated, so we must assume that all display items are invalid.
Another choice (which seems more correct) is to clear the paint controller
before painting in PageOverlay::paintContents().

But where does this PaintController come from? Isn't it new'ed up on every paint
in these cases?

[Xianzhu, 2016-05-05 16:47:33]

https://codereview.chromium.org/1412593003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/InspectorOverlay.cpp (right):

https://codereview.chromium.org/1412593003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/InspectorOverlay.cpp:118: DisplayItemCacheSkipper
cacheSkipper(graphicsContext);
On 2016/05/05 16:30:07, chrishtr wrote:
> On 2016/05/05 at 16:27:52, Xianzhu wrote:
> > On 2016/05/05 16:10:43, chrishtr wrote:
> > > I'm encountering this again in the context of investigation into issue
> 592915.
> > > This is the only
> > > place other than BoxPainter (for painting theme content) that this is
used,
> so
> > > I'm looking into
> > > getting rid of both.
> > > 
> > > Why is this necessary again? Why would the PaintController for this
overlay
> be
> > > shared with the
> > > main page?
> > 
> > The comment seems a bit misleading. The real reason of cache skipper here is
> that we never invalidate display items in this paint controller when the
display
> items are invalidated, so we must assume that all display items are invalid.
> Another choice (which seems more correct) is to clear the paint controller
> before painting in PageOverlay::paintContents().
> 
> But where does this PaintController come from? Isn't it new'ed up on every
paint
> in these cases?

Read more code and I found I misunderstood InspectorOverlay. I thought it
painted the inspected frame in some way, but actually it paints
InspectorOverlayPage.html, so this won't conflict with the inspected frame, and
the cache skipper should not be needed.

[chrishtr, 2016-05-05 16:53:05]

https://codereview.chromium.org/1412593003/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/InspectorOverlay.cpp (right):

https://codereview.chromium.org/1412593003/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/InspectorOverlay.cpp:118: DisplayItemCacheSkipper
cacheSkipper(graphicsContext);
On 2016/05/05 at 16:47:33, Xianzhu wrote:
> On 2016/05/05 16:30:07, chrishtr wrote:
> > On 2016/05/05 at 16:27:52, Xianzhu wrote:
> > > On 2016/05/05 16:10:43, chrishtr wrote:
> > > > I'm encountering this again in the context of investigation into issue
> > 592915.
> > > > This is the only
> > > > place other than BoxPainter (for painting theme content) that this is
used,
> > so
> > > > I'm looking into
> > > > getting rid of both.
> > > > 
> > > > Why is this necessary again? Why would the PaintController for this
overlay
> > be
> > > > shared with the
> > > > main page?
> > > 
> > > The comment seems a bit misleading. The real reason of cache skipper here
is
> > that we never invalidate display items in this paint controller when the
display
> > items are invalidated, so we must assume that all display items are invalid.
> > Another choice (which seems more correct) is to clear the paint controller
> > before painting in PageOverlay::paintContents().
> > 
> > But where does this PaintController come from? Isn't it new'ed up on every
paint
> > in these cases?
> 
> Read more code and I found I misunderstood InspectorOverlay. I thought it
painted the inspected frame in some way, but actually it paints
InspectorOverlayPage.html, so this won't conflict with the inspected frame, and
the cache skipper should not be needed.

Ok I'll put up a patch to remove this then.