Use ImageSkiaSource to create ImageSkia from ImagePNGReps

Use ImageSkiaSource to create ImageSkia from ImagePNGReps
 gfx::ImageKia will fetch the ImageSkiaRep based on resource scale factor (supported one) and scale accordingly.

BUG=381601
TEST=covered by test.
R=ananta@chromium.org, pkotwicz@chromium.org, rsesek@chromium.org

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

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

[oshima, 2014-06-18 00:28:06]

ananta, can you try this on windows and see if this fixes favicon quality? (on
both tab strip and bookmark)

[pkotwicz, 2014-06-18 02:22:10]

I will look at this more in depth later. At first glance this look wrong.
Favicons are resized to the requested size upon being requested. See the
implementation of SelectFaviconFramesFromPNGs().

[oshima, 2014-06-18 04:11:22]

On 2014/06/18 02:22:10, pkotwicz wrote:
> I will look at this more in depth later. At first glance this look wrong.
> Favicons are resized to the requested size upon being requested. See the
> implementation of SelectFaviconFramesFromPNGs().

Favicon may be requested either with the requested scale factor, or
with resource scale factors (1.0 and 2.0f, for example).

In DSF = 1.5 environment, this may either generate ImageSkia with 1.5, or
ImageSkia with 1.0 and 2.0f.
In either case, ImageSkia do the right thing when 1.5 is requested.

[pkotwicz, 2014-06-18 04:54:19]

Ok, I now understand what you are trying to do. I think that this CL is a good
CL.

It is annoying that the code paths will be different for:
FaviconService::GetRawFavicon(). Used in the web contents. Request is done with
potentially fractional scale factor. The resampled bitmap should be stored in
the database. (We currently do not store resampled bitmaps in the database but
we should)
FaviconService::GetFaviconImage(). Used for Chrome UI. Request is done with
integer scale factor. ImageSkia resamples if necessary. The resampled bitmap is
not stored in the database. 

The CL will make favicons look better under some cases. However, we are adding a
third resizing step. There is a limit as to how good a bitmap which was resized
three times, (in some cases using low quality resizing per the UX designer
specs) can look.

[oshima, 2014-06-18 05:01:46]

On Tue, Jun 17, 2014 at 9:54 PM, <pkotwicz@chromium.org> wrote:

> Ok, I now understand what you are trying to do. I think that this CL is a
> good
> CL.
>
> It is annoying that the code paths will be different for:
> FaviconService::GetRawFavicon(). Used in the web contents. Request is
> done with
> potentially fractional scale factor. The resampled bitmap should be stored
> in
> the database. (We currently do not store resampled bitmaps in the database
> but
> we should)
> FaviconService::GetFaviconImage(). Used for Chrome UI. Request is done
> with
> integer scale factor. ImageSkia resamples if necessary. The resampled
> bitmap is
> not stored in the database.
>
> The CL will make favicons look better under some cases. However, we are
> adding a
> third resizing step. There is a limit as to how good a bitmap which was
> resized
> three times, (in some cases using low quality resizing per the UX designer
> specs) can look.
>
> https://codereview.chromium.org/340613004/


Yes, it's definitely doing more than necessary. I have some idea to improve
it (you shouldn't have to scale it at all less it's requested), but I
wanted to take safer path. (improve quality first, then improve efficiency)

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[pkotwicz, 2014-06-18 05:19:59]

This is how we get to 4 resizes (not 3)

1) user visits go/perf on computer (A). favicon.ico has a single favicon bitmap
of size 64x64.
- If the max supported scale factor is 2x,
ImageLoadingHelper::DidDownloadImage() will resize the bitmap to 32x32
- FaviconHandler::OnDidDownloadFavicon() generates favicon bitmaps for 1x and 2x
to store in the database
  32x32 -> 16x16 resize
  32x32 -> 32x32 no resize
2) The 16x16 version of the favicon is synced to computer (B)
- On computer B, FaviconService::GetFaviconImage() queries the history database
for the favicon for "go/perf". It resamples the favicon bitmaps to the
appropriate sizes for all supported scale factors.
  The history backend has only 16x16 (only 16x16 is synced)
  16x16 -> 16x16 no-resize
  16x16 -> 32x32 low quality resize (per UX designer spec)
3) If the UI is using a fractional scale factor (e.g. 1.8), your CL will add a
fourth resamplinh operation in ImageSkia
  32x32 -> 29x29

