Add key modifier on minimize/restore button click to minimize/restore all Panels for Mac.

Add key modifier on minimize/restore button click to minimize/restore all Panels for Mac.

Panel.xib changed to remove animation layer for minimize and restore buttons. Bad 'copy-paste' from wrench menu button. Gets rid of kCGError warnings when running interactive_ui_tests.

BUG=123176
TEST=new test added.

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

[jianli, 2012-04-26 18:06:57]

http://codereview.chromium.org/10173035/diff/1/chrome/browser/ui/panels/docke...
File chrome/browser/ui/panels/docked_panel_strip.cc (right):

http://codereview.chromium.org/10173035/diff/1/chrome/browser/ui/panels/docke...
chrome/browser/ui/panels/docked_panel_strip.cc:512: AutoReset<bool>
pin(&minimizing_all_, true);
nit: please add comment for this.

http://codereview.chromium.org/10173035/diff/1/chrome/browser/ui/panels/docke...
chrome/browser/ui/panels/docked_panel_strip.cc:526: if (minimized_active_panel)
{
I think it might be a little bit safer to iterate all the panels again and
deactivate those that are minimized and still active. This is because we might
have the edge case that other panel get activated by the system or the tracking
panel is being closed.

http://codereview.chromium.org/10173035/diff/1/chrome/browser/ui/panels/panel...
File chrome/browser/ui/panels/panel_browsertest.cc (right):

http://codereview.chromium.org/10173035/diff/1/chrome/browser/ui/panels/panel...
chrome/browser/ui/panels/panel_browsertest.cc:636:
EXPECT_TRUE(panel3->IsMinimized());
It would be better to insert one more test case here:
  // Click restore button on a minimized panel. Only that panel will restore.
  ...

In addition, do we also want to verify IsActive() for all minimized panels?

http://codereview.chromium.org/10173035/diff/1/chrome/browser/ui/panels/panel...
File chrome/browser/ui/panels/panel_strip.h (right):

http://codereview.chromium.org/10173035/diff/1/chrome/browser/ui/panels/panel...
chrome/browser/ui/panels/panel_strip.h:97: virtual void MinimizeAll() = 0;
Please add some comment for these 2 methods.

[jennb, 2012-04-27 00:30:10]

http://codereview.chromium.org/10173035/diff/1/chrome/browser/ui/panels/docke...
File chrome/browser/ui/panels/docked_panel_strip.cc (right):

http://codereview.chromium.org/10173035/diff/1/chrome/browser/ui/panels/docke...
chrome/browser/ui/panels/docked_panel_strip.cc:512: AutoReset<bool>
pin(&minimizing_all_, true);
On 2012/04/26 18:06:57, jianli wrote:
> nit: please add comment for this.

Done.

http://codereview.chromium.org/10173035/diff/1/chrome/browser/ui/panels/docke...
chrome/browser/ui/panels/docked_panel_strip.cc:526: if (minimized_active_panel)
{
On 2012/04/26 18:06:57, jianli wrote:
> I think it might be a little bit safer to iterate all the panels again and
> deactivate those that are minimized and still active. This is because we might
> have the edge case that other panel get activated by the system or the
tracking
> panel is being closed.

If another panel is activated by the system while we are minimizing all, that
active panel will not be any of the minimized panels as minimized panels cannot
be activated. The active panel can only be one that has not yet been minimized,
which means we will eventually find it when we get around to minimizing it.
Thus, there is no need to iterate through again.

http://codereview.chromium.org/10173035/diff/1/chrome/browser/ui/panels/panel...
File chrome/browser/ui/panels/panel_browsertest.cc (right):

http://codereview.chromium.org/10173035/diff/1/chrome/browser/ui/panels/panel...
chrome/browser/ui/panels/panel_browsertest.cc:636:
EXPECT_TRUE(panel3->IsMinimized());
On 2012/04/26 18:06:57, jianli wrote:
> It would be better to insert one more test case here:
>   // Click restore button on a minimized panel. Only that panel will restore.

Added.

> 
> In addition, do we also want to verify IsActive() for all minimized panels?

Added checks for minimized panels not being active.

http://codereview.chromium.org/10173035/diff/1/chrome/browser/ui/panels/panel...
File chrome/browser/ui/panels/panel_strip.h (right):

http://codereview.chromium.org/10173035/diff/1/chrome/browser/ui/panels/panel...
chrome/browser/ui/panels/panel_strip.h:97: virtual void MinimizeAll() = 0;
On 2012/04/26 18:06:57, jianli wrote:
> Please add some comment for these 2 methods.

Done.

[jianli, 2012-04-27 00:39:04]

lgtm

http://codereview.chromium.org/10173035/diff/1/chrome/browser/ui/panels/docke...
File chrome/browser/ui/panels/docked_panel_strip.cc (right):

http://codereview.chromium.org/10173035/diff/1/chrome/browser/ui/panels/docke...
chrome/browser/ui/panels/docked_panel_strip.cc:526: if (minimized_active_panel)
{
On 2012/04/27 00:30:11, jennb wrote:
> On 2012/04/26 18:06:57, jianli wrote:
> > I think it might be a little bit safer to iterate all the panels again and
> > deactivate those that are minimized and still active. This is because we
might
> > have the edge case that other panel get activated by the system or the
> tracking
> > panel is being closed.
> 
> If another panel is activated by the system while we are minimizing all, that
> active panel will not be any of the minimized panels as minimized panels
cannot
> be activated. The active panel can only be one that has not yet been
minimized,
> which means we will eventually find it when we get around to minimizing it.
> Thus, there is no need to iterate through again.

What if minimized_active_panel is closed before we call Deactivate for it? This
could happen though it is rare.

[jennb, 2012-04-27 16:47:45]

Removed the IsActive checks from the new test. RunAllPending is not a good way
to wait for "enough time" before checking active status. We can't do a
WaitForPanelActiveState reliably because that would block forever unless we know
for sure which panel was active and would become inactive.

http://codereview.chromium.org/10173035/diff/1/chrome/browser/ui/panels/docke...
File chrome/browser/ui/panels/docked_panel_strip.cc (right):

http://codereview.chromium.org/10173035/diff/1/chrome/browser/ui/panels/docke...
chrome/browser/ui/panels/docked_panel_strip.cc:526: if (minimized_active_panel)
{
On 2012/04/27 00:39:04, jianli wrote:
> On 2012/04/27 00:30:11, jennb wrote:
> > On 2012/04/26 18:06:57, jianli wrote:
> > > I think it might be a little bit safer to iterate all the panels again and
> > > deactivate those that are minimized and still active. This is because we
> might
> > > have the edge case that other panel get activated by the system or the
> > tracking
> > > panel is being closed.
> > 
> > If another panel is activated by the system while we are minimizing all,
that
> > active panel will not be any of the minimized panels as minimized panels
> cannot
> > be activated. The active panel can only be one that has not yet been
> minimized,
> > which means we will eventually find it when we get around to minimizing it.
> > Thus, there is no need to iterate through again.
> 
> What if minimized_active_panel is closed before we call Deactivate for it?
This
> could happen though it is rare.

The closure would happen on a separate pass of the message loop. We are
deactivating the panel in the same method where we minimize it so it all gets
processed in one message loop pass.