Do not push the reseted needs_show_scrollbars to active tree

This issue is caused by couple reasons:

1. aura overlay scrollbars are visible (opacity=1) at initial state.
2. page is a not scrollable, then become scrollable.
3. SetScrollable and ScrollbarAnimationController creation may come from
   separate commit and no promising order.

We use needs_show_scrollbars in layerImpl to indicate the layer need to call
SAC::DidRequestShowFromMainThread. If the SetScrollable call happens before SAC
and from separate commit then:

- first commit:

1. In TreeSynchronizer::PushLayerProperties, pending tree push to active tree
   SetScrollable, set needs_show_scrollbars because bounds change or scrollable
   flag change. then reset needs_show_scrollbars in the layer of pending tree.
   [1]
2. In HandleScrollbarShowRequestsFromMain, skip because SAC not found.

- second commit:

1. In TS::PushLayerProperties, pending tree set the needs_show_scrollbars(false)
   to active tree, then call SetScrollable and quick return because no bounds
   change or scrollable flag change.
2. In HandleScrollbarShowRequestsFromMain, skip because needs_show_scrollbars is
   false.

In this patch, we only push needs_show_scrollbars to active tree when
needs_show_scrollbars is true. So we do not use the reseted value to override
the value we keep for next pick up.

[1] https://chromium.googlesource.com/chromium/src/+/a7c2ee628552ec6a9d624dcd1b78442991615715/cc/layers/layer_impl.cc#329

Bug: 740795
Change-Id: I15089ba357a67b3336bb251ce19cb2f7bf5953ba
Reviewed-on: https://chromium-review.googlesource.com/568815Reviewed-by: Weiliang Chen <weiliangc@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Commit-Queue: Jianpeng Chao <chaopeng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487089}

cc/layers/layer_impl.cc

@@ -326,7 +326,8 @@ void LayerImpl::PushPropertiesTo(LayerImpl* layer) {
   layer->scroll_tree_index_ = scroll_tree_index_;
   layer->has_will_change_transform_hint_ = has_will_change_transform_hint_;
   layer->scrollbars_hidden_ = scrollbars_hidden_;
-  layer->needs_show_scrollbars_ = needs_show_scrollbars_;
+  if (needs_show_scrollbars_)
+    layer->needs_show_scrollbars_ = needs_show_scrollbars_;
 
   if (layer_property_changed_) {
     layer->layer_tree_impl()->set_needs_update_draw_properties();

cc/layers/scrollbar_layer_unittest.cc

@@ -100,13 +100,13 @@ class FakeResourceTrackingUIResourceManager : public UIResourceManager {
   int total_ui_resource_deleted_;
 };
 
-class ScrollbarLayerTest : public testing::Test {
+class BaseScrollbarLayerTest : public testing::Test {
  public:
-  ScrollbarLayerTest() {
+  explicit BaseScrollbarLayerTest(
+      LayerTreeSettings::ScrollbarAnimator animator) {
     layer_tree_settings_.single_thread_proxy_scheduler = false;
     layer_tree_settings_.use_zero_copy = true;
-    layer_tree_settings_.scrollbar_animator =
-        LayerTreeSettings::ANDROID_OVERLAY;
+    layer_tree_settings_.scrollbar_animator = animator;
     layer_tree_settings_.scrollbar_fade_delay =
         base::TimeDelta::FromMilliseconds(20);
     layer_tree_settings_.scrollbar_fade_duration =
@@ -175,6 +175,18 @@ class ScrollbarLayerTest : public testing::Test {
   int scrollbar_layer_id_;
 };
 
+class ScrollbarLayerTest : public BaseScrollbarLayerTest {
+ public:
+  ScrollbarLayerTest()
+      : BaseScrollbarLayerTest(LayerTreeSettings::ANDROID_OVERLAY) {}
+};
+
+class AuraScrollbarLayerTest : public BaseScrollbarLayerTest {
+ public:
+  AuraScrollbarLayerTest()
+      : BaseScrollbarLayerTest(LayerTreeSettings::AURA_OVERLAY) {}
+};
+
 class FakePaintedOverlayScrollbar : public FakeScrollbar {
  public:
   FakePaintedOverlayScrollbar() : FakeScrollbar(true, true, true) {}
@@ -856,6 +868,55 @@ TEST_F(ScrollbarLayerTest, LayerChangesAffectingScrollbarGeometries) {
       !impl.host_impl()->active_tree()->ScrollbarGeometriesNeedUpdate());
 }
 
+TEST_F(AuraScrollbarLayerTest, ScrollbarLayerCreateAfterSetScrollable) {
+  // Scrollbar Layer can be created after SetScrollable is called and in a
+  // separate commit. Ensure we do not missing the DidRequestShowFromMainThread
+  // call.
+  const int kThumbThickness = 3;
+  const int kTrackStart = 0;
+
+  scoped_refptr<Layer> layer_tree_root = Layer::Create();
+  scoped_refptr<Layer> scroll_layer = Layer::Create();
+  scroll_layer->SetElementId(LayerIdToElementIdForTesting(scroll_layer->id()));
+  scoped_refptr<Layer> child1 = Layer::Create();
+  const bool kIsLeftSideVerticalScrollbar = false;
+
+  scroll_layer->AddChild(child1);
+  layer_tree_root->AddChild(scroll_layer);
+  layer_tree_host_->SetRootLayer(layer_tree_root);
+
+  layer_tree_root->SetBounds(gfx::Size(2, 2));
+  scroll_layer->SetBounds(gfx::Size(10, 10));
+  scroll_layer->SetScrollable(layer_tree_root->bounds());
+  layer_tree_host_->UpdateLayers();
+  LayerTreeHostImpl* host_impl = layer_tree_host_->host_impl();
+  host_impl->CreatePendingTree();
+  layer_tree_host_->CommitAndCreatePendingTree();
+  host_impl->ActivateSyncTree();
+
+  LayerImpl* scroll_layer_impl =
+      host_impl->active_tree()->LayerByElementId(scroll_layer->element_id());
+  EXPECT_TRUE(scroll_layer_impl->needs_show_scrollbars());
+
+  std::unique_ptr<Scrollbar> scrollbar(new FakeScrollbar(false, true, true));
+  scoped_refptr<Layer> scrollbar_layer = SolidColorScrollbarLayer::Create(
+      scrollbar->Orientation(), kThumbThickness, kTrackStart,
+      kIsLeftSideVerticalScrollbar, scroll_layer->element_id());
+  scroll_layer->InsertChild(scrollbar_layer, 1);
+
+  layer_tree_host_->UpdateLayers();
+  host_impl->CreatePendingTree();
+  layer_tree_host_->CommitAndCreatePendingTree();
+  host_impl->ActivateSyncTree();
+
+  EXPECT_TRUE(host_impl->ScrollbarAnimationControllerForElementId(
+      scroll_layer->element_id()));
+  EffectNode* node =
+      host_impl->active_tree()->property_trees()->effect_tree.Node(
+          scrollbar_layer->effect_tree_index());
+  EXPECT_EQ(node->opacity, 1.f);
+}
+
 class ScrollbarLayerSolidColorThumbTest : public testing::Test {
  public:
   ScrollbarLayerSolidColorThumbTest() {