diff options
author | bokan <bokan@chromium.org> | 2015-03-09 14:51:32 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-03-09 21:52:28 +0000 |
commit | aa5d1b033c0894dfde32bfe0b1d2cd8b2d9a1eb3 (patch) | |
tree | 84d3803e89ce24dae61fbf7cba858e1d1b06995b /cc | |
parent | a0bf8e711f92b7567e6ce3d6faba56825d36c13b (diff) | |
download | chromium_src-aa5d1b033c0894dfde32bfe0b1d2cd8b2d9a1eb3.zip chromium_src-aa5d1b033c0894dfde32bfe0b1d2cd8b2d9a1eb3.tar.gz chromium_src-aa5d1b033c0894dfde32bfe0b1d2cd8b2d9a1eb3.tar.bz2 |
(Reland) Always create top controls manager.
This CL removes the calculate_top_controls_position flag and instead
always creates the top controls manager. The methods of the manager
become no-ops if the top controls height is 0 so that it becomes
effectively enabled when a top controls height gets set.
This fixes the attached bug because some Android Chrome UI pages don't
have top controls but the manager was still created. These pages set
the height to 0 to hide the top controls but the methods of the
manager were still getting called. This caused a crash in Blink since
ScrollBy would try to calculate the shown ratio with a 0 height, causing
a NaN to propagate into Blink.
BUG=460007
Committed: https://crrev.com/7610e74eacde3468853c9765bd3f726c14df04c1
Cr-Commit-Position: refs/heads/master@{#319257}
Review URL: https://codereview.chromium.org/961023002
Cr-Commit-Position: refs/heads/master@{#319738}
Diffstat (limited to 'cc')
-rw-r--r-- | cc/base/switches.cc | 3 | ||||
-rw-r--r-- | cc/base/switches.h | 1 | ||||
-rw-r--r-- | cc/input/top_controls_manager.cc | 9 | ||||
-rw-r--r-- | cc/input/top_controls_manager_unittest.cc | 3 | ||||
-rw-r--r-- | cc/trees/layer_tree_host.cc | 10 | ||||
-rw-r--r-- | cc/trees/layer_tree_host_impl.cc | 54 | ||||
-rw-r--r-- | cc/trees/layer_tree_host_impl_unittest.cc | 4 | ||||
-rw-r--r-- | cc/trees/layer_tree_settings.cc | 1 | ||||
-rw-r--r-- | cc/trees/layer_tree_settings.h | 1 |
9 files changed, 41 insertions, 45 deletions
diff --git a/cc/base/switches.cc b/cc/base/switches.cc index 504e787..ae42482 100644 --- a/cc/base/switches.cc +++ b/cc/base/switches.cc @@ -24,9 +24,6 @@ const char kDisableMainFrameBeforeActivation[] = const char kEnableMainFrameBeforeActivation[] = "enable-main-frame-before-activation"; -const char kEnableTopControlsPositionCalculation[] = - "enable-top-controls-position-calculation"; - // Percentage of the top controls need to be hidden before they will auto hide. const char kTopControlsHideThreshold[] = "top-controls-hide-threshold"; diff --git a/cc/base/switches.h b/cc/base/switches.h index 90caef8..46fe922 100644 --- a/cc/base/switches.h +++ b/cc/base/switches.h @@ -20,7 +20,6 @@ CC_EXPORT extern const char kDisableThreadedAnimation[]; CC_EXPORT extern const char kDisableCompositedAntialiasing[]; CC_EXPORT extern const char kDisableMainFrameBeforeActivation[]; CC_EXPORT extern const char kEnableMainFrameBeforeActivation[]; -CC_EXPORT extern const char kEnableTopControlsPositionCalculation[]; CC_EXPORT extern const char kJankInsteadOfCheckerboard[]; CC_EXPORT extern const char kTopControlsHideThreshold[]; CC_EXPORT extern const char kTopControlsShowThreshold[]; diff --git a/cc/input/top_controls_manager.cc b/cc/input/top_controls_manager.cc index 3ac3b4a..56c9d8f 100644 --- a/cc/input/top_controls_manager.cc +++ b/cc/input/top_controls_manager.cc @@ -101,6 +101,9 @@ void TopControlsManager::ScrollBegin() { gfx::Vector2dF TopControlsManager::ScrollBy( const gfx::Vector2dF& pending_delta) { + if (!TopControlsHeight()) + return pending_delta; + if (pinch_gesture_active_) return pending_delta; @@ -183,6 +186,12 @@ void TopControlsManager::SetupAnimation(AnimationDirection direction) { if (top_controls_animation_ && animation_direction_ == direction) return; + if (!TopControlsHeight()) { + client_->SetCurrentTopControlsShownRatio( + direction == HIDING_CONTROLS ? 0.f : 1.f); + return; + } + top_controls_animation_ = KeyframedFloatAnimationCurve::Create(); base::TimeDelta start_time = gfx::FrameTime::Now() - base::TimeTicks(); top_controls_animation_->AddKeyframe( diff --git a/cc/input/top_controls_manager_unittest.cc b/cc/input/top_controls_manager_unittest.cc index c78f5c0..4145cac 100644 --- a/cc/input/top_controls_manager_unittest.cc +++ b/cc/input/top_controls_manager_unittest.cc @@ -414,8 +414,9 @@ TEST(TopControlsManagerTest, HeightChangeMaintainsFullyVisibleControls) { TEST(TopControlsManagerTest, GrowingHeightKeepsTopControlsHidden) { MockTopControlsManagerClient client(0.f, 0.5f, 0.5f); TopControlsManager* manager = client.manager(); + client.SetTopControlsHeight(1.f); manager->UpdateTopControlsState(HIDDEN, HIDDEN, false); - EXPECT_EQ(0.f, manager->ControlsTopOffset()); + EXPECT_EQ(-1.f, manager->ControlsTopOffset()); EXPECT_EQ(0.f, manager->ContentTopOffset()); client.SetTopControlsHeight(50.f); diff --git a/cc/trees/layer_tree_host.cc b/cc/trees/layer_tree_host.cc index eca2311..5b656a8 100644 --- a/cc/trees/layer_tree_host.cc +++ b/cc/trees/layer_tree_host.cc @@ -448,11 +448,8 @@ scoped_ptr<LayerTreeHostImpl> LayerTreeHost::CreateLayerTreeHostImpl( host_impl->SetUseGpuRasterization(UseGpuRasterization()); shared_bitmap_manager_ = NULL; gpu_memory_buffer_manager_ = NULL; - if (settings_.calculate_top_controls_position && - host_impl->top_controls_manager()) { - top_controls_manager_weak_ptr_ = - host_impl->top_controls_manager()->AsWeakPtr(); - } + top_controls_manager_weak_ptr_ = + host_impl->top_controls_manager()->AsWeakPtr(); input_handler_weak_ptr_ = host_impl->AsWeakPtr(); return host_impl.Pass(); } @@ -1131,9 +1128,6 @@ void LayerTreeHost::SetDeviceScaleFactor(float device_scale_factor) { void LayerTreeHost::UpdateTopControlsState(TopControlsState constraints, TopControlsState current, bool animate) { - if (!settings_.calculate_top_controls_position) - return; - // Top controls are only used in threaded mode. proxy_->ImplThreadTaskRunner()->PostTask( FROM_HERE, diff --git a/cc/trees/layer_tree_host_impl.cc b/cc/trees/layer_tree_host_impl.cc index 9259b34..a5cdc02 100644 --- a/cc/trees/layer_tree_host_impl.cc +++ b/cc/trees/layer_tree_host_impl.cc @@ -241,12 +241,10 @@ LayerTreeHostImpl::LayerTreeHostImpl( TRACE_EVENT_OBJECT_CREATED_WITH_ID( TRACE_DISABLED_BY_DEFAULT("cc.debug"), "cc::LayerTreeHostImpl", id_); - if (settings.calculate_top_controls_position) { - top_controls_manager_ = - TopControlsManager::Create(this, - settings.top_controls_show_threshold, - settings.top_controls_hide_threshold); - } + top_controls_manager_ = + TopControlsManager::Create(this, + settings.top_controls_show_threshold, + settings.top_controls_hide_threshold); } LayerTreeHostImpl::~LayerTreeHostImpl() { @@ -916,8 +914,7 @@ DrawResult LayerTreeHostImpl::CalculateRenderPasses( } void LayerTreeHostImpl::MainThreadHasStoppedFlinging() { - if (top_controls_manager_) - top_controls_manager_->MainThreadHasStoppedFlinging(); + top_controls_manager_->MainThreadHasStoppedFlinging(); if (input_handler_client_) input_handler_client_->MainThreadHasStoppedFlinging(); } @@ -1423,12 +1420,10 @@ CompositorFrameMetadata LayerTreeHostImpl::MakeCompositorFrameMetadata() const { metadata.root_layer_size = active_tree_->ScrollableSize(); metadata.min_page_scale_factor = active_tree_->min_page_scale_factor(); metadata.max_page_scale_factor = active_tree_->max_page_scale_factor(); - if (top_controls_manager_) { - metadata.location_bar_offset = - gfx::Vector2dF(0.f, top_controls_manager_->ControlsTopOffset()); - metadata.location_bar_content_translation = - gfx::Vector2dF(0.f, top_controls_manager_->ContentTopOffset()); - } + metadata.location_bar_offset = + gfx::Vector2dF(0.f, top_controls_manager_->ControlsTopOffset()); + metadata.location_bar_content_translation = + gfx::Vector2dF(0.f, top_controls_manager_->ContentTopOffset()); active_tree_->GetViewportSelection(&metadata.selection_start, &metadata.selection_end); @@ -1658,9 +1653,21 @@ void LayerTreeHostImpl::UpdateViewportContainerSizes() { LayerImpl* inner_container = active_tree_->InnerViewportContainerLayer(); LayerImpl* outer_container = active_tree_->OuterViewportContainerLayer(); - if (!inner_container || !top_controls_manager_) + if (!inner_container) return; + // TODO(bokan): This code is currently specific to top controls. It should be + // made general. crbug.com/464814. + if (!TopControlsHeight()) { + if (outer_container) + outer_container->SetBoundsDelta(gfx::Vector2dF()); + + inner_container->SetBoundsDelta(gfx::Vector2dF()); + active_tree_->InnerViewportScrollLayer()->SetBoundsDelta(gfx::Vector2dF()); + + return; + } + ViewportAnchor anchor(InnerViewportScrollLayer(), OuterViewportScrollLayer()); @@ -2361,8 +2368,7 @@ InputHandler::ScrollStatus LayerTreeHostImpl::ScrollBegin( InputHandler::ScrollInputType type) { TRACE_EVENT0("cc", "LayerTreeHostImpl::ScrollBegin"); - if (top_controls_manager_) - top_controls_manager_->ScrollBegin(); + top_controls_manager_->ScrollBegin(); DCHECK(!CurrentlyScrollingLayer()); ClearCurrentlyScrollingLayer(); @@ -2573,9 +2579,6 @@ bool LayerTreeHostImpl::ShouldTopControlsConsumeScroll( const gfx::Vector2dF& scroll_delta) const { DCHECK(CurrentlyScrollingLayer()); - if (!top_controls_manager_) - return false; - // Always consume if it's in the direction to show the top controls. if (scroll_delta.y() < 0) return true; @@ -2816,8 +2819,7 @@ void LayerTreeHostImpl::ClearCurrentlyScrollingLayer() { } void LayerTreeHostImpl::ScrollEnd() { - if (top_controls_manager_) - top_controls_manager_->ScrollEnd(); + top_controls_manager_->ScrollEnd(); ClearCurrentlyScrollingLayer(); } @@ -2938,8 +2940,7 @@ void LayerTreeHostImpl::PinchGestureBegin() { active_tree_->SetCurrentlyScrollingLayer( active_tree_->InnerViewportScrollLayer()); } - if (top_controls_manager_) - top_controls_manager_->PinchBegin(); + top_controls_manager_->PinchBegin(); } void LayerTreeHostImpl::PinchGestureUpdate(float magnify_delta, @@ -2998,8 +2999,7 @@ void LayerTreeHostImpl::PinchGestureEnd() { pinch_gesture_end_should_clear_scrolling_layer_ = false; ClearCurrentlyScrollingLayer(); } - if (top_controls_manager_) - top_controls_manager_->PinchEnd(); + top_controls_manager_->PinchEnd(); client_->SetNeedsCommitOnImplThread(); // When a pinch ends, we may be displaying content cached at incorrect scales, // so updating draw properties and drawing will ensure we are using the right @@ -3094,7 +3094,7 @@ void LayerTreeHostImpl::AnimatePageScale(base::TimeTicks monotonic_time) { } void LayerTreeHostImpl::AnimateTopControls(base::TimeTicks time) { - if (!top_controls_manager_ || !top_controls_manager_->animation()) + if (!top_controls_manager_->animation()) return; gfx::Vector2dF scroll = top_controls_manager_->Animate(time); diff --git a/cc/trees/layer_tree_host_impl_unittest.cc b/cc/trees/layer_tree_host_impl_unittest.cc index 14de9bf..d384ac7 100644 --- a/cc/trees/layer_tree_host_impl_unittest.cc +++ b/cc/trees/layer_tree_host_impl_unittest.cc @@ -2446,7 +2446,6 @@ class LayerTreeHostImplTopControlsTest : public LayerTreeHostImplTest { : layer_size_(10, 10), clip_size_(layer_size_), top_controls_height_(50) { - settings_.calculate_top_controls_position = true; settings_.use_pinch_virtual_viewport = true; viewport_size_ = gfx::Size(clip_size_.width(), @@ -2457,7 +2456,7 @@ class LayerTreeHostImplTopControlsTest : public LayerTreeHostImplTest { scoped_ptr<OutputSurface> output_surface) override { bool init = LayerTreeHostImplTest::CreateHostImpl(settings, output_surface.Pass()); - if (init && settings.calculate_top_controls_position) { + if (init) { host_impl_->active_tree()->set_top_controls_height(top_controls_height_); host_impl_->active_tree()->SetCurrentTopControlsShownRatio(1.f); } @@ -7455,7 +7454,6 @@ class LayerTreeHostImplWithTopControlsTest : public LayerTreeHostImplTest { public: void SetUp() override { LayerTreeSettings settings = DefaultSettings(); - settings.calculate_top_controls_position = true; CreateHostImpl(settings, CreateOutputSurface()); host_impl_->active_tree()->set_top_controls_height(top_controls_height_); host_impl_->sync_tree()->set_top_controls_height(top_controls_height_); diff --git a/cc/trees/layer_tree_settings.cc b/cc/trees/layer_tree_settings.cc index 9d605ae..0088e91 100644 --- a/cc/trees/layer_tree_settings.cc +++ b/cc/trees/layer_tree_settings.cc @@ -39,7 +39,6 @@ LayerTreeSettings::LayerTreeSettings() scrollbar_fade_duration_ms(0), scrollbar_show_scale_threshold(1.0f), solid_color_scrollbar_color(SK_ColorWHITE), - calculate_top_controls_position(false), timeout_and_draw_when_animation_checkerboards(true), maximum_number_of_failed_draws_before_draw_is_forced_(3), layer_transforms_should_scale_layer_contents(false), diff --git a/cc/trees/layer_tree_settings.h b/cc/trees/layer_tree_settings.h index 06c319a..ce8d856 100644 --- a/cc/trees/layer_tree_settings.h +++ b/cc/trees/layer_tree_settings.h @@ -51,7 +51,6 @@ class CC_EXPORT LayerTreeSettings { int scrollbar_fade_duration_ms; float scrollbar_show_scale_threshold; SkColor solid_color_scrollbar_color; - bool calculate_top_controls_position; bool timeout_and_draw_when_animation_checkerboards; int maximum_number_of_failed_draws_before_draw_is_forced_; bool layer_transforms_should_scale_layer_contents; |