cc: Fix the format of GpuMemoryBuffer for SurfaceTexture

Change switches::kContentImageTextureTarget to a list of image texture targets

Not knowing the GpuMemoryBuffer format that the compostor will use, the host is
hard to determine the image texture target properly. This CL changes
switches::kContentImageTextureTarget to a list of targets for each format, so
that the compositor can decide what format it wants to use without having to
worry about the target being wrong.

BUG=490362, 512665
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/0ffadce32916c6c4d7343b2d263d8e75606b7311
Cr-Commit-Position: refs/heads/master@{#342339}

[jchen10, 2015-07-22 07:39:35]

jie.a.chen@intel.com changed reviewers:
+ reveman@chromium.org

[jchen10, 2015-07-22 07:40:42]

Could you please have a look! Thanks!

[reveman, 2015-07-22 13:43:39]

https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/re...
is supposed to handle this already for the renderer. Is that not working?

[jchen10, 2015-07-23 06:48:24]

On 2015/07/22 13:43:39, reveman wrote:
>
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/re...
> is supposed to handle this already for the renderer. Is that not working?

Oh! BRGA_8888 is hardcoded there. But SurfaceTexture only support RGBA_8888. See
https://code.google.com/p/chromium/codesearch#chromium/src/content/common/gpu...

I've updated the CL to force to use RGBA_8888 on Android. Please have a look!

[reveman, 2015-07-23 07:48:39]

https://codereview.chromium.org/1251693003/diff/20001/content/browser/rendere...
File content/browser/renderer_host/render_process_host_impl.cc (right):

https://codereview.chromium.org/1251693003/diff/20001/content/browser/rendere...
content/browser/renderer_host/render_process_host_impl.cc:1151: #else
Let's get this fixed properly instead. The problem is that we're trying to
determine what target to use here without knowing the format that the compositor
will use. It's hard to know the format here as it depends on available
extensions.

I think the best solution is to let the compositor know what the target is for
each format. This way the compositor can decide what format it wants to use
without having to worry about the target being wrong.

Let's change the value of switches::kContentImageTextureTarget into a list of
ints. Each int will be the target to use for a supported GMB format. Something
similar to this:

https://code.google.com/p/chromium/codesearch#chromium/src/gpu/config/gpu_uti...

[jchen10, 2015-07-27 12:38:43]

On 2015/07/23 07:48:39, reveman wrote:
>
https://codereview.chromium.org/1251693003/diff/20001/content/browser/rendere...
> File content/browser/renderer_host/render_process_host_impl.cc (right):
> 
>
https://codereview.chromium.org/1251693003/diff/20001/content/browser/rendere...
> content/browser/renderer_host/render_process_host_impl.cc:1151: #else
> Let's get this fixed properly instead. The problem is that we're trying to
> determine what target to use here without knowing the format that the
compositor
> will use. It's hard to know the format here as it depends on available
> extensions.
> 
> I think the best solution is to let the compositor know what the target is for
> each format. This way the compositor can decide what format it wants to use
> without having to worry about the target being wrong.
> 
> Let's change the value of switches::kContentImageTextureTarget into a list of
> ints. Each int will be the target to use for a supported GMB format. Something
> similar to this:
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/gpu/config/gpu_uti...

I've updated the CL according to your comment. I'm hoping this is close to what
you meant.
Please take a look! Thanks a lot!

[reveman, 2015-07-27 17:07:16]

This looks great. Some nits below.

https://codereview.chromium.org/1251693003/diff/40001/cc/resources/resource_p...
File cc/resources/resource_pool.h (right):

https://codereview.chromium.org/1251693003/diff/40001/cc/resources/resource_p...
cc/resources/resource_pool.h:22: std::vector<uint> targets) {
nit: here and everywhere else in this patch "std::vector<unsigned>". that or
uint32 is preferred. we use unsigned for texture target in general as that what
GL.h defines GLenum to as I think that's best.

https://codereview.chromium.org/1251693003/diff/40001/cc/resources/resource_p...
cc/resources/resource_pool.h:55: ResourcePool(ResourceProvider*
resource_provider, std::vector<uint> targets);
Instead of having to pass the target vector to the ResourcePool, can we keep
this in the ResourceProvider and just have a helper function there to get the
correct target for a specific resource format?

https://codereview.chromium.org/1251693003/diff/40001/content/browser/rendere...
File content/browser/renderer_host/render_process_host_impl.cc (right):

https://codereview.chromium.org/1251693003/diff/40001/content/browser/rendere...
content/browser/renderer_host/render_process_host_impl.cc:403: std::string rt;
nit: maybe s/rt/str/ to be consistent with StringToUintVector

https://codereview.chromium.org/1251693003/diff/40001/content/browser/rendere...
content/browser/renderer_host/render_process_host_impl.cc:405: it !=
vector.end(); ++it) {
nit: for (auto it : vector) { ...

https://codereview.chromium.org/1251693003/diff/40001/content/renderer/render...
File content/renderer/render_thread_impl.cc (right):

https://codereview.chromium.org/1251693003/diff/40001/content/renderer/render...
content/renderer/render_thread_impl.cc:421: DCHECK(vector);
This DCHECK seems overkill. Can you instead DCHECK that the vector is empty as
we assume that when using push_back below?

https://codereview.chromium.org/1251693003/diff/40001/content/renderer/render...
content/renderer/render_thread_impl.cc:424: DCHECK(pieces.size() ==
gfx::GpuMemoryBuffer::FORMAT_LAST + 1);
nit: use DCHECK_EQ

https://codereview.chromium.org/1251693003/diff/40001/ui/compositor/composito...
File ui/compositor/compositor.cc (right):

https://codereview.chromium.org/1251693003/diff/40001/ui/compositor/composito...
ui/compositor/compositor.cc:147: settings.use_image_texture_targets[format] =
nit: DCHECK_GT(settings.use_image_texture_targets.size(), format) before this
line

[jchen10, 2015-07-28 07:01:03]

On 2015/07/27 17:07:16, reveman wrote:
> This looks great. Some nits below.
> 
>
https://codereview.chromium.org/1251693003/diff/40001/cc/resources/resource_p...
> File cc/resources/resource_pool.h (right):
> 
>
https://codereview.chromium.org/1251693003/diff/40001/cc/resources/resource_p...
> cc/resources/resource_pool.h:22: std::vector<uint> targets) {
> nit: here and everywhere else in this patch "std::vector<unsigned>". that or
> uint32 is preferred. we use unsigned for texture target in general as that
what
> GL.h defines GLenum to as I think that's best.
> 
>
https://codereview.chromium.org/1251693003/diff/40001/cc/resources/resource_p...
> cc/resources/resource_pool.h:55: ResourcePool(ResourceProvider*
> resource_provider, std::vector<uint> targets);
> Instead of having to pass the target vector to the ResourcePool, can we keep
> this in the ResourceProvider and just have a helper function there to get the
> correct target for a specific resource format?
> 
>
https://codereview.chromium.org/1251693003/diff/40001/content/browser/rendere...
> File content/browser/renderer_host/render_process_host_impl.cc (right):
> 
>
https://codereview.chromium.org/1251693003/diff/40001/content/browser/rendere...
> content/browser/renderer_host/render_process_host_impl.cc:403: std::string rt;
> nit: maybe s/rt/str/ to be consistent with StringToUintVector
> 
>
https://codereview.chromium.org/1251693003/diff/40001/content/browser/rendere...
> content/browser/renderer_host/render_process_host_impl.cc:405: it !=
> vector.end(); ++it) {
> nit: for (auto it : vector) { ...
> 
>
https://codereview.chromium.org/1251693003/diff/40001/content/renderer/render...
> File content/renderer/render_thread_impl.cc (right):
> 
>
https://codereview.chromium.org/1251693003/diff/40001/content/renderer/render...
> content/renderer/render_thread_impl.cc:421: DCHECK(vector);
> This DCHECK seems overkill. Can you instead DCHECK that the vector is empty as
> we assume that when using push_back below?
> 
>
https://codereview.chromium.org/1251693003/diff/40001/content/renderer/render...
> content/renderer/render_thread_impl.cc:424: DCHECK(pieces.size() ==
> gfx::GpuMemoryBuffer::FORMAT_LAST + 1);
> nit: use DCHECK_EQ
> 
>
https://codereview.chromium.org/1251693003/diff/40001/ui/compositor/composito...
> File ui/compositor/compositor.cc (right):
> 
>
https://codereview.chromium.org/1251693003/diff/40001/ui/compositor/composito...
> ui/compositor/compositor.cc:147: settings.use_image_texture_targets[format] =
> nit: DCHECK_GT(settings.use_image_texture_targets.size(), format) before this
> line

Thanks for reviewing the patch. Please take another review on the new patch-set
based on all your comments.

[reveman, 2015-07-28 08:58:24]

reveman@chromium.org changed reviewers:
+ piman@chromium.org

[reveman, 2015-07-28 08:58:26]

thanks!

lgtm with nits

+piman for ui/compositor/ and content/

https://codereview.chromium.org/1251693003/diff/60001/cc/resources/resource_p...
File cc/resources/resource_pool.h (right):

https://codereview.chromium.org/1251693003/diff/60001/cc/resources/resource_p...
cc/resources/resource_pool.h:59: ResourcePool(ResourceProvider*
resource_provider);
nit: explicit ResourcePool(ResourceProv...

https://codereview.chromium.org/1251693003/diff/60001/cc/resources/resource_p...
cc/resources/resource_pool.h:60: ResourcePool(ResourceProvider*
resource_provider, GLenum target);
Can we remove this ctor with "target" and and always use the image texture
target?

https://codereview.chromium.org/1251693003/diff/60001/cc/resources/resource_p...
File cc/resources/resource_provider.cc (right):

https://codereview.chromium.org/1251693003/diff/60001/cc/resources/resource_p...
cc/resources/resource_provider.cc:397: std::vector<unsigned>
use_image_texture_targets) {
nit: const std::vector<unsigned>&

https://codereview.chromium.org/1251693003/diff/60001/cc/resources/resource_p...
cc/resources/resource_provider.cc:1086: std::vector<unsigned>
use_image_texture_targets)
nit: const std::vector<unsigned>&

https://codereview.chromium.org/1251693003/diff/60001/cc/resources/resource_p...
cc/resources/resource_provider.cc:1941: gfx::GpuMemoryBuffer::Format buf_fmt =
ToGpuMemoryBufferFormat(format);
nit: remove this variable or avoid abbreviating. s/buf_fmt/buffer_format/

https://codereview.chromium.org/1251693003/diff/60001/cc/resources/resource_p...
File cc/resources/resource_provider.h (right):

https://codereview.chromium.org/1251693003/diff/60001/cc/resources/resource_p...
cc/resources/resource_provider.h:89: std::vector<unsigned>
use_image_texture_targets);
nit: const std::vector<unsigned>&

https://codereview.chromium.org/1251693003/diff/60001/cc/resources/resource_p...
cc/resources/resource_provider.h:452: std::vector<unsigned>
use_image_texture_targets);
nit: const std::vector<unsigned>&

https://codereview.chromium.org/1251693003/diff/60001/cc/trees/layer_tree_set...
File cc/trees/layer_tree_settings.h (right):

https://codereview.chromium.org/1251693003/diff/60001/cc/trees/layer_tree_set...
cc/trees/layer_tree_settings.h:82: // an array of image texture targets for each
GpuMemoryBuffer format
nit: s/an array../An array/ and s/format/format./

[piman, 2015-07-28 16:33:22]

https://codereview.chromium.org/1251693003/diff/60001/cc/trees/layer_tree_set...
File cc/trees/layer_tree_settings.h (right):

https://codereview.chromium.org/1251693003/diff/60001/cc/trees/layer_tree_set...
cc/trees/layer_tree_settings.h:83: std::vector<unsigned>
use_image_texture_targets;
Why not an array? The size is known statically.

[jchen10, 2015-07-29 04:33:48]

https://codereview.chromium.org/1251693003/diff/60001/cc/resources/resource_p...
File cc/resources/resource_pool.h (right):

https://codereview.chromium.org/1251693003/diff/60001/cc/resources/resource_p...
cc/resources/resource_pool.h:60: ResourcePool(ResourceProvider*
resource_provider, GLenum target);
On 2015/07/28 08:58:25, reveman wrote:
> Can we remove this ctor with "target" and and always use the image texture
> target?

I'm afraid we can't. The image texture is only used by the zero-copy and
one-copy tile task worker pools. Other pools(including the one-copy) need to use
GL_TEXTURE_2D target. The functions that can be used for GL_TEXTURE_2D are not
guaranteed by other texture targets, e.g. "gl*Tex*Image*()" with
GL_TEXTURE_EXTERNAL_OES(See
https://www.khronos.org/registry/gles/extensions/OES/OES_EGL_image_external.txt).
Actually I have ever tried it, but met some wired gfx error.

https://codereview.chromium.org/1251693003/diff/60001/cc/trees/layer_tree_set...
File cc/trees/layer_tree_settings.h (right):

https://codereview.chromium.org/1251693003/diff/60001/cc/trees/layer_tree_set...
cc/trees/layer_tree_settings.h:83: std::vector<unsigned>
use_image_texture_targets;
On 2015/07/28 16:33:22, piman (Very slow to review) wrote:
> Why not an array? The size is known statically.

I had wished to use an array from the start. But gave up it in order to keep the
patch more concise, since vector is handier than raw array. The initial thoughts
at that moment were:
1. The initializer function vector(count, value) could be used frequently in the
patch, so the code to re-invent it for array could be saved.
2. Vector can be passed as an parameter either by value or reference. Whereas
you have to manually copy all elements for an array in case of pass-by-value.
3. No need to include "ui/gfx/gpu_memory_buffer.h" everywhere merely for the
array size defined in it.
4. Performance penalty should be insignificant because there are a very small
number of elements.

[jchen10, 2015-07-29 04:47:40]

The CQ bit was unchecked by jie.a.chen@intel.com

[piman, 2015-07-29 05:24:41]

lgtm

[jchen10, 2015-07-30 02:36:35]

The CQ bit was checked by jie.a.chen@intel.com

[jchen10, 2015-07-30 02:36:35]

The patchset sent to the CQ was uploaded after l-g-t-m from reveman@chromium.org
Link to the patchset: https://codereview.chromium.org/1251693003/#ps80001
(title: "Fix nits")

[commit-bot: I haz the power, 2015-07-30 02:36:52]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1251693003/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1251693003/80001

[reveman, 2015-07-30 02:41:43]

https://codereview.chromium.org/1251693003/diff/60001/cc/resources/resource_p...
File cc/resources/resource_pool.h (right):

https://codereview.chromium.org/1251693003/diff/60001/cc/resources/resource_p...
cc/resources/resource_pool.h:60: ResourcePool(ResourceProvider*
resource_provider, GLenum target);
On 2015/07/29 at 04:33:47, jchen10 wrote:
> On 2015/07/28 08:58:25, reveman wrote:
> > Can we remove this ctor with "target" and and always use the image texture
> > target?
> 
> I'm afraid we can't. The image texture is only used by the zero-copy and
one-copy tile task worker pools. Other pools(including the one-copy) need to use
GL_TEXTURE_2D target. The functions that can be used for GL_TEXTURE_2D are not
guaranteed by other texture targets, e.g. "gl*Tex*Image*()" with
GL_TEXTURE_EXTERNAL_OES(See
https://www.khronos.org/registry/gles/extensions/OES/OES_EGL_image_external.txt).
Actually I have ever tried it, but met some wired gfx error.

Ok. No need to do anything in this patch but I think we should change this so
the user of this API will always have to ask for a specific target when
acquiring a resource.

[commit-bot: I haz the power, 2015-07-30 02:47:37]

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

[commit-bot: I haz the power, 2015-07-30 02:47:38]

Try jobs failed on following builders:
  linux_blink_rel on tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/7...)

[jchen10, 2015-07-30 05:15:32]

The CQ bit was checked by jie.a.chen@intel.com

[jchen10, 2015-07-30 05:15:33]

The patchset sent to the CQ was uploaded after l-g-t-m from
reveman@chromium.org, piman@chromium.org
Link to the patchset: https://codereview.chromium.org/1251693003/#ps100001
(title: "Fix tests")

[commit-bot: I haz the power, 2015-07-30 05:16:04]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1251693003/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1251693003/100001

[commit-bot: I haz the power, 2015-07-30 05:25:59]

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

[commit-bot: I haz the power, 2015-07-30 05:26:00]

Try jobs failed on following builders:
  cast_shell_linux on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[jchen10, 2015-07-30 06:08:13]

The CQ bit was checked by jie.a.chen@intel.com

[jchen10, 2015-07-30 06:08:13]

The patchset sent to the CQ was uploaded after l-g-t-m from
reveman@chromium.org, piman@chromium.org
Link to the patchset: https://codereview.chromium.org/1251693003/#ps120001
(title: "Fix content tests")

[commit-bot: I haz the power, 2015-07-30 06:09:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1251693003/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1251693003/120001

[commit-bot: I haz the power, 2015-07-30 06:25:52]

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

[commit-bot: I haz the power, 2015-07-30 06:25:52]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[jchen10, 2015-07-30 06:47:58]

The CQ bit was checked by jie.a.chen@intel.com

[jchen10, 2015-07-30 06:47:58]

The patchset sent to the CQ was uploaded after l-g-t-m from
reveman@chromium.org, piman@chromium.org
Link to the patchset: https://codereview.chromium.org/1251693003/#ps140001
(title: "Fix a type mismatch on Win")

[commit-bot: I haz the power, 2015-07-30 06:48:30]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1251693003/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1251693003/140001

[commit-bot: I haz the power, 2015-07-30 07:07:39]

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

[commit-bot: I haz the power, 2015-07-30 07:07:40]

Try jobs failed on following builders:
  android_chromium_gn_compile_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)

[jchen10, 2015-07-30 08:05:41]

The CQ bit was checked by jie.a.chen@intel.com

[jchen10, 2015-07-30 08:05:42]

The patchset sent to the CQ was uploaded after l-g-t-m from
reveman@chromium.org, piman@chromium.org
Link to the patchset: https://codereview.chromium.org/1251693003/#ps180001
(title: "Fix aligment")

[commit-bot: I haz the power, 2015-07-30 08:06:01]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1251693003/180001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1251693003/180001

[commit-bot: I haz the power, 2015-07-30 08:14:14]

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

[commit-bot: I haz the power, 2015-07-30 08:14:15]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[jchen10, 2015-07-30 08:36:13]

jie.a.chen@intel.com changed reviewers:
+ sky@chromium.org

[jchen10, 2015-07-30 08:45:20]

On 2015/07/30 08:14:15, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

Hi sky,
    Could you please take a look! "+1" from you, the owner of html_view, is
mandatory for this CL.

[jchen10, 2015-07-31 05:11:11]

jie.a.chen@intel.com changed reviewers:
+ jiajia.qin@intel.com

[jchen10, 2015-07-31 05:22:28]

jie.a.chen@intel.com changed reviewers:
- jiajia.qin@intel.com

[jchen10, 2015-07-31 05:26:34]

I have made a few new changes since your "LGTM". Please take a look! Thanks a
lot!

https://codereview.chromium.org/1251693003/diff/190001/content/renderer/rende...
File content/renderer/render_thread_impl.cc (right):

https://codereview.chromium.org/1251693003/diff/190001/content/renderer/rende...
content/renderer/render_thread_impl.cc:1463:
cmd_line->GetSwitchValueASCII(switches::kVideoImageTextureTarget);
It looks like an typo using switches::kContentImageTextureTarget here. Instead
switches::kVideoImageTextureTarget seems defined but not used.

Please review this change!

[jchen10, 2015-07-31 05:27:37]

The CQ bit was checked by jie.a.chen@intel.com

[jchen10, 2015-07-31 05:27:37]

The patchset sent to the CQ was uploaded after l-g-t-m from
reveman@chromium.org, piman@chromium.org
Link to the patchset: https://codereview.chromium.org/1251693003/#ps190001
(title: "Fix kVideoImageTextureTarget")

[commit-bot: I haz the power, 2015-07-31 05:27:42]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1251693003/190001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1251693003/190001

[commit-bot: I haz the power, 2015-07-31 05:36:56]

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

[commit-bot: I haz the power, 2015-07-31 05:36:57]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[reveman, 2015-07-31 16:05:43]

lgtm + some minor suggestions that you can ignore if you like

https://codereview.chromium.org/1251693003/diff/190001/cc/resources/resource_...
File cc/resources/resource_provider_unittest.cc (right):

https://codereview.chromium.org/1251693003/diff/190001/cc/resources/resource_...
cc/resources/resource_provider_unittest.cc:46: const std::vector<unsigned>
g_imaget_targets(gfx::GpuMemoryBuffer::FORMAT_LAST +
nit: s/g_imaget_targets/g_image_texture_targets/

maybe make this a ResourceProviderTest member instead?

https://codereview.chromium.org/1251693003/diff/190001/cc/test/pixel_test.cc
File cc/test/pixel_test.cc (right):

https://codereview.chromium.org/1251693003/diff/190001/cc/test/pixel_test.cc#...
cc/test/pixel_test.cc:38: GL_TEXTURE_2D);
same here, member of the Test class instead?

https://codereview.chromium.org/1251693003/diff/190001/content/renderer/rende...
File content/renderer/render_thread_impl.cc (right):

https://codereview.chromium.org/1251693003/diff/190001/content/renderer/rende...
content/renderer/render_thread_impl.cc:1463:
cmd_line->GetSwitchValueASCII(switches::kVideoImageTextureTarget);
On 2015/07/31 at 05:26:33, jchen10 wrote:
> It looks like an typo using switches::kContentImageTextureTarget here. Instead
switches::kVideoImageTextureTarget seems defined but not used.
> 
> Please review this change!

Nice catch. Thanks!

[jchen10, 2015-08-01 08:47:08]

The CQ bit was checked by jie.a.chen@intel.com

[jchen10, 2015-08-01 08:47:09]

The patchset sent to the CQ was uploaded after l-g-t-m from
reveman@chromium.org, piman@chromium.org
Link to the patchset: https://codereview.chromium.org/1251693003/#ps210001
(title: "Fix the global variables")

[commit-bot: I haz the power, 2015-08-01 08:47:17]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1251693003/210001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1251693003/210001

[commit-bot: I haz the power, 2015-08-01 08:54:59]

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

[commit-bot: I haz the power, 2015-08-01 08:55:00]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[jchen10, 2015-08-02 07:49:26]

The CQ bit was checked by jie.a.chen@intel.com

[commit-bot: I haz the power, 2015-08-02 07:49:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1251693003/210001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1251693003/210001

[commit-bot: I haz the power, 2015-08-02 07:56:36]

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

[commit-bot: I haz the power, 2015-08-02 07:56:37]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[jchen10, 2015-08-04 05:27:50]

On 2015/08/02 07:56:37, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

Hi sky,
    Could you take a look at the change in html_viewer? It's simple and plain.
You just need to spend a few minutes on it. Thanks a lot!

[sky, 2015-08-04 13:22:01]

LGTM

[reveman, 2015-08-04 13:37:27]

The CQ bit was checked by reveman@chromium.org

[commit-bot: I haz the power, 2015-08-04 13:38:01]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1251693003/210001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1251693003/210001

[commit-bot: I haz the power, 2015-08-04 14:38:23]

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

[commit-bot: I haz the power, 2015-08-04 14:38:25]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[reveman, 2015-08-04 16:58:58]

The CQ bit was checked by reveman@chromium.org

[commit-bot: I haz the power, 2015-08-04 16:59:13]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1251693003/210001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1251693003/210001

[commit-bot: I haz the power, 2015-08-04 18:46:07]

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

[commit-bot: I haz the power, 2015-08-04 18:46:08]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[jchen10, 2015-08-04 23:58:07]

The CQ bit was checked by jie.a.chen@intel.com

[commit-bot: I haz the power, 2015-08-04 23:59:27]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1251693003/210001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1251693003/210001

[commit-bot: I haz the power, 2015-08-05 00:09:51]

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

[commit-bot: I haz the power, 2015-08-05 00:09:53]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[jchen10, 2015-08-06 13:16:49]

The CQ bit was checked by jie.a.chen@intel.com

[jchen10, 2015-08-06 13:16:49]

The patchset sent to the CQ was uploaded after l-g-t-m from
reveman@chromium.org, sky@chromium.org, piman@chromium.org
Link to the patchset: https://codereview.chromium.org/1251693003/#ps250001
(title: "Fix the public data member")

[commit-bot: I haz the power, 2015-08-06 13:17:07]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1251693003/250001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1251693003/250001

[commit-bot: I haz the power, 2015-08-06 14:54:01]

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

[commit-bot: I haz the power, 2015-08-06 14:54:02]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[reveman, 2015-08-06 17:01:41]

The CQ bit was checked by reveman@chromium.org

[commit-bot: I haz the power, 2015-08-06 17:01:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1251693003/250001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1251693003/250001

[commit-bot: I haz the power, 2015-08-06 18:47:29]

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

[commit-bot: I haz the power, 2015-08-06 18:47:31]

Failed to apply patch for cc/resources/resource_pool.cc:
While running git apply --index -3 -p1;
  error: patch failed: cc/resources/resource_pool.cc:10
  Falling back to three-way merge...
  Applied patch to 'cc/resources/resource_pool.cc' with conflicts.
  U cc/resources/resource_pool.cc

Patch:       cc/resources/resource_pool.cc
Index: cc/resources/resource_pool.cc
diff --git a/cc/resources/resource_pool.cc b/cc/resources/resource_pool.cc
index
300c48f3d652a94458fa868f37c18da42995a933..1a17e18b4b7d067efed937f889f36c46163c5771
100644
--- a/cc/resources/resource_pool.cc
+++ b/cc/resources/resource_pool.cc
@@ -10,6 +10,16 @@
 
 namespace cc {
 
+ResourcePool::ResourcePool(ResourceProvider* resource_provider)
+    : resource_provider_(resource_provider),
+      target_(0),
+      max_memory_usage_bytes_(0),
+      max_unused_memory_usage_bytes_(0),
+      max_resource_count_(0),
+      memory_usage_bytes_(0),
+      unused_memory_usage_bytes_(0),
+      resource_count_(0) {}
+
 ResourcePool::ResourcePool(ResourceProvider* resource_provider, GLenum target)
     : resource_provider_(resource_provider),
       target_(target),
@@ -18,7 +28,9 @@ ResourcePool::ResourcePool(ResourceProvider*
resource_provider, GLenum target)
       max_resource_count_(0),
       memory_usage_bytes_(0),
       unused_memory_usage_bytes_(0),
-      resource_count_(0) {}
+      resource_count_(0) {
+  DCHECK_NE(0u, target);
+}
 
 ResourcePool::~ResourcePool() {
   while (!busy_resources_.empty()) {
@@ -55,7 +67,9 @@ scoped_ptr<ScopedResource> ResourcePool::AcquireResource(
 
   scoped_ptr<ScopedResource> resource =
       ScopedResource::Create(resource_provider_);
-  resource->AllocateManaged(size, target_, format);
+  GLenum target =
+      target_ ? target_ : resource_provider_->GetImageTextureTarget(format);
+  resource->AllocateManaged(size, target, format);
 
   DCHECK(ResourceUtil::VerifySizeInBytes<size_t>(resource->size(),
                                                  resource->format()));

[commit-bot: I haz the power, 2015-08-06 18:48:51]

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

[jchen10, 2015-08-07 08:46:29]

The CQ bit was checked by jie.a.chen@intel.com

[commit-bot: I haz the power, 2015-08-07 08:46:39]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1251693003/250001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1251693003/250001

[commit-bot: I haz the power, 2015-08-07 08:51:32]

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

[commit-bot: I haz the power, 2015-08-07 08:51:34]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[jchen10, 2015-08-07 11:45:50]

The CQ bit was checked by jie.a.chen@intel.com

[jchen10, 2015-08-07 11:45:51]

The patchset sent to the CQ was uploaded after l-g-t-m from
reveman@chromium.org, sky@chromium.org, piman@chromium.org
Link to the patchset: https://codereview.chromium.org/1251693003/#ps270001
(title: "rebase")

[commit-bot: I haz the power, 2015-08-07 11:46:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1251693003/270001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1251693003/270001

[commit-bot: I haz the power, 2015-08-07 12:59:33]

Committed patchset #15 (id:270001)

[commit-bot: I haz the power, 2015-08-07 13:00:16]

Patchset 15 (id:??) landed as
https://crrev.com/0ffadce32916c6c4d7343b2d263d8e75606b7311
Cr-Commit-Position: refs/heads/master@{#342339}