[Android] Add a flag to disable the pull-to-refresh effect

content/browser/android/overscroll_controller_android.cc

Index: content/browser/android/overscroll_controller_android.cc
diff --git a/content/browser/android/overscroll_controller_android.cc b/content/browser/android/overscroll_controller_android.cc
index db35ea49b6584653636c33e8241022b48dcd9c56..bdb79230224ea94d1be9728c47436c903fb4300b 100644
--- a/content/browser/android/overscroll_controller_android.cc
+++ b/content/browser/android/overscroll_controller_android.cc
@@ -6,12 +6,14 @@
 
 #include "base/android/build_info.h"
 #include "base/bind.h"
+#include "base/command_line.h"
 #include "cc/layers/layer.h"
 #include "cc/output/compositor_frame_metadata.h"
 #include "content/browser/android/edge_effect.h"
 #include "content/browser/android/edge_effect_l.h"
 #include "content/common/input/did_overscroll_params.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/common/content_switches.h"
 #include "third_party/WebKit/public/web/WebInputEvent.h"
 #include "ui/base/android/window_android_compositor.h"
 
@@ -34,6 +36,33 @@ scoped_ptr<EdgeEffectBase> CreateGlowEdgeEffect(
       new EdgeEffect(resource_manager, dpi_scale));
 }
 
