cc: Merge LayerTree into the LayerTreeHost.

cc: Merge LayerTree into the LayerTreeHost.

The class was split so the code could be shared with the LayerTreeHost
implementation in blimp. Since this is no longer necessary, this merges
the LayerTree back into the host.

BUG=682436
TBR=piman@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2661523003
Cr-Commit-Position: refs/heads/master@{#446791}
Committed: https://chromium.googlesource.com/chromium/src/+/b69ba945a0cf9cb00e9fe4bec3037d4255a60902

[Khushal, 2017-01-27 02:43:50]

Description was changed from

==========
cc: Merge LayerTree into the LayerTreeHost.

The class was split so the code could be shared with the LayerTreeHost
implementation in blimp. Since this is no longer necessary, this merges
the LayerTree back into the host.

BUG=682436
==========

to

==========
cc: Merge LayerTree into the LayerTreeHost.

The class was split so the code could be shared with the LayerTreeHost
implementation in blimp. Since this is no longer necessary, this merges
the LayerTree back into the host.

BUG=682436
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[Khushal, 2017-01-27 03:05:50]

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

[commit-bot: I haz the power, 2017-01-27 03:06:44]

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

[Khushal, 2017-01-27 03:07:52]

khushalsagar@chromium.org changed reviewers:
+ boliu@chromium.org, danakj@chromium.org, enne@chromium.org, pdr@chromium.org

[Khushal, 2017-01-27 03:07:52]

enne : cc/ and content/renderer/
danakj : cc/ and ui/
boliu : compositor_impl_android
pdr : Webkit/

[commit-bot: I haz the power, 2017-01-27 03:24:40]

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

[commit-bot: I haz the power, 2017-01-27 03:24:41]

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_...)

[Khushal, 2017-01-27 05:46:51]

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

[commit-bot: I haz the power, 2017-01-27 05:47:10]

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-01-27 07:08:29]

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

[commit-bot: I haz the power, 2017-01-27 07:08:30]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Khushal, 2017-01-27 07:26:50]

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

[commit-bot: I haz the power, 2017-01-27 07:27:09]

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-01-27 11:24:20]

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

[commit-bot: I haz the power, 2017-01-27 11:24:22]

Dry run: This issue passed the CQ dry run.

[danakj, 2017-01-27 17:25:09]

LGTM

https://codereview.chromium.org/2661523003/diff/60001/cc/layers/layer.h
File cc/layers/layer.h (right):

https://codereview.chromium.org/2661523003/diff/60001/cc/layers/layer.h#newco...
cc/layers/layer.h:442: LayerTreeHost* layer_tree_host() const { return
layer_tree_host_; }
There used to be this:

   LayerTreeHost* layer_tree_host() { return layer_tree_host_; }
   const LayerTreeHost* layer_tree_host() const { return layer_tree_host_; }

Is the const one never needed, there's no const Layers running around? If you
had to work around the missing const one anywhere can you add it here instead?

https://codereview.chromium.org/2661523003/diff/60001/cc/trees/layer_tree_host.h
File cc/trees/layer_tree_host.h (right):

https://codereview.chromium.org/2661523003/diff/60001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host.h:82: using LayerSet = std::unordered_set<Layer*>;
These types don't seem onerous to type out, can we drop the aliases?

https://codereview.chromium.org/2661523003/diff/60001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host.h:510: int source_frame_number_ = 0U;
 = 0 (its signed int)

https://codereview.chromium.org/2661523003/diff/60001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host.h:597: using ElementLayersMap =
std::unordered_map<ElementId, Layer*, ElementIdHash>;
same here

[boliu, 2017-01-27 18:18:41]

lgtm

[pdr., 2017-01-27 18:22:23]

This cleanup is looking great. WebKit/ LGTM

[enne (OOO), 2017-01-27 18:34:52]

lgtm

[Khushal, 2017-01-27 18:52:18]

khushalsagar@chromium.org changed reviewers:
+ piman@chromium.org

[Khushal, 2017-01-27 18:52:19]

https://codereview.chromium.org/2661523003/diff/60001/cc/layers/layer.h
File cc/layers/layer.h (right):

https://codereview.chromium.org/2661523003/diff/60001/cc/layers/layer.h#newco...
cc/layers/layer.h:442: LayerTreeHost* layer_tree_host() const { return
layer_tree_host_; }
On 2017/01/27 17:25:09, danakj (slow) wrote:
> There used to be this:
> 
>    LayerTreeHost* layer_tree_host() { return layer_tree_host_; }
>    const LayerTreeHost* layer_tree_host() const { return layer_tree_host_; }
> 
> Is the const one never needed, there's no const Layers running around? If you
> had to work around the missing const one anywhere can you add it here instead?

Nah, I don't think the const one is used anymore. I just added this one and no
errors.

https://codereview.chromium.org/2661523003/diff/60001/cc/trees/layer_tree_host.h
File cc/trees/layer_tree_host.h (right):

