SkBitmap::Config is deprecated, use SkColorType instead

SkBitmap::Config is deprecated, use SkColorType instead

BUG=skia:2706
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=281783

[reed1, 2014-07-07 14:41:26]

need owners for refactoring change: content/ chrome/ ui/gfx

[f(malita), 2014-07-07 14:51:58]

skia/ lgtm

[jochen (gone - plz use gerrit), 2014-07-07 15:15:42]

lgtm

[reed1, 2014-07-07 17:27:37]

The CQ bit was checked by reed@google.com

[commit-bot: I haz the power, 2014-07-07 17:28:39]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/reed@google.com/374633002/1

[msw, 2014-07-07 19:47:51]

https://codereview.chromium.org/374633002/diff/1/ui/gfx/android/java_bitmap.h
File ui/gfx/android/java_bitmap.h (right):

https://codereview.chromium.org/374633002/diff/1/ui/gfx/android/java_bitmap.h...
ui/gfx/android/java_bitmap.h:58: SkColorType color_type);
Why is there no corresponding change to the signatures and implementations of
these two functions in ui/gfx/android/java_bitmap.cc?

https://codereview.chromium.org/374633002/diff/1/ui/gfx/color_analysis_unitte...
File ui/gfx/color_analysis_unittest.cc (right):

https://codereview.chromium.org/374633002/diff/1/ui/gfx/color_analysis_unitte...
ui/gfx/color_analysis_unittest.cc:152: DCHECK(bitmap.colorType() ==
kAlpha_8_SkColorType);
nit: DCHECK_EQ

[reed1, 2014-07-07 19:51:47]

https://codereview.chromium.org/374633002/diff/1/ui/gfx/android/java_bitmap.h
File ui/gfx/android/java_bitmap.h (right):

https://codereview.chromium.org/374633002/diff/1/ui/gfx/android/java_bitmap.h...
ui/gfx/android/java_bitmap.h:58: SkColorType color_type);
On 2014/07/07 19:47:51, msw wrote:
> Why is there no corresponding change to the signatures and implementations of
> these two functions in ui/gfx/android/java_bitmap.cc?

Good catch. I'm updating those files now (didn't compile).

https://codereview.chromium.org/374633002/diff/1/ui/gfx/color_analysis_unitte...
File ui/gfx/color_analysis_unittest.cc (right):

https://codereview.chromium.org/374633002/diff/1/ui/gfx/color_analysis_unitte...
ui/gfx/color_analysis_unittest.cc:152: DCHECK(bitmap.colorType() ==
kAlpha_8_SkColorType);
On 2014/07/07 19:47:51, msw wrote:
> nit: DCHECK_EQ

Done.

[reed2, 2014-07-08 01:01:48]

ptal

[msw, 2014-07-08 01:11:29]

https://codereview.chromium.org/374633002/diff/60001/ui/gfx/android/java_bitm...
File ui/gfx/android/java_bitmap.cc (right):