+scoped_ptr<OverscrollGlow> CreateGlowEffect(
+    ui::WindowAndroidCompositor* compositor,
+    float dpi_scale) {
+  static bool sEnableGlowEffect =

[aelias_OOO_until_Jul13, 2014/11/19 06:21:51]

Please remove these statics, static global state often comes back to bite in
unexpected ways.  I'd rather just pay the string lookup cost every time.

[jdduke (slow), 2014/11/19 16:56:22]

Done.

+      !base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kDisableOverscrollEdgeEffect);
+  if (!sEnableGlowEffect)
+    return scoped_ptr<OverscrollGlow>();
+
+  return make_scoped_ptr(new OverscrollGlow(
+      base::Bind(&CreateGlowEdgeEffect,
+                 &compositor->GetSystemUIResourceManager(), dpi_scale)));
+}
+
+scoped_ptr<OverscrollRefresh> CreateRefreshEffect(
+    ui::WindowAndroidCompositor* compositor,
+    OverscrollRefreshClient* client) {
+  static bool sEnableRefreshEffect =
+      !base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kDisablePullToRefreshEffect);
+  if (!sEnableRefreshEffect)
+    return scoped_ptr<OverscrollRefresh>();
+
+  return make_scoped_ptr(
+      new OverscrollRefresh(&compositor->GetSystemUIResourceManager(), client));
+}
+
 }  // namespace
 
 OverscrollControllerAndroid::OverscrollControllerAndroid(
@@ -43,13 +72,13 @@ OverscrollControllerAndroid::OverscrollControllerAndroid(
     : WebContentsObserver(web_contents),
       compositor_(compositor),
       dpi_scale_(dpi_scale),
-      enabled_(true),
-      glow_effect_(base::Bind(&CreateGlowEdgeEffect,
-                              &compositor->GetSystemUIResourceManager(),
-                              dpi_scale_)),
-      refresh_effect_(&compositor->GetSystemUIResourceManager(), this),
+      enabled_(false),
+      glow_effect_(CreateGlowEffect(compositor, dpi_scale)),
+      refresh_effect_(CreateRefreshEffect(compositor, this)),
       triggered_refresh_active_(false) {
   DCHECK(web_contents);
+  DCHECK(compositor);
+  enabled_ = glow_effect_ || refresh_effect_;

[aelias_OOO_until_Jul13, 2014/11/19 06:21:50]

This complicates the meaning of "enabled_" by baking into it information we
already have from the other members anyway.  I'd rather it just be allowed to be
"true" even if glow_effect_ and refresh_effect_ are both null.

[jdduke (slow), 2014/11/19 16:56:22]

Done.

 }
 
 OverscrollControllerAndroid::~OverscrollControllerAndroid() {
@@ -60,11 +89,14 @@ bool OverscrollControllerAndroid::WillHandleGestureEvent(
   if (!enabled_)
     return false;
 
+  if (!refresh_effect_)
+    return false;
+
   bool handled = false;
   bool maybe_needs_animate = false;
   switch (event.type) {
     case blink::WebInputEvent::GestureScrollBegin:
-      refresh_effect_.OnScrollBegin();
+      refresh_effect_->OnScrollBegin();
       break;
 
     case blink::WebInputEvent::GestureScrollUpdate: {
@@ -72,11 +104,11 @@ bool OverscrollControllerAndroid::WillHandleGestureEvent(
                                   event.data.scrollUpdate.deltaY);
       scroll_delta.Scale(dpi_scale_);
       maybe_needs_animate = true;
-      handled = refresh_effect_.WillHandleScrollUpdate(scroll_delta);
+      handled = refresh_effect_->WillHandleScrollUpdate(scroll_delta);
     } break;
 
     case blink::WebInputEvent::GestureScrollEnd:
-      refresh_effect_.OnScrollEnd(gfx::Vector2dF());
+      refresh_effect_->OnScrollEnd(gfx::Vector2dF());
       maybe_needs_animate = true;
       break;
 
@@ -84,8 +116,8 @@ bool OverscrollControllerAndroid::WillHandleGestureEvent(
       gfx::Vector2dF scroll_velocity(event.data.flingStart.velocityX,
                                      event.data.flingStart.velocityY);
       scroll_velocity.Scale(dpi_scale_);
-      refresh_effect_.OnScrollEnd(scroll_velocity);
-      if (refresh_effect_.IsActive()) {
+      refresh_effect_->OnScrollEnd(scroll_velocity);
+      if (refresh_effect_->IsActive()) {
         // TODO(jdduke): Figure out a cleaner way of suppressing a fling.
         // It's important that the any downstream code sees a scroll-ending
         // event (in this case GestureFlingStart) if it has seen a scroll begin.
@@ -105,7 +137,7 @@ bool OverscrollControllerAndroid::WillHandleGestureEvent(
       break;
   }
 
-  if (maybe_needs_animate && refresh_effect_.IsActive())
+  if (maybe_needs_animate && refresh_effect_->IsActive())
     SetNeedsAnimate();
 
   return handled;
@@ -124,13 +156,14 @@ void OverscrollControllerAndroid::OnGestureEventAck(
     OnOverscrolled(DidOverscrollParams());
   }
 
-  if (event.type == blink::WebInputEvent::GestureScrollUpdate) {
+  if (event.type == blink::WebInputEvent::GestureScrollUpdate &&
+      refresh_effect_) {
     // The effect should only be allowed if both the causal touch events go
     // unconsumed and the generated scroll events go unconsumed.
     // TODO(jdduke): Prevent activation if the first touchmove was consumed,
     // i.e., the first GSU was prevented.
     bool consumed = ack_result == INPUT_EVENT_ACK_STATE_CONSUMED;
-    refresh_effect_.OnScrollUpdateAck(consumed);
+    refresh_effect_->OnScrollUpdateAck(consumed);
   }
 }
 
@@ -139,14 +172,16 @@ void OverscrollControllerAndroid::OnOverscrolled(
   if (!enabled_)
     return;
 
-  if (refresh_effect_.IsActive() ||
-      refresh_effect_.IsAwaitingScrollUpdateAck()) {
+  if (refresh_effect_ &&
+      (refresh_effect_->IsActive() ||
+       refresh_effect_->IsAwaitingScrollUpdateAck())) {
     // An active (or potentially active) refresh effect should always pre-empt
     // the passive glow effect.
     return;
   }
 
-  if (glow_effect_.OnOverscrolled(
+  if (glow_effect_ &&
+      glow_effect_->OnOverscrolled(
       base::TimeTicks::Now(),
       gfx::ScaleVector2d(params.accumulated_overscroll, dpi_scale_),
       gfx::ScaleVector2d(params.latest_overscroll_delta, dpi_scale_),
@@ -163,13 +198,19 @@ bool OverscrollControllerAndroid::Animate(base::TimeTicks current_time,
   if (!enabled_)
     return false;
 
-  bool needs_animate = refresh_effect_.Animate(current_time, parent_layer);
-  needs_animate |= glow_effect_.Animate(current_time, parent_layer);
+  bool needs_animate = false;
+  if (refresh_effect_)
+    needs_animate |= refresh_effect_->Animate(current_time, parent_layer);
+  if (glow_effect_)
+    needs_animate |= glow_effect_->Animate(current_time, parent_layer);
   return needs_animate;
 }
 
 void OverscrollControllerAndroid::OnFrameMetadataUpdated(
     const cc::CompositorFrameMetadata& frame_metadata) {
+  if (!refresh_effect_ && !glow_effect_)
+    return;
+
   const float scale_factor =
       frame_metadata.page_scale_factor * frame_metadata.device_scale_factor;
   gfx::SizeF viewport_size =
@@ -179,13 +220,17 @@ void OverscrollControllerAndroid::OnFrameMetadataUpdated(
   gfx::Vector2dF content_scroll_offset =
       gfx::ScaleVector2d(frame_metadata.root_scroll_offset, scale_factor);
 
-  refresh_effect_.UpdateDisplay(viewport_size, content_scroll_offset);
-  glow_effect_.UpdateDisplay(viewport_size, content_size,
-                             content_scroll_offset);
+  if (refresh_effect_)
+    refresh_effect_->UpdateDisplay(viewport_size, content_scroll_offset);
+
+  if (glow_effect_) {
+    glow_effect_->UpdateDisplay(viewport_size, content_size,
+                                content_scroll_offset);
+  }
 }
 
 void OverscrollControllerAndroid::Enable() {
-  enabled_ = true;
+  enabled_ = glow_effect_ || refresh_effect_;

[aelias_OOO_until_Jul13, 2014/11/19 06:21:51]

As mentioned above, I'd prefer this to just be "enabled_ = true"

[jdduke (slow), 2014/11/19 16:56:22]

Done.

 }
 
 void OverscrollControllerAndroid::Disable() {
@@ -193,8 +238,10 @@ void OverscrollControllerAndroid::Disable() {
     return;
   enabled_ = false;
   if (!enabled_) {
-    refresh_effect_.Reset();
-    glow_effect_.Reset();
+    if (refresh_effect_)
+      refresh_effect_->Reset();
+    if (glow_effect_)
+      glow_effect_->Reset();
   }
 }