[oshima, 2014-06-18 05:26:50]

On 2014/06/18 05:19:59, pkotwicz wrote:
> This is how we get to 4 resizes (not 3)
> 
> 1) user visits go/perf on computer (A). favicon.ico has a single favicon
bitmap
> of size 64x64.
> - If the max supported scale factor is 2x,
> ImageLoadingHelper::DidDownloadImage() will resize the bitmap to 32x32

why this is necessary?

> - FaviconHandler::OnDidDownloadFavicon() generates favicon bitmaps for 1x and
2x
> to store in the database
>   32x32 -> 16x16 resize
>   32x32 -> 32x32 no resize
>
> 2) The 16x16 version of the favicon is synced to computer (B)
> - On computer B, FaviconService::GetFaviconImage() queries the history
database
> for the favicon for "go/perf". It resamples the favicon bitmaps to the
> appropriate sizes for all supported scale factors.
>   The history backend has only 16x16 (only 16x16 is synced)
>   16x16 -> 16x16 no-resize
>   16x16 -> 32x32 low quality resize (per UX designer spec)

This don't have to happen. The image source should simply return 1x if there is
only 1x, and gfx::ImageSkia should directly scale 16x16 to requested scale.

> 3) If the UI is using a fractional scale factor (e.g. 1.8), your CL will add a
> fourth resamplinh operation in ImageSkia
>   32x32 -> 29x29

[pkotwicz, 2014-06-18 14:22:48]

It is true that you can simplify SelectFaviconFramesFromPNGs() as a result of
your CL.

HOWEVER, you need to validate the change in resampling algorithm with the UX
designers. In particular we currently use nearest neighbor scaling when
resampling by an integer multiple. I can't find the original email from Cole,
but thakis@ references it in http://crbug.com/138550 in comment #3

It would also be nice to have a consistent default resampling algorithm. In
particular, FindRepresentation() in image_skia.cc uses a different resampling
algorithm than PNGImageSource.

[pkotwicz, 2014-06-18 14:25:06]

The integer multiple scaling algorithm matters because this is the algorithm
used to resample favicons which come from sync the first time that a user uses a
Chromebook Pixel

[oshima, 2014-06-18 15:46:47]

On 2014/06/18 14:22:48, pkotwicz wrote:
> It is true that you can simplify SelectFaviconFramesFromPNGs() as a result of
> your CL.
> 
> HOWEVER, you need to validate the change in resampling algorithm with the UX
> designers. In particular we currently use nearest neighbor scaling when
> resampling by an integer multiple. I can't find the original email from Cole,
> but thakis@ references it in http://crbug.com/138550 in comment #3
> 
> It would also be nice to have a consistent default resampling algorithm. In
> particular, FindRepresentation() in image_skia.cc uses a different resampling
> algorithm than PNGImageSource.

