Exit tabbed fullscreen mode on navigation.

chrome/browser/ui/browser.cc

Index: chrome/browser/ui/browser.cc
diff --git a/chrome/browser/ui/browser.cc b/chrome/browser/ui/browser.cc
index 89d35dc3b8f1eeb2adc42dd66f81606d5fa2d0fe..efa2b400d84a811d2c3c12c725e00f94e7da20cb 100644
--- a/chrome/browser/ui/browser.cc
+++ b/chrome/browser/ui/browser.cc
@@ -305,6 +305,9 @@ Browser::Browser(Type type, Profile* profile)
   registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_READY_FOR_INSTALL,
                  NotificationService::AllSources());
 
+  registrar_.Add(this, content::NOTIFICATION_NAV_ENTRY_COMMITTED,

[Peter Kasting, 2011/09/23 19:20:06]

Why is this here when you have the more-specific one below?

[koz (OOO until 15th September), 2011/09/26 08:15:21]

Oops, this was left over from previous iterations.
Removed.

+                 NotificationService::AllSources());
+
   PrefService* local_state = g_browser_process->local_state();
   if (local_state) {
     local_pref_registrar_.Init(local_state);
@@ -1702,8 +1705,11 @@ void Browser::ToggleFullscreenMode() {
 }
 
 void Browser::NotifyTabOfFullscreenExitIfNecessary() {
-  if (fullscreened_tab_)
+  if (fullscreened_tab_) {
     fullscreened_tab_->ExitFullscreenMode();
+    registrar_.Remove(this, content::NOTIFICATION_NAV_ENTRY_COMMITTED,
+        Source<NavigationController>(&fullscreened_tab_->controller()));
+  }
   fullscreened_tab_ = NULL;
   tab_caused_fullscreen_ = false;
 }
@@ -3808,8 +3814,16 @@ void Browser::ToggleFullscreenModeForTab(TabContents* tab,
     bool enter_fullscreen) {
   if (tab != GetSelectedTabContents())
     return;
-  fullscreened_tab_ = enter_fullscreen ?
-      TabContentsWrapper::GetCurrentWrapperForContents(tab) : NULL;
+  if (enter_fullscreen) {
+    fullscreened_tab_ =
+      TabContentsWrapper::GetCurrentWrapperForContents(tab);
+    registrar_.Add(this, content::NOTIFICATION_NAV_ENTRY_COMMITTED,
+        Source<NavigationController>(&fullscreened_tab_->controller()));
+  } else {
+    registrar_.Remove(this, content::NOTIFICATION_NAV_ENTRY_COMMITTED,

[Peter Kasting, 2011/09/23 19:20:06]

Is this codepath guaranteed to be reached in all cases (e.g. if a tab is
closed)?  If there are any unmatched cases, you'll slowly build up bad entries
in the registrar, which could have strange consequences.

[koz (OOO until 15th September), 2011/09/26 08:15:21]

I think it is. It is equivalent to exiting tabbed fullscreen mode, so this will
get cleaned up any time that tabbed fullscreen gets exited.

+        Source<NavigationController>(&fullscreened_tab_->controller()));
+    fullscreened_tab_ = NULL;
+  }
   bool in_correct_mode_for_tab_fullscreen;
 #if defined(OS_MACOSX)
   in_correct_mode_for_tab_fullscreen = window_->InPresentationMode();
@@ -4136,6 +4150,10 @@ void Browser::Observe(int type,
       if (profile_->IsSameProfile(Source<Profile>(source).ptr()))
         UpdateBookmarkBarState(BOOKMARK_BAR_STATE_CHANGE_PREF_CHANGE);
       break;
+    case content::NOTIFICATION_NAV_ENTRY_COMMITTED: {
+      ExitTabbedFullscreenModeIfNecessary();

[Peter Kasting, 2011/09/23 19:20:06]

Are you sure this is what you want to do?  It seems like this will force tabs
out of fullscreen when e.g. they have an AUTO_SUBFRAME navigation -- that
doesn't seem right.

[koz (OOO until 15th September), 2011/09/26 08:15:21]

I've added a check for NavigationTypes that should not cause tabbed fullscreen
to exit. Do they seem okay to you?

+      break;
+    }
 
     default:
       NOTREACHED() << "Got a notification we didn't register for.";