Make the visual feedback for taking screenshot, with fix.

Make the visual feedback for taking screenshot, with fix.

This is almost identical with http://codereview.chromium.org/9430041
but contains the fix for the compilation failure on linux-clang
http://build.chromium.org/p/chromium.chromiumos/builders/Linux%20ChromiumOS%20%28Clang%20dbg%29/builds/8613/steps/compile/logs/stdio

BUG=113467
TEST=manually


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=124929

[Daniel Erat, 2012-03-01 17:15:40]

https://chromiumcodereview.appspot.com/9545014/diff/1/chrome/browser/ui/views...
File chrome/browser/ui/views/aura/screenshot_taker.cc (right):

https://chromiumcodereview.appspot.com/9545014/diff/1/chrome/browser/ui/views...
chrome/browser/ui/views/aura/screenshot_taker.cc:116: SkColorSetA(SK_ColorWHITE,
kVisualFeedbackLayerOpacity));
When I was adding Layer::LAYER_SOLID_COLOR, I thought it was the case that
partially-transparent colors didn't work; they showed up as opaque instead. 
Assuming that this is still an issue, it'd be better to use an opaque color here
and call Layer::SetOpacity(0.25).

[Jun Mukai, 2012-03-02 03:41:18]

http://codereview.chromium.org/9545014/diff/1/chrome/browser/ui/views/aura/sc...
File chrome/browser/ui/views/aura/screenshot_taker.cc (right):

http://codereview.chromium.org/9545014/diff/1/chrome/browser/ui/views/aura/sc...
chrome/browser/ui/views/aura/screenshot_taker.cc:116: SkColorSetA(SK_ColorWHITE,
kVisualFeedbackLayerOpacity));
On 2012/03/01 17:15:40, Daniel Erat wrote:
> When I was adding Layer::LAYER_SOLID_COLOR, I thought it was the case that
> partially-transparent colors didn't work; they showed up as opaque instead. 
> Assuming that this is still an issue, it'd be better to use an opaque color
here
> and call Layer::SetOpacity(0.25).

Added SetOpacity.  Just curious, is this a bug?  When it's fixed, then, the
color would be much less opaque?

[Daniel Erat, 2012-03-02 15:58:51]

On 2012/03/02 03:41:18, mukai wrote:
>
http://codereview.chromium.org/9545014/diff/1/chrome/browser/ui/views/aura/sc...
> File chrome/browser/ui/views/aura/screenshot_taker.cc (right):
> 
>
http://codereview.chromium.org/9545014/diff/1/chrome/browser/ui/views/aura/sc...
> chrome/browser/ui/views/aura/screenshot_taker.cc:116:
SkColorSetA(SK_ColorWHITE,
> kVisualFeedbackLayerOpacity));
> On 2012/03/01 17:15:40, Daniel Erat wrote:
> > When I was adding Layer::LAYER_SOLID_COLOR, I thought it was the case that
> > partially-transparent colors didn't work; they showed up as opaque instead. 
> > Assuming that this is still an issue, it'd be better to use an opaque color
> here
> > and call Layer::SetOpacity(0.25).
> 
> Added SetOpacity.  Just curious, is this a bug?  When it's fixed, then, the
> color would be much less opaque?

Yeah, I meant that you should just call
visual_feedback_layer_->SetColor(SK_ColorWHITE) after setting the layer's
opacity instead.  It'd be good to also make kVisualFeedbackLayerOpacity be a
float containing 0.25f instead.

[Jun Mukai, 2012-03-02 23:20:03]

