Fixing activation states from the new launcher. Also adding a whole bunch of unit tests for the new…

Fixing activation states from the new launcher. Also adding a whole bunch of unit tests for the new launcher.


BUG=170470
TEST=unittest


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

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

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

[Mr4D (OOO till 08-26), 2013-01-17 23:03:01]

Hello sky!

I was adding all the unit tests and fixed the edge conditions for the browser
activation state - as well as (thanks to the unit tests) handling the case where
an item (which is not pinned) gets only started once.

Please have a look!

[sky, 2013-01-17 23:59:09]

https://codereview.chromium.org/12022002/diff/2001/chrome/browser/ui/ash/laun...
File chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.cc
(right):

https://codereview.chromium.org/12022002/diff/2001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.cc:1029: //
The browser active if it is not trying to close all tabs.
is active.

Also, is closing_all true if an unload handler dialog or download warning is
shown? Download warning may not be shown for chromeos, but unload handler dialog
could be.

[Mr4D (OOO till 08-26), 2013-01-18 17:46:08]

Added observer - but not to browser but to BrowserList. Also - could not add the
observer to the BrowserLauncherItemController since that will go away before the
browser is gone. Since there is also no object managing the browseritem it
landed in the LauncherItemControllerPerApp class.

Please have a look!

Note: I suppose this is the first step to getting rid of
BrowserLauncherItemController ;).

https://codereview.chromium.org/12022002/diff/2001/chrome/browser/ui/ash/laun...
File chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.cc
(right):

https://codereview.chromium.org/12022002/diff/2001/chrome/browser/ui/ash/laun...
chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.cc:1029: //
The browser active if it is not trying to close all tabs.
Added now global observer instead. Also changed comment.

[sky, 2013-01-18 18:12:09]

LGTM

[commit-bot: I haz the power, 2013-01-18 19:01:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/skuhne@chromium.org/12022002/13002

[commit-bot: I haz the power, 2013-01-18 19:01:14]

Failed to apply patch for
chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.h:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file
chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.h
  Hunk #1 FAILED at 25.
  Hunk #2 FAILED at 62.
  Hunk #3 succeeded at 295 (offset 7 lines).
  Hunk #4 succeeded at 317 (offset 7 lines).
  2 out of 4 hunks FAILED -- saving rejects to file
chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.h.rej

Patch:       chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.h
Index: chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.h
diff --git a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.h
b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.h
index
2b7830b0c7f3f905bb9355372693e76fde38a87b..dc55c3bf8abcdb939bf6cda3dbc7f9eb2868c09e
100644
--- a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.h
+++ b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_per_app.h
@@ -25,6 +25,7 @@
 #include "chrome/browser/ui/ash/launcher/chrome_launcher_app_menu_item.h"
 #include "chrome/browser/ui/ash/launcher/chrome_launcher_controller.h"
 #include "chrome/browser/ui/ash/launcher/shell_window_launcher_controller.h"
+#include "chrome/browser/ui/browser_list_observer.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 #include "ui/aura/window_observer.h"
@@ -62,7 +63,8 @@ class ChromeLauncherControllerPerApp : public
ash::LauncherModelObserver,
                                        public ChromeLauncherController,
                                        public content::NotificationObserver,
                                        public PrefServiceSyncableObserver,
-                                       public AppSyncUIStateObserver {
+                                       public AppSyncUIStateObserver,
+                                       public chrome::BrowserListObserver {
  public:
   ChromeLauncherControllerPerApp(Profile* profile, ash::LauncherModel* model);
   virtual ~ChromeLauncherControllerPerApp();
@@ -288,6 +290,9 @@ class ChromeLauncherControllerPerApp : public
ash::LauncherModelObserver,
   // Note that for incognito windows the incognito icon will be returned.
   gfx::Image GetAppListIcon(content::WebContents* web_contents) const;
 
+  // Overridden from chrome::BrowserListObserver.
+  virtual void OnBrowserRemoved(Browser* browser) OVERRIDE;
+
  protected:
   // ChromeLauncherController overrides:
 
@@ -307,6 +312,9 @@ class ChromeLauncherControllerPerApp : public
ash::LauncherModelObserver,
  private:
   friend class ChromeLauncherControllerPerAppTest;
 
+  // Updates the activation state of the Broswer item.
+  void UpdateBrowserItemStatus();
+
   typedef std::map<ash::LauncherID, LauncherItemController*>
           IDToItemControllerMap;
   typedef std::list<content::WebContents*> WebContentsList;

[commit-bot: I haz the power, 2013-01-18 23:00:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/skuhne@chromium.org/12022002/15003

[commit-bot: I haz the power, 2013-01-18 23:32:10]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_aura.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-01-19 00:50:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/skuhne@chromium.org/12022002/11010

[commit-bot: I haz the power, 2013-01-19 01:10:00]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_aura.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-01-19 01:47:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/skuhne@chromium.org/12022002/11010

[commit-bot: I haz the power, 2013-01-19 02:00:11]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_aura.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-01-19 03:04:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/skuhne@chromium.org/12022002/11010

[commit-bot: I haz the power, 2013-01-19 03:18:17]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_aura.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-01-19 03:57:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/skuhne@chromium.org/12022002/11010

[commit-bot: I haz the power, 2013-01-19 04:47:33]

Change committed as 177840

[commit-bot: I haz the power, 2013-01-24 16:24:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/skuhne@chromium.org/12022002/20001

[commit-bot: I haz the power, 2013-01-24 17:06:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/skuhne@chromium.org/12022002/30001

[commit-bot: I haz the power, 2013-01-24 17:45:27]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_chromeos.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-01-24 18:11:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/skuhne@chromium.org/12022002/24005

[commit-bot: I haz the power, 2013-01-24 20:09:02]

Change committed as 178630

[commit-bot: I haz the power, 2013-01-24 23:12:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/skuhne@chromium.org/12022002/31012

[commit-bot: I haz the power, 2013-01-25 02:02:23]

Change committed as 178733