Ignores screenshot taking invocations if it's too close to the previous one.

Ignores screenshot taking invocations if it's too close to the previous one.


R=sky@chromium.org
BUG=135535
TEST=manually checked the behavior by keep pressing Ctrl-F5 on lumpy


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

[sky, 2012-07-20 16:13:31]

http://codereview.chromium.org/10802046/diff/1/chrome/browser/ui/ash/screensh...
File chrome/browser/ui/ash/screenshot_taker.cc (right):

http://codereview.chromium.org/10802046/diff/1/chrome/browser/ui/ash/screensh...
chrome/browser/ui/ash/screenshot_taker.cc:147: aura::RootWindow* root_window =
window->GetRootWindow();
Do we really care about what root window this came from? Can't we have the time
apply to all?
If not, the map needs to be pruned when rootwindows are removed.

[Jun Mukai, 2012-07-20 17:07:32]

http://codereview.chromium.org/10802046/diff/1/chrome/browser/ui/ash/screensh...
File chrome/browser/ui/ash/screenshot_taker.cc (right):

http://codereview.chromium.org/10802046/diff/1/chrome/browser/ui/ash/screensh...
chrome/browser/ui/ash/screenshot_taker.cc:147: aura::RootWindow* root_window =
window->GetRootWindow();
On 2012/07/20 16:13:31, sky wrote:
> Do we really care about what root window this came from? Can't we have the
time
> apply to all?
> If not, the map needs to be pruned when rootwindows are removed.

This method is invoked several times for a single Ctrl-F5 when multiple displays
are available.  see ash/accelerators/accelerator_controller.cc.  Possibly we can
check the timestamp in the accelerator_controller...

[sky, 2012-07-20 23:23:20]

On Fri, Jul 20, 2012 at 10:07 AM,  <mukai@chromium.org> wrote:
>
>
http://codereview.chromium.org/10802046/diff/1/chrome/browser/ui/ash/screensh...
> File chrome/browser/ui/ash/screenshot_taker.cc (right):
>
>
http://codereview.chromium.org/10802046/diff/1/chrome/browser/ui/ash/screensh...
> chrome/browser/ui/ash/screenshot_taker.cc:147: aura::RootWindow*
> root_window = window->GetRootWindow();
> On 2012/07/20 16:13:31, sky wrote:
>>
>> Do we really care about what root window this came from? Can't we have
>
> the time
>>
>> apply to all?
>> If not, the map needs to be pruned when rootwindows are removed.
>
>
> This method is invoked several times for a single Ctrl-F5 when multiple
> displays are available.  see ash/accelerators/accelerator_controller.cc.
>  Possibly we can check the timestamp in the accelerator_controller...
>
> http://codereview.chromium.org/10802046/

Isn't that the bug? We should make sure if there are multiple displays
we only process the accelerator from one of them.

  -Scott

[Jun Mukai, 2012-07-21 04:43:11]

On 2012/07/20 23:23:20, sky wrote:
> On Fri, Jul 20, 2012 at 10:07 AM,  <mailto:mukai@chromium.org> wrote:
> >
> >
>
http://codereview.chromium.org/10802046/diff/1/chrome/browser/ui/ash/screensh...
> > File chrome/browser/ui/ash/screenshot_taker.cc (right):
> >
> >
>
http://codereview.chromium.org/10802046/diff/1/chrome/browser/ui/ash/screensh...
> > chrome/browser/ui/ash/screenshot_taker.cc:147: aura::RootWindow*
> > root_window = window->GetRootWindow();
> > On 2012/07/20 16:13:31, sky wrote:
> >>
> >> Do we really care about what root window this came from? Can't we have
> >
> > the time
> >>
> >> apply to all?
> >> If not, the map needs to be pruned when rootwindows are removed.
> >
> >
> > This method is invoked several times for a single Ctrl-F5 when multiple
> > displays are available.  see ash/accelerators/accelerator_controller.cc.
> >  Possibly we can check the timestamp in the accelerator_controller...
> >
> > http://codereview.chromium.org/10802046/
> 
> Isn't that the bug? We should make sure if there are multiple displays
> we only process the accelerator from one of them.
> 
>   -Scott

That was discussed in crbug.com/130718#7 and I don't think that's a bug.

[Jun Mukai, 2012-07-23 14:50:39]

Updated the code to use a single timstamp.  Please take another look.

[sky, 2012-07-23 16:19:31]

Before reviewing this I'm waiting for a response from my question in
http://codereview.chromium.org/10786026/ .

On Mon, Jul 23, 2012 at 7:50 AM,  <mukai@chromium.org> wrote:
> Updated the code to use a single timstamp.  Please take another look.
>
> http://codereview.chromium.org/10802046/

[sky, 2012-07-23 18:29:38]

LGTM

[commit-bot: I haz the power, 2012-07-24 06:43:55]

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

[commit-bot: I haz the power, 2012-07-24 08:21:57]

Change committed as 148062