On 2012/03/02 15:58:51, Daniel Erat wrote:
> On 2012/03/02 03:41:18, mukai wrote:
> >
>
http://codereview.chromium.org/9545014/diff/1/chrome/browser/ui/views/aura/sc...
> > File chrome/browser/ui/views/aura/screenshot_taker.cc (right):
> > 
> >
>
http://codereview.chromium.org/9545014/diff/1/chrome/browser/ui/views/aura/sc...
> > chrome/browser/ui/views/aura/screenshot_taker.cc:116:
> SkColorSetA(SK_ColorWHITE,
> > kVisualFeedbackLayerOpacity));
> > On 2012/03/01 17:15:40, Daniel Erat wrote:
> > > When I was adding Layer::LAYER_SOLID_COLOR, I thought it was the case that
> > > partially-transparent colors didn't work; they showed up as opaque
instead. 
> > > Assuming that this is still an issue, it'd be better to use an opaque
color
> > here
> > > and call Layer::SetOpacity(0.25).
> > 
> > Added SetOpacity.  Just curious, is this a bug?  When it's fixed, then, the
> > color would be much less opaque?
> 
> Yeah, I meant that you should just call
> visual_feedback_layer_->SetColor(SK_ColorWHITE) after setting the layer's
> opacity instead.  It'd be good to also make kVisualFeedbackLayerOpacity be a
> float containing 0.25f instead.

Hmm, it does not seem working when I tested it on Goobuntu.  Even when
SetOpacity() is set, the visual_feedback_layer_ is not transparent.  I thought
it is a restriction on LAYER_SOLID_COLOR and I should specify the opacity by the
color alpha value...
So let me make sure:
- this is a bug and will be fixed soon or later
- so we can go ahead with SetOpacity and SK_ColorWHITE even though it's not
transparent, because it anyway makes the visual feedback

# ...or, this only happens with CrOS binary on Linux.  works well on actual
devices?

[Daniel Erat, 2012-03-03 00:03:55]

On 2012/03/02 23:20:03, mukai wrote:
> On 2012/03/02 15:58:51, Daniel Erat wrote:
> > On 2012/03/02 03:41:18, mukai wrote:
> > >
> >
>
http://codereview.chromium.org/9545014/diff/1/chrome/browser/ui/views/aura/sc...
> > > File chrome/browser/ui/views/aura/screenshot_taker.cc (right):
> > > 
> > >
> >
>
http://codereview.chromium.org/9545014/diff/1/chrome/browser/ui/views/aura/sc...
> > > chrome/browser/ui/views/aura/screenshot_taker.cc:116:
> > SkColorSetA(SK_ColorWHITE,
> > > kVisualFeedbackLayerOpacity));
> > > On 2012/03/01 17:15:40, Daniel Erat wrote:
> > > > When I was adding Layer::LAYER_SOLID_COLOR, I thought it was the case
that
> > > > partially-transparent colors didn't work; they showed up as opaque
> instead. 
> > > > Assuming that this is still an issue, it'd be better to use an opaque
> color
> > > here
> > > > and call Layer::SetOpacity(0.25).
> > > 
> > > Added SetOpacity.  Just curious, is this a bug?  When it's fixed, then,
the
> > > color would be much less opaque?
> > 
> > Yeah, I meant that you should just call
> > visual_feedback_layer_->SetColor(SK_ColorWHITE) after setting the layer's
> > opacity instead.  It'd be good to also make kVisualFeedbackLayerOpacity be a
> > float containing 0.25f instead.
> 
> Hmm, it does not seem working when I tested it on Goobuntu.  Even when
> SetOpacity() is set, the visual_feedback_layer_ is not transparent.  I thought
> it is a restriction on LAYER_SOLID_COLOR and I should specify the opacity by
the
> color alpha value...
> So let me make sure:
> - this is a bug and will be fixed soon or later
> - so we can go ahead with SetOpacity and SK_ColorWHITE even though it's not
> transparent, because it anyway makes the visual feedback
> 
> # ...or, this only happens with CrOS binary on Linux.  works well on actual
> devices?

http://codereview.chromium.org/9289036 is the LAYER_SOLID_COLOR changelist; I
described the problem that I saw in the review comments.

But yes, I also see an opaque layer even when using SetOpacity().  Mind just
sticking with that for now?  I've filed http://crbug.com/116631 to track the
problem.  Sorry for the bad advice!

[Jun Mukai, 2012-03-05 01:03:43]

Fixed the code around opacity.  PTAL.

[Daniel Erat, 2012-03-05 04:26:43]

lgtm

[commit-bot: I haz the power, 2012-03-05 07:23:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mukai@chromium.org/9545014/9001

[commit-bot: I haz the power, 2012-03-05 09:05:05]

Change committed as 124929