diff options
author | bokan <bokan@chromium.org> | 2015-03-05 08:19:35 -0800 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-03-05 16:20:19 +0000 |
commit | 7610e74eacde3468853c9765bd3f726c14df04c1 (patch) | |
tree | 41ae127fea698a6872b12f9e63e595f79a86b7bb | |
parent | a9df577b430b3c3fa58cb2e00dd17c410290c062 (diff) | |
download | chromium_src-7610e74eacde3468853c9765bd3f726c14df04c1.zip chromium_src-7610e74eacde3468853c9765bd3f726c14df04c1.tar.gz chromium_src-7610e74eacde3468853c9765bd3f726c14df04c1.tar.bz2 |
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
Review URL: https://codereview.chromium.org/961023002
Cr-Commit-Position: refs/heads/master@{#319257}
-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 | 48 | ||||
-rw-r--r-- | cc/trees/layer_tree_host.cc | 10 | ||||
-rw-r--r-- | cc/trees/layer_tree_host_impl.cc | 42 | ||||
-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 | ||||
-rw-r--r-- | chrome/android/java/src/org/chromium/chrome/browser/ApplicationInitialization.java | 2 | ||||
-rw-r--r-- | chrome/browser/chromeos/login/chrome_restart_request.cc | 1 | ||||
-rw-r--r-- | content/browser/renderer_host/compositor_impl_android.cc | 1 | ||||
-rw-r--r-- | content/browser/renderer_host/render_process_host_impl.cc | 1 | ||||
-rw-r--r-- | content/public/android/java/src/org/chromium/content/common/ContentSwitches.java | 4 | ||||
-rw-r--r-- | content/renderer/gpu/render_widget_compositor.cc | 2 |
15 files changed, 74 insertions, 56 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 7362805..c8bd534 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; @@ -180,6 +183,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 16d0e5d..4befa15 100644 --- a/cc/input/top_controls_manager_unittest.cc +++ b/cc/input/top_controls_manager_unittest.cc @@ -409,8 +409,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); @@ -445,5 +446,50 @@ TEST(TopControlsManagerTest, ShrinkingHeightKeepsTopControlsHidden) { EXPECT_FLOAT_EQ(0.f, manager->ContentTopOffset()); } +TEST(TopControlsManagerTest, ZeroTopControlsHeightScrolling) { + MockTopControlsManagerClient client(0.f, 0.5f, 0.5f); + client.SetCurrentTopControlsShownRatio(0.f); + TopControlsManager* manager = client.manager(); + manager->UpdateTopControlsState(BOTH, BOTH, false); + + manager->ScrollBegin(); + EXPECT_FLOAT_EQ(-10.f, manager->ScrollBy(gfx::Vector2dF(0.f, -10.f)).y()); + EXPECT_FLOAT_EQ(0.f, manager->TopControlsShownRatio()); + + client.SetTopControlsHeight(20.f); + EXPECT_FLOAT_EQ(0.f, manager->TopControlsShownRatio()); + + EXPECT_FLOAT_EQ(0.f, manager->ScrollBy(gfx::Vector2dF(0.f, -15.f)).y()); + EXPECT_FLOAT_EQ(0.75f, manager->TopControlsShownRatio()); + client.SetTopControlsHeight(0.f); + manager->ScrollEnd(); + + EXPECT_FALSE(manager->animation()); + EXPECT_FLOAT_EQ(1.f, manager->TopControlsShownRatio()); +} + +TEST(TopControlsManagerTest, ZeroTopControlsHeightAnimating) { + MockTopControlsManagerClient client(0.f, 0.5f, 0.5f); + client.SetCurrentTopControlsShownRatio(0.f); + TopControlsManager* manager = client.manager(); + + manager->UpdateTopControlsState(BOTH, HIDDEN, false); + EXPECT_FLOAT_EQ(0.f, manager->TopControlsShownRatio()); + manager->UpdateTopControlsState(BOTH, SHOWN, false); + EXPECT_FLOAT_EQ(1.f, manager->TopControlsShownRatio()); + + manager->UpdateTopControlsState(BOTH, HIDDEN, true); + EXPECT_FLOAT_EQ(0.f, manager->TopControlsShownRatio()); + EXPECT_FALSE(manager->animation()); + manager->UpdateTopControlsState(BOTH, SHOWN, true); + EXPECT_FLOAT_EQ(1.f, manager->TopControlsShownRatio()); + EXPECT_FALSE(manager->animation()); + + client.SetCurrentTopControlsShownRatio(0.3f); + manager->MainThreadHasStoppedFlinging(); + EXPECT_FALSE(manager->animation()); + EXPECT_FLOAT_EQ(0.f, manager->TopControlsShownRatio()); +} + } // namespace } // namespace cc diff --git a/cc/trees/layer_tree_host.cc b/cc/trees/layer_tree_host.cc index 92e9c25..d893b33 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(); } @@ -1158,9 +1155,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 c03f7f79..2f4d9a9 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,7 +1653,7 @@ 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; ViewportAnchor anchor(InnerViewportScrollLayer(), @@ -2356,8 +2351,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(); @@ -2568,9 +2562,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; @@ -2811,8 +2802,7 @@ void LayerTreeHostImpl::ClearCurrentlyScrollingLayer() { } void LayerTreeHostImpl::ScrollEnd() { - if (top_controls_manager_) - top_controls_manager_->ScrollEnd(); + top_controls_manager_->ScrollEnd(); ClearCurrentlyScrollingLayer(); } @@ -2933,8 +2923,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, @@ -2993,8 +2982,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 @@ -3089,7 +3077,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 110bee9..4b2bc75 100644 --- a/cc/trees/layer_tree_host_impl_unittest.cc +++ b/cc/trees/layer_tree_host_impl_unittest.cc @@ -2478,7 +2478,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(), @@ -2489,7 +2488,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); } @@ -7453,7 +7452,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; diff --git a/chrome/android/java/src/org/chromium/chrome/browser/ApplicationInitialization.java b/chrome/android/java/src/org/chromium/chrome/browser/ApplicationInitialization.java index 6f0472d..f98c1f8 100644 --- a/chrome/android/java/src/org/chromium/chrome/browser/ApplicationInitialization.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/ApplicationInitialization.java @@ -35,8 +35,6 @@ public final class ApplicationInitialization { CommandLine commandLine = CommandLine.getInstance(); if (commandLine.hasSwitch(ChromeSwitches.DISABLE_FULLSCREEN)) return; - commandLine.appendSwitch(ContentSwitches.ENABLE_TOP_CONTROLS_POSITION_CALCULATION); - TypedValue threshold = new TypedValue(); resources.getValue(R.floats.top_controls_show_threshold, threshold, true); commandLine.appendSwitchWithValue( diff --git a/chrome/browser/chromeos/login/chrome_restart_request.cc b/chrome/browser/chromeos/login/chrome_restart_request.cc index 7a10b51..329470a 100644 --- a/chrome/browser/chromeos/login/chrome_restart_request.cc +++ b/chrome/browser/chromeos/login/chrome_restart_request.cc @@ -191,7 +191,6 @@ std::string DeriveCommandLine(const GURL& start_url, cc::switches::kEnablePinchVirtualViewport, cc::switches::kEnablePropertyTreeVerification, cc::switches::kEnableMainFrameBeforeActivation, - cc::switches::kEnableTopControlsPositionCalculation, cc::switches::kMaxTilesForInterestArea, cc::switches::kMaxUnusedResourceMemoryUsagePercentage, cc::switches::kShowCompositedLayerBorders, diff --git a/content/browser/renderer_host/compositor_impl_android.cc b/content/browser/renderer_host/compositor_impl_android.cc index 6a7c1ee..65cb728 100644 --- a/content/browser/renderer_host/compositor_impl_android.cc +++ b/content/browser/renderer_host/compositor_impl_android.cc @@ -379,7 +379,6 @@ void CompositorImpl::CreateLayerTreeHost() { settings.renderer_settings.allow_antialiasing = false; settings.renderer_settings.highp_threshold_min = 2048; settings.impl_side_painting = true; - settings.calculate_top_controls_position = false; base::CommandLine* command_line = base::CommandLine::ForCurrentProcess(); settings.initial_debug_state.SetRecordRenderingStats( diff --git a/content/browser/renderer_host/render_process_host_impl.cc b/content/browser/renderer_host/render_process_host_impl.cc index 7f5c54d..b1eae97 100644 --- a/content/browser/renderer_host/render_process_host_impl.cc +++ b/content/browser/renderer_host/render_process_host_impl.cc @@ -1324,7 +1324,6 @@ void RenderProcessHostImpl::PropagateBrowserCommandLineToRenderer( cc::switches::kDisableThreadedAnimation, cc::switches::kEnableGpuBenchmarking, cc::switches::kEnableMainFrameBeforeActivation, - cc::switches::kEnableTopControlsPositionCalculation, cc::switches::kMaxTilesForInterestArea, cc::switches::kMaxUnusedResourceMemoryUsagePercentage, cc::switches::kShowCompositedLayerBorders, diff --git a/content/public/android/java/src/org/chromium/content/common/ContentSwitches.java b/content/public/android/java/src/org/chromium/content/common/ContentSwitches.java index 44a854c..a671db7 100644 --- a/content/public/android/java/src/org/chromium/content/common/ContentSwitches.java +++ b/content/public/android/java/src/org/chromium/content/common/ContentSwitches.java @@ -41,10 +41,6 @@ public abstract class ContentSwitches { // Sets the ISO country code that will be used for phone number detection. public static final String NETWORK_COUNTRY_ISO = "network-country-iso"; - // Whether to enable the auto-hiding top controls. - public static final String ENABLE_TOP_CONTROLS_POSITION_CALCULATION = - "enable-top-controls-position-calculation"; - // How much of the top controls need to be shown before they will auto show. public static final String TOP_CONTROLS_SHOW_THRESHOLD = "top-controls-show-threshold"; diff --git a/content/renderer/gpu/render_widget_compositor.cc b/content/renderer/gpu/render_widget_compositor.cc index c01231e..00268c29 100644 --- a/content/renderer/gpu/render_widget_compositor.cc +++ b/content/renderer/gpu/render_widget_compositor.cc @@ -275,8 +275,6 @@ void RenderWidgetCompositor::Initialize() { compositor_deps_->IsElasticOverscrollEnabled(); settings.use_image_texture_target = compositor_deps_->GetImageTextureTarget(); - settings.calculate_top_controls_position = - cmd->HasSwitch(cc::switches::kEnableTopControlsPositionCalculation); if (cmd->HasSwitch(cc::switches::kTopControlsShowThreshold)) { std::string top_threshold_str = cmd->GetSwitchValueASCII(cc::switches::kTopControlsShowThreshold); |