Make the AwPixelInfo more skia version independent

Make the AwPixelInfo more skia version independent

Previously the config and clip_region field formats depended on skia
internal implementation details. This patch decouples the interface
from the specific skia version in use.


BUG=

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=216841

[Kristian Monsen, 2013-07-25 03:35:17]

lgtm, but InProcessViewRenderer::DrawSWInternal is getting so large now I think
it could be beneficial to create some sub functions?

https://codereview.chromium.org/20234002/diff/4001/android_webview/browser/in...
File android_webview/browser/in_process_view_renderer.cc (right):

https://codereview.chromium.org/20234002/diff/4001/android_webview/browser/in...
android_webview/browser/in_process_view_renderer.cc:279: const gfx::Rect& clip)
{
This is now only used for the early out, do we still need it?

https://codereview.chromium.org/20234002/diff/4001/android_webview/browser/in...
android_webview/browser/in_process_view_renderer.cc:295: JNIEnv* env =
AttachCurrentThread();
I think this is only needed for the kNo_Config?

https://codereview.chromium.org/20234002/diff/4001/android_webview/public/bro...
File android_webview/public/browser/draw_sw.h (right):

https://codereview.chromium.org/20234002/diff/4001/android_webview/public/bro...
android_webview/public/browser/draw_sw.h:17: static const int
kAwPixelInfoVersion = 2;
Is the versioning for future compatibility reasons?

[joth, 2013-08-04 06:04:08]

Now rebased on https://codereview.chromium.org/22035002/ -- will land that one
first

https://codereview.chromium.org/20234002/diff/4001/android_webview/browser/in...
File android_webview/browser/in_process_view_renderer.cc (right):

https://codereview.chromium.org/20234002/diff/4001/android_webview/browser/in...
android_webview/browser/in_process_view_renderer.cc:279: const gfx::Rect& clip)
{
On 2013/07/25 03:35:17, Kristian Monsen wrote:
> This is now only used for the early out, do we still need it?

still needed for the RasterizeIntoBitmap() call

https://codereview.chromium.org/20234002/diff/4001/android_webview/browser/in...
android_webview/browser/in_process_view_renderer.cc:295: JNIEnv* env =
AttachCurrentThread();
On 2013/07/25 03:35:17, Kristian Monsen wrote:
> I think this is only needed for the kNo_Config?

Done.

https://codereview.chromium.org/20234002/diff/4001/android_webview/public/bro...
File android_webview/public/browser/draw_sw.h (right):

https://codereview.chromium.org/20234002/diff/4001/android_webview/public/bro...
android_webview/public/browser/draw_sw.h:17: static const int
kAwPixelInfoVersion = 2;
On 2013/07/25 03:35:17, Kristian Monsen wrote:
> Is the versioning for future compatibility reasons?

yeah, I removed it from this patch, will do that separately (and consistently)

[joth, 2013-08-10 00:54:50]

https://codereview.chromium.org/20234002/diff/4001/android_webview/browser/in...
File android_webview/browser/in_process_view_renderer.cc (right):

https://codereview.chromium.org/20234002/diff/4001/android_webview/browser/in...
android_webview/browser/in_process_view_renderer.cc:295: JNIEnv* env =
AttachCurrentThread();
On 2013/08/04 06:04:08, joth wrote:
> On 2013/07/25 03:35:17, Kristian Monsen wrote:
> > I think this is only needed for the kNo_Config?
> 
> Done.
...actually, is needed for sw_functions->access_pixel

https://codereview.chromium.org/20234002/diff/4001/android_webview/public/bro...
File android_webview/public/browser/draw_sw.h (right):

https://codereview.chromium.org/20234002/diff/4001/android_webview/public/bro...
android_webview/public/browser/draw_sw.h:17: static const int
kAwPixelInfoVersion = 2;
On 2013/08/04 06:04:08, joth wrote:
> On 2013/07/25 03:35:17, Kristian Monsen wrote:
> > Is the versioning for future compatibility reasons?
> 
> yeah, I removed it from this patch, will do that separately (and consistently)

reneged: it's more convenient having versioning info in each struct than
remembering to update a global version, so I left it here.

[sgurun-gerrit only, 2013-08-10 01:04:47]

lgtm

https://codereview.chromium.org/20234002/diff/15001/android_webview/browser/i...
File android_webview/browser/in_process_view_renderer.cc (right):

https://codereview.chromium.org/20234002/diff/15001/android_webview/browser/i...
android_webview/browser/in_process_view_renderer.cc:127: private:
Need DISALLOW_COPY_AND_ASSIGN

https://codereview.chromium.org/20234002/diff/15001/android_webview/browser/i...
android_webview/browser/in_process_view_renderer.cc:441: config =
feels like a switch would be more readable...

[joth, 2013-08-10 01:22:32]

https://codereview.chromium.org/20234002/diff/15001/android_webview/browser/i...
File android_webview/browser/in_process_view_renderer.cc (right):

https://codereview.chromium.org/20234002/diff/15001/android_webview/browser/i...
android_webview/browser/in_process_view_renderer.cc:127: private:
On 2013/08/10 01:04:47, sgurun wrote:
> Need DISALLOW_COPY_AND_ASSIGN

Done.

https://codereview.chromium.org/20234002/diff/15001/android_webview/browser/i...
android_webview/browser/in_process_view_renderer.cc:441: config =
On 2013/08/10 01:04:47, sgurun wrote:
> feels like a switch would be more readable...

Done.

[joth, 2013-08-10 01:34:54]

+boliu for a committer +2

[boliu, 2013-08-10 01:41:40]

lgtm

https://codereview.chromium.org/20234002/diff/22001/android_webview/browser/i...
File android_webview/browser/in_process_view_renderer.cc (right):

https://codereview.chromium.org/20234002/diff/22001/android_webview/browser/i...
android_webview/browser/in_process_view_renderer.cc:461: if (config ==
SkBitmap::kNo_Config) {
can this be the else block to "if (pixels)" on line 442?

[joth, 2013-08-10 01:45:48]

I think this might be clearer split into more sub-functions and perhaps a
dedicated SW render .cc file. But that's for another CL. (And, milestone)

https://codereview.chromium.org/20234002/diff/22001/android_webview/browser/i...
File android_webview/browser/in_process_view_renderer.cc (right):

https://codereview.chromium.org/20234002/diff/22001/android_webview/browser/i...
android_webview/browser/in_process_view_renderer.cc:461: if (config ==
SkBitmap::kNo_Config) {
On 2013/08/10 01:41:40, boliu wrote:
> can this be the else block to "if (pixels)" on line 442?

no - line 445, 448 and 458 all assign to |config| and so will affect the outcome

[commit-bot: I haz the power, 2013-08-10 01:46:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/joth@chromium.org/20234002/25001

[commit-bot: I haz the power, 2013-08-10 02:08:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/joth@chromium.org/20234002/38001

[commit-bot: I haz the power, 2013-08-10 13:48:59]

Change committed as 216841