From 5945ac2267eb79b3bfe9609d9b77f6d3c854abfe Mon Sep 17 00:00:00 2001 From: "jdduke@chromium.org" Date: Wed, 26 Mar 2014 04:24:22 +0000 Subject: [PATCH] [Android] Harden overscroll resource allocation It appears that overscroll scenarios are not covered by existing integration tests on Android. Add such a test while cleaning up an overscroll-related code path for loading potentially non-existent resources. BUG=341406 Review URL: https://codereview.chromium.org/210623002 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@259474 0039d316-1c4b-4281-b951-d872f2087c98 --- content/browser/android/overscroll_glow.cc | 23 ++++++++++-- .../content/browser/ContentViewScrollingTest.java | 29 +++++++++++++++ .../java/src/org/chromium/ui/gfx/BitmapHelper.java | 3 +- ui/gfx/android/java_bitmap.cc | 43 +++++++++++----------- ui/gfx/android/java_bitmap.h | 24 ++++++++---- 5 files changed, 88 insertions(+), 34 deletions(-) diff --git a/content/browser/android/overscroll_glow.cc b/content/browser/android/overscroll_glow.cc index 8e08de45eca9..f4a0168cd556 100644 --- a/content/browser/android/overscroll_glow.cc +++ b/content/browser/android/overscroll_glow.cc @@ -9,6 +9,7 @@ #include "base/threading/worker_pool.h" #include "cc/layers/image_layer.h" #include "content/browser/android/edge_effect.h" +#include "skia/ext/image_operations.h" #include "ui/gfx/android/java_bitmap.h" using std::max; @@ -20,16 +21,30 @@ namespace { const float kEpsilon = 1e-3f; +SkBitmap CreateSkBitmapFromAndroidResource(const char* name, gfx::Size size) { + base::android::ScopedJavaLocalRef jobj = + gfx::CreateJavaBitmapFromAndroidResource(name, size); + if (jobj.is_null()) + return SkBitmap(); + + SkBitmap bitmap = CreateSkBitmapFromJavaBitmap(gfx::JavaBitmap(jobj.obj())); + if (bitmap.isNull()) + return bitmap; + + return skia::ImageOperations::Resize( + bitmap, skia::ImageOperations::RESIZE_BOX, size.width(), size.height()); +} + class OverscrollResources { public: OverscrollResources() { TRACE_EVENT0("browser", "OverscrollResources::Create"); edge_bitmap_ = - gfx::CreateSkBitmapFromResource("android:drawable/overscroll_edge", - gfx::Size(128, 12)); + CreateSkBitmapFromAndroidResource("android:drawable/overscroll_edge", + gfx::Size(128, 12)); glow_bitmap_ = - gfx::CreateSkBitmapFromResource("android:drawable/overscroll_glow", - gfx::Size(128, 64)); + CreateSkBitmapFromAndroidResource("android:drawable/overscroll_glow", + gfx::Size(128, 64)); } const SkBitmap& edge_bitmap() { return edge_bitmap_; } diff --git a/content/public/android/javatests/src/org/chromium/content/browser/ContentViewScrollingTest.java b/content/public/android/javatests/src/org/chromium/content/browser/ContentViewScrollingTest.java index 92d3a3db7ce9..6da349b46cbc 100644 --- a/content/public/android/javatests/src/org/chromium/content/browser/ContentViewScrollingTest.java +++ b/content/public/android/javatests/src/org/chromium/content/browser/ContentViewScrollingTest.java @@ -197,6 +197,35 @@ public class ContentViewScrollingTest extends ContentShellTestBase { assertWaitForScroll(false, false); } + + /** + * To ensure the device properly responds to bounds-exceeding scrolls, e.g., overscroll + * effects are properly initialized. + */ + @SmallTest + @Feature({"Main"}) + public void testOverScroll() throws Throwable { + // Overscroll lower-left. + scrollTo(-10000, 10000); + assertWaitForScroll(true, false); + + // Overscroll lower-right. + scrollTo(10000, 10000); + assertWaitForScroll(false, false); + + // Overscroll upper-right. + scrollTo(10000, -10000); + assertWaitForScroll(false, true); + + // Overscroll top-left. + scrollTo(-10000, -10000); + assertWaitForScroll(true, true); + + // Diagonal overscroll lower-right. + scrollTo(10000, 10000); + assertWaitForScroll(false, false); + } + /** * To ensure the AccessibilityEvent notifications (Eg:TYPE_VIEW_SCROLLED) are being sent * properly on scrolling a page. diff --git a/ui/android/java/src/org/chromium/ui/gfx/BitmapHelper.java b/ui/android/java/src/org/chromium/ui/gfx/BitmapHelper.java index dd04217183f9..5c4549cd9d8f 100644 --- a/ui/android/java/src/org/chromium/ui/gfx/BitmapHelper.java +++ b/ui/android/java/src/org/chromium/ui/gfx/BitmapHelper.java @@ -39,14 +39,15 @@ public class BitmapHelper { int reqHeight) { Resources res = Resources.getSystem(); int resId = res.getIdentifier(name, null, null); + if (resId == 0) return null; final BitmapFactory.Options options = new BitmapFactory.Options(); options.inJustDecodeBounds = true; BitmapFactory.decodeResource(res, resId, options); options.inSampleSize = calculateInSampleSize(options, reqWidth, reqHeight); - options.inJustDecodeBounds = false; + options.inPreferredConfig = Bitmap.Config.ARGB_8888; return BitmapFactory.decodeResource(res, resId, options); } diff --git a/ui/gfx/android/java_bitmap.cc b/ui/gfx/android/java_bitmap.cc index 844385b78d27..90df12a71fad 100644 --- a/ui/gfx/android/java_bitmap.cc +++ b/ui/gfx/android/java_bitmap.cc @@ -9,7 +9,6 @@ #include "base/android/jni_string.h" #include "base/logging.h" #include "jni/BitmapHelper_jni.h" -#include "skia/ext/image_operations.h" #include "ui/gfx/size.h" using base::android::AttachCurrentThread; @@ -61,13 +60,27 @@ static int SkBitmapConfigToBitmapFormat(SkBitmap::Config bitmap_config) { ScopedJavaLocalRef CreateJavaBitmap(int width, int height, SkBitmap::Config bitmap_config) { + DCHECK_GT(width, 0); + DCHECK_GT(height, 0); int java_bitmap_config = SkBitmapConfigToBitmapFormat(bitmap_config); return Java_BitmapHelper_createBitmap( AttachCurrentThread(), width, height, java_bitmap_config); } +ScopedJavaLocalRef CreateJavaBitmapFromAndroidResource( + const char* name, + gfx::Size size) { + DCHECK(name); + DCHECK(!size.IsEmpty()); + JNIEnv* env = AttachCurrentThread(); + ScopedJavaLocalRef jname(env, env->NewStringUTF(name)); + return Java_BitmapHelper_decodeDrawableResource( + env, jname.obj(), size.width(), size.height()); +} + ScopedJavaLocalRef ConvertToJavaBitmap(const SkBitmap* skbitmap) { DCHECK(skbitmap); + DCHECK(!skbitmap->isNull()); SkBitmap::Config bitmap_config = skbitmap->getConfig(); DCHECK((bitmap_config == SkBitmap::kRGB_565_Config) || (bitmap_config == SkBitmap::kARGB_8888_Config)); @@ -82,8 +95,13 @@ ScopedJavaLocalRef ConvertToJavaBitmap(const SkBitmap* skbitmap) { return jbitmap; } -SkBitmap CreateSkBitmapFromJavaBitmap(JavaBitmap& jbitmap) { - DCHECK_EQ(jbitmap.format(), ANDROID_BITMAP_FORMAT_RGBA_8888); +SkBitmap CreateSkBitmapFromJavaBitmap(const JavaBitmap& jbitmap) { + // TODO(jdduke): Convert to DCHECK's when sufficient data has been capture for + // crbug.com/341406. + CHECK_EQ(jbitmap.format(), ANDROID_BITMAP_FORMAT_RGBA_8888); + CHECK(!jbitmap.size().IsEmpty()); + CHECK_GT(jbitmap.stride(), 0U); + CHECK(jbitmap.pixels()); gfx::Size src_size = jbitmap.size(); @@ -97,30 +115,13 @@ SkBitmap CreateSkBitmapFromJavaBitmap(JavaBitmap& jbitmap) { << "x" << src_size.height() << " stride=" << jbitmap.stride(); } SkAutoLockPixels dst_lock(skbitmap); - void* src_pixels = jbitmap.pixels(); + const void* src_pixels = jbitmap.pixels(); void* dst_pixels = skbitmap.getPixels(); - CHECK(src_pixels); - memcpy(dst_pixels, src_pixels, skbitmap.getSize()); return skbitmap; } -SkBitmap CreateSkBitmapFromResource(const char* name, gfx::Size size) { - DCHECK(!size.IsEmpty()); - JNIEnv* env = AttachCurrentThread(); - ScopedJavaLocalRef jname(env, env->NewStringUTF(name)); - ScopedJavaLocalRef jobj(Java_BitmapHelper_decodeDrawableResource( - env, jname.obj(), size.width(), size.height())); - if (jobj.is_null()) - return SkBitmap(); - - JavaBitmap jbitmap(jobj.obj()); - SkBitmap bitmap = CreateSkBitmapFromJavaBitmap(jbitmap); - return skia::ImageOperations::Resize( - bitmap, skia::ImageOperations::RESIZE_BOX, size.width(), size.height()); -} - SkBitmap::Config ConvertToSkiaConfig(jobject bitmap_config) { int jbitmap_config = Java_BitmapHelper_getBitmapFormatForConfig( AttachCurrentThread(), bitmap_config); diff --git a/ui/gfx/android/java_bitmap.h b/ui/gfx/android/java_bitmap.h index 5cda137a1eef..44a17cade744 100644 --- a/ui/gfx/android/java_bitmap.h +++ b/ui/gfx/android/java_bitmap.h @@ -31,6 +31,7 @@ class GFX_EXPORT JavaBitmap { ~JavaBitmap(); inline void* pixels() { return pixels_; } + inline const void* pixels() const { return pixels_; } inline const gfx::Size& size() const { return size_; } // Formats are in android/bitmap.h; e.g. ANDROID_BITMAP_FORMAT_RGBA_8888 inline int format() const { return format_; } @@ -49,22 +50,29 @@ class GFX_EXPORT JavaBitmap { DISALLOW_COPY_AND_ASSIGN(JavaBitmap); }; -// Allocates a Java-backed bitmap (android.graphics.Bitmap) with the given size -// and configuration. +// Allocates a Java-backed bitmap (android.graphics.Bitmap) with the given +// (non-empty!) size and configuration. GFX_EXPORT base::android::ScopedJavaLocalRef CreateJavaBitmap( int width, int height, SkBitmap::Config bitmap_config); +// Loads a Java-backed bitmap (android.graphics.Bitmap) from the provided +// drawable resource identifier (e.g., android:drawable/overscroll_glow). If the +// resource loads successfully, it will be integrally scaled down, preserving +// aspect ratio, to a size no smaller than |size|. Otherwise, null is returned. +GFX_EXPORT base::android::ScopedJavaLocalRef + CreateJavaBitmapFromAndroidResource(const char* name, gfx::Size size); + +// Converts |skbitmap| to a Java-backed bitmap (android.graphics.Bitmap). +// Note: |skbitmap| is assumed to be non-null, non-empty and one of RGBA_8888 or +// RGB_565 formats. GFX_EXPORT base::android::ScopedJavaLocalRef ConvertToJavaBitmap( const SkBitmap* skbitmap); -GFX_EXPORT SkBitmap CreateSkBitmapFromJavaBitmap(JavaBitmap& jbitmap); - -// If the resource loads successfully, it will be resized to |size|. -// Note: If the source resource is smaller than |size|, quality may suffer. -GFX_EXPORT SkBitmap CreateSkBitmapFromResource(const char* name, - gfx::Size size); +// Converts |bitmap| to an SkBitmap of the same size and format. +// Note: |jbitmap| is assumed to be non-null, non-empty and of format RGBA_8888. +GFX_EXPORT SkBitmap CreateSkBitmapFromJavaBitmap(const JavaBitmap& jbitmap); // Returns a Skia config value for the requested input java Bitmap.Config. GFX_EXPORT SkBitmap::Config ConvertToSkiaConfig(jobject bitmap_config); -- 2.11.4.GIT