https://codereview.chromium.org/2661523003/diff/60001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host.h:82: using LayerSet = std::unordered_set<Layer*>;
On 2017/01/27 17:25:09, danakj (slow) wrote:
> These types don't seem onerous to type out, can we drop the aliases?

There is the place where we use the iterator and it looks unwieldy to write the
whole thing there. Dropped the set and made the map private.

https://codereview.chromium.org/2661523003/diff/60001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host.h:510: int source_frame_number_ = 0U;
On 2017/01/27 17:25:09, danakj (slow) wrote:
>  = 0 (its signed int)

Done.

https://codereview.chromium.org/2661523003/diff/60001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host.h:597: using ElementLayersMap =
std::unordered_map<ElementId, Layer*, ElementIdHash>;
On 2017/01/27 17:25:09, danakj (slow) wrote:
> same here

same as LayerIdMap.

[Khushal, 2017-01-27 18:52:37]

Description was changed from

==========
cc: Merge LayerTree into the LayerTreeHost.

The class was split so the code could be shared with the LayerTreeHost
implementation in blimp. Since this is no longer necessary, this merges
the LayerTree back into the host.

BUG=682436
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
cc: Merge LayerTree into the LayerTreeHost.

The class was split so the code could be shared with the LayerTreeHost
implementation in blimp. Since this is no longer necessary, this merges
the LayerTree back into the host.

BUG=682436
TBR=piman@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[Khushal, 2017-01-27 18:53:10]

TBRing piman for the 2 test files. Trivial changes.

[Khushal, 2017-01-27 18:53:18]

The CQ bit was checked by khushalsagar@chromium.org

[Khushal, 2017-01-27 18:53:19]

The patchset sent to the CQ was uploaded after l-g-t-m from danakj@chromium.org,
pdr@chromium.org, boliu@chromium.org, enne@chromium.org
Link to the patchset: https://codereview.chromium.org/2661523003/#ps80001
(title: "dana's comments.")

[commit-bot: I haz the power, 2017-01-27 18:53:47]

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

[danakj, 2017-01-27 19:07:08]

https://codereview.chromium.org/2661523003/diff/60001/cc/trees/layer_tree_host.h
File cc/trees/layer_tree_host.h (right):

https://codereview.chromium.org/2661523003/diff/60001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host.h:82: using LayerSet = std::unordered_set<Layer*>;
On 2017/01/27 18:52:19, Khushal wrote:
> On 2017/01/27 17:25:09, danakj (slow) wrote:
> > These types don't seem onerous to type out, can we drop the aliases?
> 
> There is the place where we use the iterator and it looks unwieldy to write
the
> whole thing there. Dropped the set and made the map private.

That should use auto :)

[Khushal, 2017-01-27 19:10:05]

The CQ bit was unchecked by khushalsagar@chromium.org

[Khushal, 2017-01-27 19:24:16]

The CQ bit was checked by khushalsagar@chromium.org

[Khushal, 2017-01-27 19:24:17]

The patchset sent to the CQ was uploaded after l-g-t-m from danakj@chromium.org,
pdr@chromium.org, enne@chromium.org, boliu@chromium.org
Link to the patchset: https://codereview.chromium.org/2661523003/#ps100001
(title: "auto")

[Khushal, 2017-01-27 19:24:40]

https://codereview.chromium.org/2661523003/diff/60001/cc/trees/layer_tree_host.h
File cc/trees/layer_tree_host.h (right):

https://codereview.chromium.org/2661523003/diff/60001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host.h:82: using LayerSet = std::unordered_set<Layer*>;
On 2017/01/27 19:07:08, danakj (slow) wrote:
> On 2017/01/27 18:52:19, Khushal wrote:
> > On 2017/01/27 17:25:09, danakj (slow) wrote:
> > > These types don't seem onerous to type out, can we drop the aliases?
> > 
> > There is the place where we use the iterator and it looks unwieldy to write
> the
> > whole thing there. Dropped the set and made the map private.
> 
> That should use auto :)

Doh. Done. :P

[commit-bot: I haz the power, 2017-01-27 19:24:46]

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-01-27 22:20:11]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1485545056318800,
"parent_rev": "f998a5e1f13847384ddc3574d549b2759fcca04c", "commit_rev":
"b69ba945a0cf9cb00e9fe4bec3037d4255a60902"}

[commit-bot: I haz the power, 2017-01-27 22:20:42]

Description was changed from

==========
cc: Merge LayerTree into the LayerTreeHost.

The class was split so the code could be shared with the LayerTreeHost
implementation in blimp. Since this is no longer necessary, this merges
the LayerTree back into the host.

BUG=682436
TBR=piman@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
cc: Merge LayerTree into the LayerTreeHost.

The class was split so the code could be shared with the LayerTreeHost
implementation in blimp. Since this is no longer necessary, this merges
the LayerTree back into the host.

BUG=682436
TBR=piman@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

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

[commit-bot: I haz the power, 2017-01-27 22:20:43]

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