PNGImageSource does nothing to the image. It simply returns the ImageSkiaRep
decoded from PNGRep,
and this CL does not change the current behavior (ImageSkiaSource scales only if
the requested
scale and returned scale differs. That is, you requested 1.5 and get 2.0 (=
lancosz),
or you request 1.0 or 2.0 and you get the exact one (= no scaling).

[scottmg, 2014-06-18 20:31:27]

On 2014/06/18 00:28:06, oshima wrote:
> ananta, can you try this on windows and see if this fixes favicon quality? (on
> both tab strip and bookmark)

Seeing some strange behaviour on Windows at DSF=1.5 with this patch

https://screenshot.googleplex.com/mZVHT7vedN

Some seem unchanged and still double-scaled (? or something) and after some
reloads the nicer looking search one got blurry too

https://screenshot.googleplex.com/cA3xQKqrrv

[oshima, 2014-06-18 22:06:06]

On 2014/06/18 20:31:27, scottmg wrote:
> On 2014/06/18 00:28:06, oshima wrote:
> > ananta, can you try this on windows and see if this fixes favicon quality?
(on
> > both tab strip and bookmark)
> 
> Seeing some strange behaviour on Windows at DSF=1.5 with this patch
> 
> https://screenshot.googleplex.com/mZVHT7vedN

Seems like it uses 1x data (from sync?) first time. If you close the window and
open new one (and site supports high res favicon),
you should get the better result.
 
> Some seem unchanged and still double-scaled (? or something) and after some
> reloads the nicer looking search one got blurry too
> 
> https://screenshot.googleplex.com/cA3xQKqrrv

I don't know why that happened, but I think that's separate issue. I'll look
into these issues separately.

[pkotwicz, 2014-06-18 22:56:33]

Favicons get updated when you visit a site. So if the favicon looks bad because
it came from sync, if you visit the site it will look better. Closing the
browser and reopening the browser window should have no effect. If it does, then
it is likely a bug

The double scaling is on purpose and was requested by the UX designers. We may
want to change that but will need their blessing

[oshima, 2014-06-18 23:11:43]

On 2014/06/18 22:56:33, pkotwicz wrote:
> Favicons get updated when you visit a site. So if the favicon looks bad
because
> it came from sync, if you visit the site it will look better. Closing the
> browser and reopening the browser window should have no effect. If it does,
then
> it is likely a bug

I believe that is a separate bug and will address in separate CL, and
that's probably why the original reporter had an issue before my GetImageScale
removal.

> The double scaling is on purpose and was requested by the UX designers. We may
> want to change that but will need their blessing

[ananta, 2014-06-18 23:37:45]

LGTM. Cursory testing on the patch have symptoms already described by scottmg
and oshima.

[oshima, 2014-06-19 05:22:23]

On 2014/06/18 23:37:45, ananta wrote:
> LGTM. Cursory testing on the patch have symptoms already described by scottmg
> and oshima.

The issue where the very first time gets bad favicon is because it uses
different code path.
My next next CL should address it.

I still cannot reproduce the scott's 2nd issue and will keep looking.

[pkotwicz, 2014-06-19 14:16:13]

Looks good mostly.

You need to also change the implementation of ImageSkiaFromPNG() and
Get1xPNGBytesFromImageSkia() in image_ios.mm

You should reach out to the designers to figure out how favicons should be
resampled. If you can remove one of the in between resampling steps (e.g.
SelectFaviconFramesFromPNGs()) that would be very helpful to the quality of the
result.

https://codereview.chromium.org/340613004/diff/90001/ui/gfx/image/image.cc
File ui/gfx/image/image.cc (right):

https://codereview.chromium.org/340613004/diff/90001/ui/gfx/image/image.cc#ne...
ui/gfx/image/image.cc:76: const ImageSkiaRep* rep = NULL;
Can you please cleanup the comments. Based on the comments it seems as if we are
content with any ImageSkiaRep whose scale is larger than |scale|.

However, what we are trying to do is find the ImageSkiaRep with the smallest
scale which is still bigger or equal to |scale|.

https://codereview.chromium.org/340613004/diff/90001/ui/gfx/image/image.cc#ne...
ui/gfx/image/image.cc:150: const ImageSkia* image_skia) {
We should also do scaling here.

[oshima, 2014-06-19 15:50:41]

> You need to also change the implementation of ImageSkiaFromPNG() and
> Get1xPNGBytesFromImageSkia() in image_ios.mm

They only supports scales defined in ScaleFactor, so it won't be necessary.

>You should reach out to the designers to figure out how favicons should be
> resampled. If you can remove one of the in between resampling steps (e.g.
> SelectFaviconFramesFromPNGs()) that would be very helpful to the quality of
the
> result.

I had no plan to change how favicons are resmapled (it doesn't change now,
and i have an idea how to do this without multiple resampling steps), but I'll
check with Glen/Jenn.

https://codereview.chromium.org/340613004/diff/90001/ui/gfx/image/image.cc
File ui/gfx/image/image.cc (right):

https://codereview.chromium.org/340613004/diff/90001/ui/gfx/image/image.cc#ne...
ui/gfx/image/image.cc:76: const ImageSkiaRep* rep = NULL;
On 2014/06/19 14:16:13, pkotwicz wrote:
> Can you please cleanup the comments. Based on the comments it seems as if we
are
> content with any ImageSkiaRep whose scale is larger than |scale|.
> 
> However, what we are trying to do is find the ImageSkiaRep with the smallest
> scale which is still bigger or equal to |scale|.

will do.

https://codereview.chromium.org/340613004/diff/90001/ui/gfx/image/image.cc#ne...
ui/gfx/image/image.cc:150: const ImageSkia* image_skia) {
On 2014/06/19 14:16:13, pkotwicz wrote:
> We should also do scaling here.

can you explain why this is necessary?

[oshima, 2014-06-19 17:15:05]

https://codereview.chromium.org/340613004/diff/90001/ui/gfx/image/image.cc
File ui/gfx/image/image.cc (right):

https://codereview.chromium.org/340613004/diff/90001/ui/gfx/image/image.cc#ne...
ui/gfx/image/image.cc:76: const ImageSkiaRep* rep = NULL;
On 2014/06/19 15:50:40, oshima wrote:
> On 2014/06/19 14:16:13, pkotwicz wrote:
> > Can you please cleanup the comments. Based on the comments it seems as if we
> are
> > content with any ImageSkiaRep whose scale is larger than |scale|.
> > 
> > However, what we are trying to do is find the ImageSkiaRep with the smallest
> > scale which is still bigger or equal to |scale|.
> 
> will do.

Done. (your description wasn't exactly correct. please see the updated comment
and let me know if you still don't like it)

[pkotwicz, 2014-06-19 21:09:09]

On second though modifying ImageSkiaFromPNG() and Get1xPNGBytesFromImageSkia()
in image_ios.mm is not as easy as I thought. I am ok with not doing this.

https://codereview.chromium.org/340613004/diff/90001/ui/gfx/image/image.cc
File ui/gfx/image/image.cc (right):

https://codereview.chromium.org/340613004/diff/90001/ui/gfx/image/image.cc#ne...
ui/gfx/image/image.cc:150: const ImageSkia* image_skia) {
Doing resizing going PNG->ImageSkia and ImageSkia->PNG is nicely symmetric

My other interest is that implementing both makes it possible to simplify
SelectFaviconFramesFromPNGs() (pending approval by the designers about always
using Lanczos3 for resampling)

https://codereview.chromium.org/340613004/diff/150001/ui/gfx/image/image.cc
File ui/gfx/image/image.cc (right):

https://codereview.chromium.org/340613004/diff/150001/ui/gfx/image/image.cc#n...
ui/gfx/image/image.cc:77: // gfx::ImageSkia passes one of the resource scale
factor, and
How about: "gfx::ImageSkia passes one of the resource scale factors. The source
should return:
1) The ImageSkiaRep with the highest scale if all of the available scales ..."

Aside: It would be nice if we can use either 'scale factor' or 'scale' in the
function comments.

https://codereview.chromium.org/340613004/diff/150001/ui/gfx/image/image.cc#n...
ui/gfx/image/image.cc:98: const gfx::ImageSkiaRep rep = ToImageSkiaRep(png_rep);
Nit: the const here is unnecessary

[oshima, 2014-06-19 21:29:58]

https://codereview.chromium.org/340613004/diff/90001/ui/gfx/image/image.cc
File ui/gfx/image/image.cc (right):

https://codereview.chromium.org/340613004/diff/90001/ui/gfx/image/image.cc#ne...
ui/gfx/image/image.cc:150: const ImageSkia* image_skia) {
On 2014/06/19 21:09:09, pkotwicz wrote:
> Doing resizing going PNG->ImageSkia and ImageSkia->PNG is nicely symmetric

Sorry I still don't understand. This is supposed to get 1x. Why you need scaling
here?

It is true that GetRepresentation(1.0f) may not return 1.0x but it has nothing
to do with
this CL and it's been like that.

If you want to change to make sure it's always return 1.0f, then that should be
an separate issue.

> My other interest is that implementing both makes it possible to simplify
> SelectFaviconFramesFromPNGs() (pending approval by the designers about always
> using Lanczos3 for resampling)

I have a plan to simplify it (with current schemes). I'll also ask if we can
simply uses lancosz. I agree that using different algorithm may not make much
sense as we now support arbitrary scale factors.

https://codereview.chromium.org/340613004/diff/150001/ui/gfx/image/image.cc
File ui/gfx/image/image.cc (right):

https://codereview.chromium.org/340613004/diff/150001/ui/gfx/image/image.cc#n...
ui/gfx/image/image.cc:77: // gfx::ImageSkia passes one of the resource scale
factor, and
On 2014/06/19 21:09:09, pkotwicz wrote:
> How about: "gfx::ImageSkia passes one of the resource scale factors. The
source
> should return:
> 1) The ImageSkiaRep with the highest scale if all of the available scales ..."
> 
> Aside: It would be nice if we can use either 'scale factor' or 'scale' in the
> function comments.

