[Mac] Retain location bar focus on switching to fullscreen and back.

chrome/browser/ui/cocoa/browser_window_controller_private.mm

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #import "chrome/browser/ui/cocoa/browser_window_controller_private.h"
 
 #include <cmath>
 
 #import "base/auto_reset.h"
 #include "base/command_line.h"
@@ skipping 944 matching lines @@
     [subviews addObject:[bookmarkBarController_ view]];
   if ([toolbarController_ view])
     [subviews addObject:[toolbarController_ view]];
   if ([infoBarContainerController_ view])
     [subviews addObject:[infoBarContainerController_ view]];
   if ([self tabContentArea])
     [subviews addObject:[self tabContentArea]];
   if ([findBarCocoaController_ view])
     [subviews addObject:[findBarCocoaController_ view]];
 
+  // -setContentViewSubviews: changes the view hierarchy, and if the

[spqchan, 2017/02/08 19:21:50]

"is focused will" -> "is focused, it will

[shrike, 2017/02/08 19:51:42]

I massaged it a bit more to avoid "it will cause it" which is confusing.

+  // location bar is focused will cause it to resign first responder.
+  // Remember the location bar's focus state and restore it after changing the
+  // view hierarchy.
+  BOOL locationBarHadFocus = [self locationBarHasFocus];
   [self setContentViewSubviews:subviews];
+  if (locationBarHadFocus) {

[spqchan, 2017/02/08 19:21:50]

nit: No curly brackets required. Also maybe you can just directly put [self
locationBarHasFocus] in the if statement?

[shrike, 2017/02/08 19:51:42]

I removed the curly braces. Coding if statements without curly braces invites
bugs so I always add them, but I see that it is the style that you use.

[spqchan, 2017/02/09 01:50:34]

Hey, to be fair, the reason I use this style in the first place is because it
got nitpicked by my reviewers  :)
According to rsesek and tapted, we shouldn't use curly braces if the if
statement only contains one line

[shrike, 2017/02/09 02:18:29]

I hear you that your reviewers told you that curly braces were wrong. The style
guide allows either form. In my not-so-humble opinion, coding if statements
without curly braces is not right (and I've seen at least one bug in Chromium
source that resulted from this practice). The style guide says to adopt the
prevailing style of the surrounding code so I go with that, but when I can I
stick with curly braces even though the reviewers say it's a nit (it's not).

[spqchan, 2017/02/09 05:09:36]

Makes sense. Since the codebase will be changed so that the underscore will be
in the front of the variable instead of the end, perhaps this can go with it?

+    [self focusLocationBar:YES];
+  }
 }
 
 - (void)updateSubviewZOrderFullscreen {
   base::scoped_nsobject<NSMutableArray> subviews([[NSMutableArray alloc] init]);
 
   // The infobar should overlay the toolbar if the toolbar is fully shown.
   FullscreenToolbarLayout layout = [fullscreenToolbarController_ computeLayout];
   BOOL shouldInfoBarOverlayToolbar =
       ui::IsCGFloatEqual(layout.toolbarFraction, 1.0);
 
@@ skipping 19 matching lines @@
 
   if ([toolbarController_ view])
     [subviews addObject:[toolbarController_ view]];
 
   if (shouldInfoBarOverlayToolbar && [infoBarContainerController_ view])
     [subviews addObject:[infoBarContainerController_ view]];
 
   if ([findBarCocoaController_ view])
     [subviews addObject:[findBarCocoaController_ view]];
 
+  // -setContentViewSubviews: changes the view hierarchy, and if the
+  // location bar is focused will cause it to resign first responder.
+  // Remember the location bar's focus state and restore it after changing the
+  // view hierarchy.
+  BOOL locationBarHadFocus = [self locationBarHasFocus];
   [self setContentViewSubviews:subviews];
+  if (locationBarHadFocus) {
+    [self focusLocationBar:YES];
+  }
 }
 
 - (void)setContentViewSubviews:(NSArray*)subviews {
   // Subviews already match.
   if ([[self.chromeContentView subviews] isEqual:subviews])
     return;
 
   // The tabContentArea isn't a subview, so just set all the subviews.
   NSView* tabContentArea = [self tabContentArea];
   if (![[self.chromeContentView subviews] containsObject:tabContentArea]) {
@@ skipping 176 matching lines @@
   if (fullscreenLowPowerCoordinator_)
     fullscreenLowPowerCoordinator_->SetHasActiveSheet(false);
 }
 
 - (void)childWindowsDidChange {
   if (fullscreenLowPowerCoordinator_)
     fullscreenLowPowerCoordinator_->ChildWindowsChanged();
 }
 
 @end  // @implementation BrowserWindowController(Private)