Plumb CompositedDisplayList to WebViewImpl

Plumb CompositedDisplayList to WebViewImpl

This patch sets up the scaffolding for plumbing the new
composited display list to the compositor.

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

[pdr., 2015-08-19 17:58:51]

pdr@chromium.org changed reviewers:
+ enne@chromium.org, jbroman@chromium.org

[pdr., 2015-08-19 17:59:01]

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

[commit-bot: I haz the power, 2015-08-19 17:59:16]

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

[commit-bot: I haz the power, 2015-08-19 18:29:34]

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

[commit-bot: I haz the power, 2015-08-19 18:29:34]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[jbroman, 2015-08-19 18:33:50]

Seems reasonable to me.

https://codereview.chromium.org/1301033002/diff/20001/Source/web/WebViewImpl.h
File Source/web/WebViewImpl.h (right):

https://codereview.chromium.org/1301033002/diff/20001/Source/web/WebViewImpl....
Source/web/WebViewImpl.h:746: OwnPtr<WebCompositedDisplayList>
m_compositedDisplayList;
Nit: we could consider making WebCompositedDisplayList a member of WebViewImpl,
since it is itself already basically an OwnPtr<CompositedDisplayList>
internally. Then you'd forward the PassOwnPtr<CompositedDisplayList> all the way
into WebViewImpl, where you would do
m_compositedDisplayList.assign(passCompositedDisplayList) or similar.

i.e. here:
    WebCompositedDisplayList m_compositedDisplayList;
in the impl:
    void WebViewImpl::setCompositedDisplayList(PassOwnPtr<CompositedDisplayList>
compositedDisplayList)
    {
        m_compositedDisplayList.assign(compositedDisplayList);
    }
    WebCompositedDisplayList* WebViewImpl::compositedDisplayList() { return
&m_compositedDisplayList; }

I don't feel very strongly about this instance, though.

https://codereview.chromium.org/1301033002/diff/20001/public/web/WebView.h
File public/web/WebView.h (right):

https://codereview.chromium.org/1301033002/diff/20001/public/web/WebView.h#ne...
public/web/WebView.h:458: virtual void
setCompositedDisplayList(WebCompositedDisplayList*) { }
Will the embedder ever call this? If not, suggest making this take
PassOwnPtr<WebCompositedDisplayList> and making it a concrete method on
WebViewImpl only.

[pdr., 2015-08-20 04:01:31]

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

[commit-bot: I haz the power, 2015-08-20 04:01:34]

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

[pdr., 2015-08-20 04:01:55]

PTAL

https://codereview.chromium.org/1301033002/diff/20001/Source/web/WebViewImpl.h
File Source/web/WebViewImpl.h (right):

https://codereview.chromium.org/1301033002/diff/20001/Source/web/WebViewImpl....
Source/web/WebViewImpl.h:746: OwnPtr<WebCompositedDisplayList>
m_compositedDisplayList;
On 2015/08/19 at 18:33:50, jbroman wrote:
> Nit: we could consider making WebCompositedDisplayList a member of
WebViewImpl, since it is itself already basically an
OwnPtr<CompositedDisplayList> internally. Then you'd forward the
PassOwnPtr<CompositedDisplayList> all the way into WebViewImpl, where you would
do m_compositedDisplayList.assign(passCompositedDisplayList) or similar.
> 
> i.e. here:
>     WebCompositedDisplayList m_compositedDisplayList;
> in the impl:
>     void
WebViewImpl::setCompositedDisplayList(PassOwnPtr<CompositedDisplayList>
compositedDisplayList)
>     {
>         m_compositedDisplayList.assign(compositedDisplayList);
>     }
>     WebCompositedDisplayList* WebViewImpl::compositedDisplayList() { return
&m_compositedDisplayList; }
> 
> I don't feel very strongly about this instance, though.

This worked out nicely since it moves the WebCDL <-> CDL logic up a bit and out
of Page.

https://codereview.chromium.org/1301033002/diff/20001/public/web/WebView.h
File public/web/WebView.h (right):

https://codereview.chromium.org/1301033002/diff/20001/public/web/WebView.h#ne...
public/web/WebView.h:458: virtual void
setCompositedDisplayList(WebCompositedDisplayList*) { }
On 2015/08/19 at 18:33:50, jbroman wrote:
> Will the embedder ever call this? If not, suggest making this take
PassOwnPtr<WebCompositedDisplayList> and making it a concrete method on
WebViewImpl only.

Good idea. Done.

[commit-bot: I haz the power, 2015-08-20 04:25:20]

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

[commit-bot: I haz the power, 2015-08-20 04:25:20]

Dry run: Try jobs failed on following builders:
  chromeos_x86-generic_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)

[pdr., 2015-08-20 18:16:59]

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

[commit-bot: I haz the power, 2015-08-20 18:17:24]

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

[pdr., 2015-08-20 18:17:24]

On 2015/08/20 at 04:25:20, commit-bot wrote:
> Dry run: Try jobs failed on following builders:
>   chromeos_x86-generic_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)

These failures were flakes, kicking off another dry run.

[commit-bot: I haz the power, 2015-08-20 18:41:28]

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

[commit-bot: I haz the power, 2015-08-20 18:41:29]

Dry run: This issue passed the CQ dry run.

[jbroman, 2015-08-20 20:24:47]

lima golf tango mike









lgtm

https://codereview.chromium.org/1301033002/diff/80001/public/platform/WebComp...
File public/platform/WebCompositedDisplayList.h (right):

https://codereview.chromium.org/1301033002/diff/80001/public/platform/WebComp...
public/platform/WebCompositedDisplayList.h:23: ~WebCompositedDisplayList();
super-nit: please move the destructor above member functions

(bonus: this lets you combine the INSIDE_BLINK blocks)

[pdr., 2015-08-20 20:36:30]

On 2015/08/20 at 20:24:47, jbroman wrote:
> lima golf tango mike
> 
> 
> 
> 
> 
> 
> 
> 
> 
> lgtm
> 
>
https://codereview.chromium.org/1301033002/diff/80001/public/platform/WebComp...
> File public/platform/WebCompositedDisplayList.h (right):
> 
>
https://codereview.chromium.org/1301033002/diff/80001/public/platform/WebComp...
> public/platform/WebCompositedDisplayList.h:23: ~WebCompositedDisplayList();
> super-nit: please move the destructor above member functions
> 
> (bonus: this lets you combine the INSIDE_BLINK blocks)

Bonus: done.

Patch will now continue the descent into tree.. this analogy could use some
improvements.

[pdr., 2015-08-20 20:36:32]

The CQ bit was checked by pdr@chromium.org

[pdr., 2015-08-20 20:36:33]

The patchset sent to the CQ was uploaded after l-g-t-m from jbroman@chromium.org
Link to the patchset: https://codereview.chromium.org/1301033002/#ps100001
(title: "Final checklist: cleanup ordering in WebCompositedDisplayList")

[commit-bot: I haz the power, 2015-08-20 20:36:57]

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

[commit-bot: I haz the power, 2015-08-20 23:08:57]

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

[commit-bot: I haz the power, 2015-08-20 23:08:58]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[pdr., 2015-08-20 23:36:24]

The CQ bit was checked by pdr@chromium.org

[commit-bot: I haz the power, 2015-08-20 23:37:08]

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

[commit-bot: I haz the power, 2015-08-21 00:34:42]

Committed patchset #6 (id:100001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=200945