Done.

https://codereview.chromium.org/340613004/diff/150001/ui/gfx/image/image.cc#n...
ui/gfx/image/image.cc:77: // gfx::ImageSkia passes one of the resource scale
factor, and
On 2014/06/19 21:09:09, pkotwicz wrote:
> How about: "gfx::ImageSkia passes one of the resource scale factors. The
source
> should return:
> 1) The ImageSkiaRep with the highest scale if all of the available scales ..."
> 
> Aside: It would be nice if we can use either 'scale factor' or 'scale' in the
> function comments.

I'm planning to do some cleanup there.

https://codereview.chromium.org/340613004/diff/150001/ui/gfx/image/image.cc#n...
ui/gfx/image/image.cc:98: const gfx::ImageSkiaRep rep = ToImageSkiaRep(png_rep);
On 2014/06/19 21:09:09, pkotwicz wrote:
> Nit: the const here is unnecessary

I think this is fine. It guarantees that rep won't change in between. I
understand it's not necessary, but it's not harmful either.

[pkotwicz, 2014-06-19 21:41:46]

Ok, LGTM

https://codereview.chromium.org/340613004/diff/150001/ui/gfx/image/image.cc
File ui/gfx/image/image.cc (right):

https://codereview.chromium.org/340613004/diff/150001/ui/gfx/image/image.cc#n...
ui/gfx/image/image.cc:98: const gfx::ImageSkiaRep rep = ToImageSkiaRep(png_rep);
We insert |rep| by copy into |image_skia_reps_| so the const does not really
help much.

