cc: Fix BytesPerPixel issue and refactor.

1. With introduction of compressed formats, e.g. ETC1, the number of
bits per pixel is becoming less than 8. Computing bytes per pixel by
BitsPerPixel(format) / 8, might give different values. So compute the
bits per row first and divide it by 8 to get the bytes per row.

2. Move resource size computation functions to separate file,
i.e. resource_util.cc.

3. Move resource format related functions to resource_format.h. As
there is dependecy on third_party/khronos/GLES2/gl2.h &
third_party/khronos/GLES2/gl2ext.h, the inlined functions have been
written as non-inlined functions and header files have been included
in resource_format.cc.
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/8e4942838460a8d66a31c46baf265a5a8b61a16a
Cr-Commit-Position: refs/heads/master@{#341492}

[prashant.n, 2015-06-24 16:26:26]

prashant.n@samsung.com changed reviewers:
+ danakj@chromium.org, enne@chromium.org, reveman@chromium.org

[prashant.n, 2015-06-24 16:26:26]

PTAL.

[reveman, 2015-06-24 18:02:26]

https://codereview.chromium.org/1202843008/diff/40001/cc/resources/resource.h
File cc/resources/resource.h (left):

https://codereview.chromium.org/1202843008/diff/40001/cc/resources/resource.h...
cc/resources/resource.h:55: size.height() / 8;
hm, looks like this was broken for height < 8 before this, that's pretty bad.

https://codereview.chromium.org/1202843008/diff/40001/cc/resources/resource.h
File cc/resources/resource.h (right):

https://codereview.chromium.org/1202843008/diff/40001/cc/resources/resource.h...
cc/resources/resource.h:56: 8;
Can you add a temporary bytes_per_row variable here to make this a bit easier to
read?

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

https://codereview.chromium.org/1202843008/diff/40001/cc/resources/resource_p...
cc/resources/resource_provider.cc:1528: base::CheckedNumeric<int> num_bytes =
resource->size.height();
why CheckedNumeric?

https://codereview.chromium.org/1202843008/diff/40001/cc/resources/resource_p...
cc/resources/resource_provider.cc:1529: int bytes_per_row_aligned =
How about just bytes_per_row and round up to multiple of 4 in the call below as
before?

https://codereview.chromium.org/1202843008/diff/40001/cc/resources/video_reso...
File cc/resources/video_resource_updater.cc (right):

https://codereview.chromium.org/1202843008/diff/40001/cc/resources/video_reso...
cc/resources/video_resource_updater.cc:310: size_t bits_per_pixel =
BitsPerPixel(plane_resource.resource_format);
Not sure we need to change this. we have an explicit yuv_resource_format check
above so we know that bpp is 8 or greater.

[vmpstr, 2015-06-24 18:46:13]

vmpstr@chromium.org changed reviewers:
+ vmpstr@chromium.org

[vmpstr, 2015-06-24 18:46:14]

https://codereview.chromium.org/1202843008/diff/40001/cc/resources/resource.h
File cc/resources/resource.h (left):

https://codereview.chromium.org/1202843008/diff/40001/cc/resources/resource.h...
cc/resources/resource.h:55: size.height() / 8;
On 2015/06/24 18:02:26, reveman wrote:
> hm, looks like this was broken for height < 8 before this, that's pretty bad.

I think the order of multiplicative operations is well defined to be left to
right, so this was probably OK.

[prashant.n, 2015-06-25 15:23:04]

PTAL

https://codereview.chromium.org/1202843008/diff/40001/cc/resources/video_reso...
File cc/resources/video_resource_updater.cc (right):

https://codereview.chromium.org/1202843008/diff/40001/cc/resources/video_reso...
cc/resources/video_resource_updater.cc:310: size_t bits_per_pixel =
BitsPerPixel(plane_resource.resource_format);
On 2015/06/24 18:02:26, reveman wrote:
> Not sure we need to change this. we have an explicit yuv_resource_format check
> above so we know that bpp is 8 or greater.
I think for consistency it is okay.

https://codereview.chromium.org/1202843008/diff/60001/cc/resources/resource_u...
File cc/resources/resource_util.h (right):

https://codereview.chromium.org/1202843008/diff/60001/cc/resources/resource_u...
cc/resources/resource_util.h:17: // that fits in a size_t.
These are just the functions moved out resource_provider.h + one new function
added for aligned data.

When grouped all these functions together, IMO we should have some different
function names than these.

reveman@, vmpstr@, wdyt?

I also think of adding one more function for BytesPerRow? Should we add it?

[prashant.n, 2015-06-25 15:26:02]

prashant.n@samsung.com changed reviewers:
+ epenner@chromium.org

[prashant.n, 2015-07-01 17:18:29]

PTAL.

[prashant.n, 2015-07-14 03:31:34]

On 2015/07/01 17:18:29, prashant.n wrote:
> PTAL.

Anybody back to review this?

[vmpstr, 2015-07-14 17:44:15]

https://codereview.chromium.org/1202843008/diff/120001/cc/resources/resource_...
File cc/resources/resource_format.h (right):

https://codereview.chromium.org/1202843008/diff/120001/cc/resources/resource_...
cc/resources/resource_format.h:13: typedef unsigned int GLenum;
Can you instead include one of the following? (whichever provides GLenum)

"third_party/khronos/GLES2/gl2.h"
"third_party/khronos/GLES2/gl2ext.h"

https://codereview.chromium.org/1202843008/diff/120001/cc/resources/resource_...
File cc/resources/resource_util.cc (right):

https://codereview.chromium.org/1202843008/diff/120001/cc/resources/resource_...
cc/resources/resource_util.cc:27: size_t ResourceUtil::CheckedSizeInBytes(const
gfx::Size& size,
I'd weakly prefer SizeInBytes to be the checked version and UncheckedSizeInBytes
to be the unchecked one. This makes it clear that if the author is using
Unchecked, then they have reasoned out why we don't have to worry about
overflow.

[prashant.n, 2015-07-15 07:03:31]

https://codereview.chromium.org/1202843008/diff/120001/cc/resources/resource_...
File cc/resources/resource_util.cc (right):

https://codereview.chromium.org/1202843008/diff/120001/cc/resources/resource_...
cc/resources/resource_util.cc:27: size_t ResourceUtil::CheckedSizeInBytes(const
gfx::Size& size,
On 2015/07/14 17:44:15, vmpstr wrote:
> I'd weakly prefer SizeInBytes to be the checked version and
UncheckedSizeInBytes
> to be the unchecked one. This makes it clear that if the author is using
> Unchecked, then they have reasoned out why we don't have to worry about
> overflow. 

You mean to say - instead of SizeInBytes, use UncheckedSizeInBytes?

[vmpstr, 2015-07-15 17:43:23]

https://codereview.chromium.org/1202843008/diff/120001/cc/resources/resource_...
File cc/resources/resource_util.cc (right):

https://codereview.chromium.org/1202843008/diff/120001/cc/resources/resource_...
cc/resources/resource_util.cc:27: size_t ResourceUtil::CheckedSizeInBytes(const
gfx::Size& size,
On 2015/07/15 07:03:31, prashant.n wrote:
> On 2015/07/14 17:44:15, vmpstr wrote:
> > I'd weakly prefer SizeInBytes to be the checked version and
> UncheckedSizeInBytes
> > to be the unchecked one. This makes it clear that if the author is using
> > Unchecked, then they have reasoned out why we don't have to worry about
> > overflow. 
> 
> You mean to say - instead of SizeInBytes, use UncheckedSizeInBytes?

Yeah, just a function rename. If it's just Foo it should be safe (protected
against overflow), if it's UncheckedFoo, then it's clear that this function can
overflow if the input isn't controlled.

[prashant.n, 2015-07-15 18:22:09]

> > 
> > You mean to say - instead of SizeInBytes, use UncheckedSizeInBytes?
> 
> Yeah, just a function rename. If it's just Foo it should be safe (protected
> against overflow), if it's UncheckedFoo, then it's clear that this function
can
> overflow if the input isn't controlled.

I understand now, I'll push the patch by changing names.

[prashant.n, 2015-07-16 05:54:02]

PTAL

https://codereview.chromium.org/1202843008/diff/120001/cc/resources/resource_...
File cc/resources/resource_format.h (right):

https://codereview.chromium.org/1202843008/diff/120001/cc/resources/resource_...
cc/resources/resource_format.h:13: typedef unsigned int GLenum;
On 2015/07/14 17:44:15, vmpstr wrote:
> Can you instead include one of the following? (whichever provides GLenum)
> 
> "third_party/khronos/GLES2/gl2.h"
> "third_party/khronos/GLES2/gl2ext.h"

Including third_party/khronos/GLES2/gl2.h causes redefinition errors with
third_party/mesa/src/include/GL/gl.h.

[vmpstr, 2015-07-16 19:04:00]

https://codereview.chromium.org/1202843008/diff/160001/cc/resources/resource_...
File cc/resources/resource_format.cc (right):

https://codereview.chromium.org/1202843008/diff/160001/cc/resources/resource_...
cc/resources/resource_format.cc:52: static const unsigned
format_gl_data_type[RESOURCE_FORMAT_MAX + 1] = {
nit: GLenum instead of unsigned

https://codereview.chromium.org/1202843008/diff/160001/cc/resources/resource_...
cc/resources/resource_format.cc:67: static const unsigned
format_gl_data_format[RESOURCE_FORMAT_MAX + 1] = {
nit: same here.

https://codereview.chromium.org/1202843008/diff/160001/cc/resources/resource_...
File cc/resources/resource_format.h (right):

https://codereview.chromium.org/1202843008/diff/160001/cc/resources/resource_...
cc/resources/resource_format.h:12: // TODO(prashant.n): Remove GLenum typedef.
Can you file a bug for this and reference it in the comment

https://codereview.chromium.org/1202843008/diff/160001/cc/resources/resource_...
cc/resources/resource_format.h:32: CC_EXPORT extern int
BitsPerPixel(ResourceFormat format);
I don't think you need extern here?

https://codereview.chromium.org/1202843008/diff/160001/cc/resources/resource_...
File cc/resources/resource_provider.cc (right):

https://codereview.chromium.org/1202843008/diff/160001/cc/resources/resource_...
cc/resources/resource_provider.cc:721: int image_bytes =
ResourceUtil::CheckedSizeInBytes(image_size, ETC1);
This will cause a possible truncation size_t -> int. I think you need to keep an
explicit CheckedNumeric here, since CompressedTexImage2D takes an int.

https://codereview.chromium.org/1202843008/diff/160001/cc/resources/resource_...
cc/resources/resource_provider.cc:1524: int resource_bytes =
ResourceUtil::UncheckedSizeInBytesAligned(
I don't think this will compile on windows because of a truncation warning.

[prashant.n, 2015-07-22 15:44:58]

PTAL

https://codereview.chromium.org/1202843008/diff/160001/cc/resources/resource_...
File cc/resources/resource_provider.cc (right):

https://codereview.chromium.org/1202843008/diff/160001/cc/resources/resource_...
cc/resources/resource_provider.cc:1524: int resource_bytes =
ResourceUtil::UncheckedSizeInBytesAligned(
On 2015/07/16 19:04:00, vmpstr wrote:
> I don't think this will compile on windows because of a truncation warning.

I'll change this to size_t. But BufferData takes singed long int for second
parameter, which again can lead to truncation when actually passed to function.
But currently resource size is upto (512*512*4), so we may not get the
truncation problem.

[vmpstr, 2015-07-22 18:05:55]

Other than the comment below, this change looks good.

https://codereview.chromium.org/1202843008/diff/180001/cc/resources/resource_...
File cc/resources/resource_util.h (right):

https://codereview.chromium.org/1202843008/diff/180001/cc/resources/resource_...
cc/resources/resource_util.h:18: static int CheckedSizeInBytes(const gfx::Size&
size, ResourceFormat format);
It's a bit awkward that the Checked version returns an int and an Unchecked one
returns a size_t. Can you do one of the following options:

1. Make everything be size_t, at the call site where you want an int, do a
CHECK_LE(result, static_cast<size_t>(std::numeric_limits<int>::max()));
followed by a static_cast to int. 

or

2. Make these functions be templated, so the use would be

ResourceUtil::CheckedInBytes<int>(...);
and
ResourceUtil::CheckedInBytes<size_t>(...);

[prashant.n, 2015-07-23 05:27:10]

https://codereview.chromium.org/1202843008/diff/180001/cc/resources/resource_...
File cc/resources/resource_util.h (right):

https://codereview.chromium.org/1202843008/diff/180001/cc/resources/resource_...
cc/resources/resource_util.h:18: static int CheckedSizeInBytes(const gfx::Size&
size, ResourceFormat format);
I thought of adding following code; but it has a drawback that if sizeof(T) >
sizeof(size_t) there will be truncation. But considering resource sizes, it may
not go beyond sizeof(unsigned int) i.e. 4 bytes unsigned data. So there will not
be problem. If this is okay, I'll add following code, otherwise we can have
CheckedSizeInBytes itself as a templated as discussed in your point 2. Do let me
know your opinion on this.

  
static size_t CheckedSizeInBytes(const gfx::Size& size,
                                   ResourceFormat format);  
static T CheckedSizeInBytes(const gfx::Size& size, ResourceFormat format) {
  static_assert(std::numeric_limits<T>::is_integer,
                    "T must be an integer type");
  size_t bytes = CheckedSizeInBytes(size, format);
  CHECK_LE(bytes, static_cast<size_t>(std::numeric_limits<T>::max()));
  return static_cast<T>(bytes);
}

[vmpstr, 2015-07-23 17:56:27]

https://codereview.chromium.org/1202843008/diff/180001/cc/resources/resource_...
File cc/resources/resource_util.h (right):

https://codereview.chromium.org/1202843008/diff/180001/cc/resources/resource_...
cc/resources/resource_util.h:18: static int CheckedSizeInBytes(const gfx::Size&
size, ResourceFormat format);
On 2015/07/23 05:27:10, prashant.n wrote:
> I thought of adding following code; but it has a drawback that if sizeof(T) >
> sizeof(size_t) there will be truncation. But considering resource sizes, it
may
> not go beyond sizeof(unsigned int) i.e. 4 bytes unsigned data. So there will
not
> be problem. If this is okay, I'll add following code, otherwise we can have
> CheckedSizeInBytes itself as a templated as discussed in your point 2. Do let
me
> know your opinion on this.
> 
>   
> static size_t CheckedSizeInBytes(const gfx::Size& size,
>                                    ResourceFormat format);  
> static T CheckedSizeInBytes(const gfx::Size& size, ResourceFormat format) {
>   static_assert(std::numeric_limits<T>::is_integer,
>                     "T must be an integer type");
>   size_t bytes = CheckedSizeInBytes(size, format);
>   CHECK_LE(bytes, static_cast<size_t>(std::numeric_limits<T>::max()));
>   return static_cast<T>(bytes);
> }

What about the following?

template <typename T>
T ResourceUtil::CheckedSizeInBytes(const gfx::Size& size,
                                   ResourceFormat format) {
  DCHECK(VerifySizeInBytes<T>(size, format));
  base::CheckedNumeric<T> checked_value = BitsPerPixel(format);
  checked_value *= size.width();
  checked_value *= size.height();
  checked_value /= 8;
  return checked_value.ValueOrDie();
}

Would that work?

[prashant.n, 2015-07-24 06:28:36]

PTAL.

[ericrk, 2015-07-24 20:03:46]

On 2015/07/24 06:28:36, prashant.n wrote:
> PTAL.

Taking a look as well. Overall LGTM with a few nits.

[ericrk, 2015-07-24 20:03:51]

ericrk@chromium.org changed reviewers:
+ ericrk@chromium.org

[ericrk, 2015-07-24 20:03:52]

https://codereview.chromium.org/1202843008/diff/200001/cc/resources/resource_...
File cc/resources/resource_format.cc (right):

https://codereview.chromium.org/1202843008/diff/200001/cc/resources/resource_...
cc/resources/resource_format.cc:52: static const GLenum
format_gl_data_type[RESOURCE_FORMAT_MAX + 1] = {
There's actually no warning/error generated if you underfill this array (if
RESOURCE_FORMAT_MAX increases).

One alternative that might be safer is:

static const GLenum format_gl_data_type[] = {...};
static_assert(arraysize(format_gl_data_type) == RESOURCE_FORMAT_MAX + 1,
"format_gl_data_type does not handle all cases.");

or something like that. same below.

https://codereview.chromium.org/1202843008/diff/200001/cc/resources/resource_...
File cc/resources/resource_util.cc (right):

https://codereview.chromium.org/1202843008/diff/200001/cc/resources/resource_...
cc/resources/resource_util.cc:34: DCHECK(VerifySizeInBytes<size_t>(size,
format));
Super nit (feel free to ignore): the following can still fail even if this check
passed or the caller pre-checked, as the rounded up width could cause math
overflow vs the non-rounded-up one used in the checks.

Same problem above, but almost impossible to hit, as you'd have to be within 4
of the limit.

https://codereview.chromium.org/1202843008/diff/200001/cc/resources/resource_...
File cc/resources/resource_util.h (right):

https://codereview.chromium.org/1202843008/diff/200001/cc/resources/resource_...
cc/resources/resource_util.h:51: static size_t UncheckedWidthInBytes(size_t
width, ResourceFormat format);
I like the templatization - can you templatize the rest of these as well for
consistency?

[prashant.n, 2015-07-25 07:15:43]

https://codereview.chromium.org/1202843008/diff/200001/cc/resources/resource_...
File cc/resources/resource_format.cc (right):

https://codereview.chromium.org/1202843008/diff/200001/cc/resources/resource_...
cc/resources/resource_format.cc:52: static const GLenum
format_gl_data_type[RESOURCE_FORMAT_MAX + 1] = {
Sounds good. I'll incorporate this.

https://codereview.chromium.org/1202843008/diff/200001/cc/resources/resource_...
File cc/resources/resource_util.cc (right):

https://codereview.chromium.org/1202843008/diff/200001/cc/resources/resource_...
cc/resources/resource_util.cc:34: DCHECK(VerifySizeInBytes<size_t>(size,
format));
Hmm, I'll check on this.

https://codereview.chromium.org/1202843008/diff/200001/cc/resources/resource_...
File cc/resources/resource_util.h (right):

https://codereview.chromium.org/1202843008/diff/200001/cc/resources/resource_...
cc/resources/resource_util.h:51: static size_t UncheckedWidthInBytes(size_t
width, ResourceFormat format);
Hmm, I had thought of this earlier, but then we should prevent users using other
than size_t. I'll check how we can do it and update.

[prashant.n, 2015-07-27 17:37:55]

All review comments addressed, but few unittests are failing on my machine.
Checking if that is due to this change. Until I check, kindly hold on this
patch.

[prashant.n, 2015-07-28 15:47:32]

PTAL at newly added functions and integral width portion. (Now all unit tests
are passing.)

https://codereview.chromium.org/1202843008/diff/300001/cc/resources/resource_...
File cc/resources/resource_util.h (right):

https://codereview.chromium.org/1202843008/diff/300001/cc/resources/resource_...
cc/resources/resource_util.h:94: checked_value =
MathUtil::RoundUp<T>(checked_value.ValueOrDie(), 8);
Need to ensure that width is integral value.

If BitsPerPixel = 4 e.g. ETC1, and width is odd, last pixel should not be
truncated.

[prashant.n, 2015-07-28 16:19:45]

Looks we may need to write unit test for resource_util functions.

[prashant.n, 2015-07-29 16:27:45]

Now PTAL.

[ericrk, 2015-07-29 18:19:24]

Looks good. Thanks for adding the tests!

https://codereview.chromium.org/1202843008/diff/300001/cc/resources/resource_...
File cc/resources/resource_util.h (right):

https://codereview.chromium.org/1202843008/diff/300001/cc/resources/resource_...
cc/resources/resource_util.h:16: #define VERIFY_INTEGER_TYPE                    
    \
This should be undefined at the end of the file to prevent it from leaking into
the global namespace.

Another option would be to move the template implementations to the cc file and
just explicitly define the int/size_t instantiations here, which would lead to a
linker error if someone tried to use this with a non size-t/int type.

Would be a bit cleaner maybe, but up to you.

https://codereview.chromium.org/1202843008/diff/300001/cc/resources/resource_...
cc/resources/resource_util.h:94: checked_value =
MathUtil::RoundUp<T>(checked_value.ValueOrDie(), 8);
On 2015/07/28 15:47:32, prashant.n wrote:
> Need to ensure that width is integral value.
> 
> If BitsPerPixel = 4 e.g. ETC1, and width is odd, last pixel should not be
> truncated.

If we were within 8 of MAX_INT, RoundUp could overflow... I don't think we are
likely to hit this, but it would be nice to have checked roudning logic for
completeness. At least a TODO would be good.

https://codereview.chromium.org/1202843008/diff/300001/cc/resources/resource_...
cc/resources/resource_util.h:145: checked_value =
MathUtil::RoundUp<T>(checked_value.ValueOrDie(), 8);
same as above

[enne (OOO), 2015-07-29 18:21:21]

https://codereview.chromium.org/1202843008/diff/300001/cc/resources/resource_...
File cc/resources/resource_util.h (right):

https://codereview.chromium.org/1202843008/diff/300001/cc/resources/resource_...
cc/resources/resource_util.h:16: #define VERIFY_INTEGER_TYPE                    
    \
Yeah, I'd prefer not having a #define in a header file.

[prashant.n, 2015-07-30 18:45:02]

Please review and let me your opinion for moving template definitions to .cc.

https://codereview.chromium.org/1202843008/diff/300001/cc/resources/resource_...
File cc/resources/resource_util.h (right):

https://codereview.chromium.org/1202843008/diff/300001/cc/resources/resource_...
cc/resources/resource_util.h:16: #define VERIFY_INTEGER_TYPE                    
    \
1. Removed macro.

2. Moving functions in .cc would have problem of having unneeded defintions.
e.g. CheckedSizeInBytes<size_t> is not needed. We can skip instantiating those,
but header file definitions would be more scalable.

Your point regarding avoiding other than size_t and int also looks good.

Kindly vote your opinion.

https://codereview.chromium.org/1202843008/diff/300001/cc/resources/resource_...
cc/resources/resource_util.h:94: checked_value =
MathUtil::RoundUp<T>(checked_value.ValueOrDie(), 8);
Handled and TODO added.

[vmpstr, 2015-07-30 20:50:58]

lgtm stamp when ericrk is happy with it.

https://codereview.chromium.org/1202843008/diff/380001/cc/resources/resource_...
File cc/resources/resource_util.h (right):

https://codereview.chromium.org/1202843008/diff/380001/cc/resources/resource_...
cc/resources/resource_util.h:156: std::numeric_limits<T>::is_integer &&
!std::is_same<T, bool>::value,
std::is_same is a c++11 library feature, which we can't use right now. I would
recommend doing sizeof(T) > 1 or something like that.

[ericrk, 2015-07-30 22:19:03]

On 2015/07/30 20:50:58, vmpstr wrote:
> lgtm stamp when ericrk is happy with it.
> 
>
https://codereview.chromium.org/1202843008/diff/380001/cc/resources/resource_...
> File cc/resources/resource_util.h (right):
> 
>
https://codereview.chromium.org/1202843008/diff/380001/cc/resources/resource_...
> cc/resources/resource_util.h:156: std::numeric_limits<T>::is_integer &&
> !std::is_same<T, bool>::value,
> std::is_same is a c++11 library feature, which we can't use right now. I would
> recommend doing sizeof(T) > 1 or something like that.

lgtm pending vmpstr's comment.

[prashant.n, 2015-07-31 06:05:13]

The CQ bit was checked by prashant.n@samsung.com

[prashant.n, 2015-07-31 06:05:14]

The patchset sent to the CQ was uploaded after l-g-t-m from vmpstr@chromium.org,
ericrk@chromium.org
Link to the patchset: https://codereview.chromium.org/1202843008/#ps400001
(title: "Replaced std::is_same by local definition.")

[commit-bot: I haz the power, 2015-07-31 06:05:28]

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

[commit-bot: I haz the power, 2015-07-31 06:07:37]

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

[commit-bot: I haz the power, 2015-07-31 06:07:38]

Try jobs failed on following builders:
  linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[prashant.n, 2015-07-31 09:34:28]

The CQ bit was checked by prashant.n@samsung.com to run a CQ dry run

[commit-bot: I haz the power, 2015-07-31 09:34:46]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1202843008/420001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1202843008/420001

[commit-bot: I haz the power, 2015-07-31 09:43:56]

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

[commit-bot: I haz the power, 2015-07-31 09:43:58]

Dry run: 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...)

[prashant.n, 2015-07-31 11:05:22]

The CQ bit was checked by prashant.n@samsung.com

[prashant.n, 2015-07-31 11:05:22]

The patchset sent to the CQ was uploaded after l-g-t-m from vmpstr@chromium.org,
ericrk@chromium.org
Link to the patchset: https://codereview.chromium.org/1202843008/#ps460001
(title: "Rebase errors.")

[commit-bot: I haz the power, 2015-07-31 11:05:47]

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

[commit-bot: I haz the power, 2015-07-31 11:58:44]

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

[commit-bot: I haz the power, 2015-07-31 11:58:45]

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_...)

[prashant.n, 2015-07-31 13:07:37]

The CQ bit was checked by prashant.n@samsung.com

[prashant.n, 2015-07-31 13:07:38]

The patchset sent to the CQ was uploaded after l-g-t-m from vmpstr@chromium.org,
ericrk@chromium.org
Link to the patchset: https://codereview.chromium.org/1202843008/#ps480001
(title: "Win build break; video_frame->stride returns int only.")

[commit-bot: I haz the power, 2015-07-31 13:07:52]

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

[commit-bot: I haz the power, 2015-07-31 15:25:34]

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

[commit-bot: I haz the power, 2015-07-31 15:25:36]

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

[prashant.n, 2015-08-01 09:59:17]

The CQ bit was checked by prashant.n@samsung.com

[prashant.n, 2015-08-01 09:59:17]

The patchset sent to the CQ was uploaded after l-g-t-m from vmpstr@chromium.org,
ericrk@chromium.org
Link to the patchset: https://codereview.chromium.org/1202843008/#ps500001
(title: "Fixed Android build break.")

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

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

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

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

[commit-bot: I haz the power, 2015-08-01 11:13:10]

Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[prashant.n, 2015-08-03 05:52:38]

The CQ bit was checked by prashant.n@samsung.com

[commit-bot: I haz the power, 2015-08-03 05:52:51]

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

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

Committed patchset #26 (id:500001)

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

Patchset 26 (id:??) landed as
https://crrev.com/8e4942838460a8d66a31c46baf265a5a8b61a16a
Cr-Commit-Position: refs/heads/master@{#341492}