https://codereview.chromium.org/374633002/diff/60001/ui/gfx/android/java_bitm...
ui/gfx/android/java_bitmap.cc:61: ScopedJavaLocalRef<jobject>
CreateJavaBitmap(int width,
Doesn't this need to be updated like the declaration?

https://codereview.chromium.org/374633002/diff/60001/ui/gfx/android/java_bitm...
ui/gfx/android/java_bitmap.cc:88: ScopedJavaLocalRef<jobject> jbitmap =
CreateJavaBitmap(
Doesn't this call need to be updated?

https://codereview.chromium.org/374633002/diff/60001/ui/gfx/android/java_bitm...
File ui/gfx/android/java_bitmap.h (right):

https://codereview.chromium.org/374633002/diff/60001/ui/gfx/android/java_bitm...
ui/gfx/android/java_bitmap.h:54: // (non-empty!) size and configuration.
nit: update comment s/configuration/color type/.

https://codereview.chromium.org/374633002/diff/60001/ui/gfx/android/java_bitm...
ui/gfx/android/java_bitmap.h:55: GFX_EXPORT
base::android::ScopedJavaLocalRef<jobject> CreateJavaBitmap(
Update the call in content/browser/android/overscroll_glow.cc

https://codereview.chromium.org/374633002/diff/60001/ui/gfx/android/java_bitm...
ui/gfx/android/java_bitmap.h:77: // Returns a Skia config value for the
requested input java Bitmap.Config.
nit: update comment s/Skia config value/Skia color type/.

[reed1, 2014-07-08 12:50:31]

The CQ bit was checked by reed@google.com

[reed1, 2014-07-08 12:50:32]

The CQ bit was unchecked by reed@google.com

[reed1, 2014-07-08 12:50:34]

The CQ bit was checked by reed@google.com

[reed1, 2014-07-08 12:50:40]

The CQ bit was unchecked by reed@google.com

[reed1, 2014-07-08 13:01:57]

https://codereview.chromium.org/374633002/diff/60001/ui/gfx/android/java_bitm...
File ui/gfx/android/java_bitmap.cc (right):

https://codereview.chromium.org/374633002/diff/60001/ui/gfx/android/java_bitm...
ui/gfx/android/java_bitmap.cc:61: ScopedJavaLocalRef<jobject>
CreateJavaBitmap(int width,
On 2014/07/08 01:11:29, msw wrote:
> Doesn't this need to be updated like the declaration?

Done.

https://codereview.chromium.org/374633002/diff/60001/ui/gfx/android/java_bitm...
ui/gfx/android/java_bitmap.cc:88: ScopedJavaLocalRef<jobject> jbitmap =
CreateJavaBitmap(
On 2014/07/08 01:11:29, msw wrote:
> Doesn't this call need to be updated?

Done.

https://codereview.chromium.org/374633002/diff/60001/ui/gfx/android/java_bitm...
File ui/gfx/android/java_bitmap.h (right):

https://codereview.chromium.org/374633002/diff/60001/ui/gfx/android/java_bitm...
ui/gfx/android/java_bitmap.h:54: // (non-empty!) size and configuration.
On 2014/07/08 01:11:29, msw wrote:
> nit: update comment s/configuration/color type/.

Done.

https://codereview.chromium.org/374633002/diff/60001/ui/gfx/android/java_bitm...
ui/gfx/android/java_bitmap.h:54: // (non-empty!) size and configuration.
On 2014/07/08 01:11:29, msw wrote:
> nit: update comment s/configuration/color type/.

Done.

https://codereview.chromium.org/374633002/diff/60001/ui/gfx/android/java_bitm...
ui/gfx/android/java_bitmap.h:55: GFX_EXPORT
base::android::ScopedJavaLocalRef<jobject> CreateJavaBitmap(
On 2014/07/08 01:11:29, msw wrote:
> Update the call in content/browser/android/overscroll_glow.cc

I don't see a reference to CreateJavaBitmap in that file. What am I missing?

https://codereview.chromium.org/374633002/diff/60001/ui/gfx/android/java_bitm...
ui/gfx/android/java_bitmap.h:77: // Returns a Skia config value for the
requested input java Bitmap.Config.
On 2014/07/08 01:11:29, msw wrote:
> nit: update comment s/Skia config value/Skia color type/.

Done.

[reed1, 2014-07-08 13:21:22]

The CQ bit was checked by reed@google.com

[commit-bot: I haz the power, 2014-07-08 13:22:39]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/reed@google.com/374633002/80001

[reed1, 2014-07-08 13:54:24]

ptal

[commit-bot: I haz the power, 2014-07-08 16:31:14]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)

[commit-bot: I haz the power, 2014-07-08 16:33:32]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-07-08 16:33:34]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/chromium_presubmit/bu...)

[msw, 2014-07-08 16:52:07]

lgtm with a nit

https://codereview.chromium.org/374633002/diff/60001/ui/gfx/android/java_bitm...
File ui/gfx/android/java_bitmap.h (right):

https://codereview.chromium.org/374633002/diff/60001/ui/gfx/android/java_bitm...
ui/gfx/android/java_bitmap.h:55: GFX_EXPORT
base::android::ScopedJavaLocalRef<jobject> CreateJavaBitmap(
On 2014/07/08 13:01:57, reed1 wrote:
> On 2014/07/08 01:11:29, msw wrote:
> > Update the call in content/browser/android/overscroll_glow.cc
> 
> I don't see a reference to CreateJavaBitmap in that file. What am I missing?

Ah, sorry, that calls CreateJavaBitmapFromAndroidResource.

https://codereview.chromium.org/374633002/diff/80001/ui/gfx/android/java_bitm...
File ui/gfx/android/java_bitmap.cc (right):

https://codereview.chromium.org/374633002/diff/80001/ui/gfx/android/java_bitm...
ui/gfx/android/java_bitmap.cc:50: case kN32_SkColorType:
nit q: this kN32_SkColorType could be kBGRA_8888_SkColorType and then yields
BITMAP_FORMAT_ARGB_8888, should this switch handle the possible channel ordering
difference?

[reed1, 2014-07-08 17:02:37]

https://codereview.chromium.org/374633002/diff/80001/ui/gfx/android/java_bitm...
File ui/gfx/android/java_bitmap.cc (right):

https://codereview.chromium.org/374633002/diff/80001/ui/gfx/android/java_bitm...
ui/gfx/android/java_bitmap.cc:50: case kN32_SkColorType:
On 2014/07/08 16:52:07, msw wrote:
> nit q: this kN32_SkColorType could be kBGRA_8888_SkColorType and then yields
> BITMAP_FORMAT_ARGB_8888, should this switch handle the possible channel
ordering
> difference?

kN32_SkColorType is an exact match for the legacy SkBitmap::kARGB_8888_Config :
both map to whatever the compile-time swizzle was set for skia's renderer. I
think this is the right change for this CL.

Part of the motivation for switching to SkColorType (and SkImageInfo in the
larger sense) is to allow for exactly this sort of expansion : supporting
natively RGBA and BGRA in the same build (and premul and unpremul). That change
will come later.

[reed1, 2014-07-08 17:14:13]

The CQ bit was checked by reed@google.com

[commit-bot: I haz the power, 2014-07-08 17:16:45]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/reed@google.com/374633002/80001

[commit-bot: I haz the power, 2014-07-08 19:09:57]

Change committed as 281783