[oshima, 2014-06-19 22:08:37]

rsesek@ -> owners

[Robert Sesek, 2014-06-19 22:12:05]

https://codereview.chromium.org/340613004/diff/170001/ui/gfx/image/image.cc
File ui/gfx/image/image.cc (left):

https://codereview.chromium.org/340613004/diff/170001/ui/gfx/image/image.cc#o...
ui/gfx/image/image.cc:38: const std::vector<gfx::ImagePNGRep>& image_png_reps);
These seem like unrelated changes. Is it necessary to include in this CL?

https://codereview.chromium.org/340613004/diff/170001/ui/gfx/image/image.cc
File ui/gfx/image/image.cc (right):

https://codereview.chromium.org/340613004/diff/170001/ui/gfx/image/image.cc#n...
ui/gfx/image/image.cc:98: bool AddPngData(const ImagePNGRep& png_rep) {
The class is named PNGImageSource yet this is AddPngData. This should be
AddPNGData.

[oshima, 2014-06-19 23:03:01]

https://codereview.chromium.org/340613004/diff/170001/ui/gfx/image/image.cc
File ui/gfx/image/image.cc (left):

https://codereview.chromium.org/340613004/diff/170001/ui/gfx/image/image.cc#o...
ui/gfx/image/image.cc:38: const std::vector<gfx::ImagePNGRep>& image_png_reps);
On 2014/06/19 22:12:05, rsesek wrote:
> These seem like unrelated changes. Is it necessary to include in this CL?

I removed unnecessary namespace just because I noticed them. I'll remove these
change if you don't like. let me know.

https://codereview.chromium.org/340613004/diff/170001/ui/gfx/image/image.cc
File ui/gfx/image/image.cc (right):

https://codereview.chromium.org/340613004/diff/170001/ui/gfx/image/image.cc#n...
ui/gfx/image/image.cc:98: bool AddPngData(const ImagePNGRep& png_rep) {
On 2014/06/19 22:12:05, rsesek wrote:
> The class is named PNGImageSource yet this is AddPngData. This should be
> AddPNGData.

Done.

[Robert Sesek, 2014-06-19 23:04:33]

LGTM

https://codereview.chromium.org/340613004/diff/170001/ui/gfx/image/image.cc
File ui/gfx/image/image.cc (left):

https://codereview.chromium.org/340613004/diff/170001/ui/gfx/image/image.cc#o...
ui/gfx/image/image.cc:38: const std::vector<gfx::ImagePNGRep>& image_png_reps);
On 2014/06/19 23:03:01, oshima wrote:
> On 2014/06/19 22:12:05, rsesek wrote:
> > These seem like unrelated changes. Is it necessary to include in this CL?
> 
> I removed unnecessary namespace just because I noticed them. I'll remove these
> change if you don't like. let me know.

