LayerTreeHostRemote doesn't build property tree |
||
Issue descriptionIn layer_tree_host_unittest.cc, some test cases will compare property tree states between LTH remote and LTH impl, these will fail since there is no property tree in LTH remote. We probably want to figure out if we can modify code to enable those tests for LTH remote.
,
Oct 14 2016
So there can be a problem in 2 cases, if they end up comparing data in the PropertyTrees itself from the main copy to pending/active copy. Or if they end up comparing data set as a result of building PropertyTrees, for instance |transform_tree_index_| on the Layer with a LayerImpl, since the Layer on the engine will never have correct values for these. We should make a note of which tests have this problem here and then we can come back to seeing if there is a clean strategy to try to enable them. LayerTreeHostTestPushNodeOwnerToNodeIdMap -> compares tree indexes from Layers.
,
Oct 24 2016
Also some cc::Layer subclass uses property tree in the main thread, which means the existence of property tree might be an underlying assumption. This issue will also cause the test crash since LTH remote doesn't build the property tree. Such as PaintedScrollbarLayer, PaintedScrollbarLayer::Update will use the transform tree.
,
Oct 24 2016
We don't need to worry about tests that have PaintedScrollbar layers. They are disabled in blimp. We should only use solid color scrollbar layers. Though I ran into issues with blink not expecting cc to pull display lists for layers that were not drawable, at GraphicsLayer::getPaintController. So I'll look into building PropertyTrees on the engine side just for draw_property_utils::FindLayersThatNeedUpdates and so there are no other surprises. They still shouldn't need to be sent to the client.
,
Oct 28 2016
This raised another issue in test case LayerTreeHostTestLayersPushProperties. (when num_commits_ is 12) In normal LTH in process flow, property tree will be build and |subtree_property_changed_| will get propagated recursively through layer tree hierarchy. During the recursion, when a layer's |subtree_property_changed_| changed from false to true, it will call SetNeedsPushProperties. In LTH remote, we don't build property tree on the engine, so |subtree_property_changed_| only will get propagate on the LTH in process on the client side, but the test is verifying the engine layer tree so the states will be incorrect. Also we don't clear |subtree_property_changed_| in Layer::ToLayerPropertiesProto. Changing this will cause another unit test to fail. ------------------------------------------------------------------------------- Call stack where building property tree propagates |subtree_property_changed_| and trigger SetNeedsPushProperties. #2 0x000001076c61 cc::PushPropertiesCountingLayer::SetNeedsPushProperties() #3 0x7f5d9fdebc5e cc::Layer::SetSubtreePropertyChanged() #4 0x7f5da00fa79f cc::(anonymous namespace)::SetLayerPropertyChangedForChild() #5 0x7f5da00f70e2 cc::(anonymous namespace)::BuildPropertyTreesInternal<>() #6 0x7f5da00f711e cc::(anonymous namespace)::BuildPropertyTreesInternal<>() #7 0x7f5da0100cd3 cc::BuildPropertyTreesTopLevelInternal<>() #8 0x7f5da00f67ce cc::PropertyTreeBuilder::BuildPropertyTrees() #9 0x7f5da00967ff cc::LayerTreeHostInProcess::DoUpdateLayers()
,
Oct 28 2016
Also on test case LayerTreeHostTestImplLayersPushProperties. Since remote LTH doesn't build property tree, the layers need to push properties on client and engine are actually different, so it doesn't make sense to compare the push properties count between engine and client(both main thread and impl thread tree).
,
Nov 30 2016
|
||
►
Sign in to add a comment |
||
Comment 1 by khushals...@chromium.org
, Oct 13 2016