If it's easy to undo, I think I'd prefer to keep the change logically separate.
If not, I don't feel strongly.

[oshima, 2014-06-19 23:46:38]

https://codereview.chromium.org/340613004/diff/170001/ui/gfx/image/image.cc
File ui/gfx/image/image.cc (left):

https://codereview.chromium.org/340613004/diff/170001/ui/gfx/image/image.cc#o...
ui/gfx/image/image.cc:38: const std::vector<gfx::ImagePNGRep>& image_png_reps);
On 2014/06/19 23:04:33, rsesek wrote:
> On 2014/06/19 23:03:01, oshima wrote:
> > On 2014/06/19 22:12:05, rsesek wrote:
> > > These seem like unrelated changes. Is it necessary to include in this CL?
> > 
> > I removed unnecessary namespace just because I noticed them. I'll remove
these
> > change if you don't like. let me know.
> 
> If it's easy to undo, I think I'd prefer to keep the change logically
separate.
> If not, I don't feel strongly.

The original code was inconsistent, so it's not simple replace. sorry.
At least, this is btter.

[oshima, 2014-06-19 23:46:45]

The CQ bit was checked by oshima@chromium.org

[Robert Sesek, 2014-06-19 23:48:56]

https://codereview.chromium.org/340613004/diff/170001/ui/gfx/image/image.cc
File ui/gfx/image/image.cc (left):

https://codereview.chromium.org/340613004/diff/170001/ui/gfx/image/image.cc#o...
ui/gfx/image/image.cc:38: const std::vector<gfx::ImagePNGRep>& image_png_reps);
On 2014/06/19 23:46:38, oshima wrote:
> On 2014/06/19 23:04:33, rsesek wrote:
> > On 2014/06/19 23:03:01, oshima wrote:
> > > On 2014/06/19 22:12:05, rsesek wrote:
> > > > These seem like unrelated changes. Is it necessary to include in this
CL?
> > > 
> > > I removed unnecessary namespace just because I noticed them. I'll remove
> these
> > > change if you don't like. let me know.
> > 
> > If it's easy to undo, I think I'd prefer to keep the change logically
> separate.
> > If not, I don't feel strongly.
> 
> The original code was inconsistent, so it's not simple replace. sorry.
> At least, this is btter.

I thought you'd maybe have done it in a separate local git commit.

[commit-bot: I haz the power, 2014-06-19 23:50:43]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/oshima@chromium.org/340613004/190001

[oshima, 2014-06-19 23:58:20]

On 2014/06/19 23:48:56, rsesek wrote:
> https://codereview.chromium.org/340613004/diff/170001/ui/gfx/image/image.cc
> File ui/gfx/image/image.cc (left):
> 
>
https://codereview.chromium.org/340613004/diff/170001/ui/gfx/image/image.cc#o...
> ui/gfx/image/image.cc:38: const std::vector<gfx::ImagePNGRep>&
image_png_reps);
> On 2014/06/19 23:46:38, oshima wrote:
> > On 2014/06/19 23:04:33, rsesek wrote:
> > > On 2014/06/19 23:03:01, oshima wrote:
> > > > On 2014/06/19 22:12:05, rsesek wrote:
> > > > > These seem like unrelated changes. Is it necessary to include in this
> CL?
> > > > 
> > > > I removed unnecessary namespace just because I noticed them. I'll remove
> > these
> > > > change if you don't like. let me know.
> > > 
> > > If it's easy to undo, I think I'd prefer to keep the change logically
> > separate.
> > > If not, I don't feel strongly.
> > 
> > The original code was inconsistent, so it's not simple replace. sorry.
> > At least, this is btter.
> 
> I thought you'd maybe have done it in a separate local git commit.

I still can make it separate CLs manually. Please let me know if you want me to
do it.


I still can do manual

[oshima, 2014-06-20 04:35:51]

Committed patchset #6 manually as r278589 (presubmit successful).

[oshima, 2014-06-20 16:57:24]

The CQ bit was checked by oshima@chromium.org

[commit-bot: I haz the power, 2014-06-20 17:00:47]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/oshima@chromium.org/340613004/230001

[oshima, 2014-06-20 18:54:24]

Committed patchset #7 manually as r278783 (presubmit successful).