Commands should have consistent behaviors across different modes

chrome/browser/app_controller_mac.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/app_controller_mac.h"
 
 #include <stddef.h>
 
 #include "base/auto_reset.h"
 #include "base/bind.h"
@@ skipping 1479 matching lines @@
 
   // No need to |BuildMenu| here.  It is done lazily upon menu access.
   [bookmarkItem setSubmenu:bookmarkMenuBridge_->BookmarkMenu()];
   [bookmarkItem setHidden:hidden];
 
   historyMenuBridge_.reset(new HistoryMenuBridge(lastProfile_));
   historyMenuBridge_->BuildMenu();
 
   chrome::BrowserCommandController::
       UpdateSharedCommandsForIncognitoAvailability(
-          menuState_.get(), lastProfile_);
+          menuState_.get(), lastProfile_, false);

[msw, 2017/02/14 22:34:50]

How do you know the relevant window is not fullscreen here? (ditto below)
Should UpdateSharedCommandsForIncognitoAvailability be non-static and determine
|is_fullscreen| internally, like UpdateCommandsForIncognitoAvailability? Or can
we get the relevant browser and check that here (via
chrome::FindLastActiveWithProfile, chrome::GetLastActiveBrowser(), or like
menuNeedsUpdate)? You might want to ping a knowledgable cocoa reviewer; I don't
know our Mac code too well.

[Hzj_jie, 2017/02/15 02:00:49]

AFAIK, menuState_ here is the state of menu items. i.e. Even we do not reserve
new-incognitor-window function, we still need this menu item to be available.
I will definitely add the original owner of the logic into the review.

[msw, 2017/02/15 18:40:51]

Sorry, but I don't understand how your reply answers my primary question:
  How do you know the relevant window is not fullscreen here?
This now always disables IDC_NEW_INCOGNITO_WINDOW and IDC_NEW_WINDOW...

[Hzj_jie, 2017/02/15 21:20:16]

No, I do not know, and I do not think we are talking about a window here. When
is_fullscreen is false, the logic is consistent with existing one.

Monica (noms@) wrote this logic 2+ years ago, but she is not working on another
project now. I am still looking for her replacement.

But no matter, since I have reverted the change to
UpdateSharedCommandsForIncognitoAvailability() function, we do not need to
change this file anymore.

   profilePrefRegistrar_.reset(new PrefChangeRegistrar());
   profilePrefRegistrar_->Init(lastProfile_->GetPrefs());
   profilePrefRegistrar_->Add(
       prefs::kIncognitoModeAvailability,
       base::Bind(&chrome::BrowserCommandController::
                      UpdateSharedCommandsForIncognitoAvailability,
                  menuState_.get(),
-                 lastProfile_));
+                 lastProfile_,
+                 false));
 }
 
 - (void)applicationDidChangeScreenParameters:(NSNotification*)notification {
   // During this callback the working area is not always already updated. Defer.
   [self performSelector:@selector(delayedScreenParametersUpdate)
              withObject:nil
              afterDelay:0];
 }
 
 - (void)delayedScreenParametersUpdate {
@@ skipping 88 matching lines @@
 
 //---------------------------------------------------------------------------
 
 namespace app_controller_mac {
 
 bool IsOpeningNewWindow() {
   return g_is_opening_new_window;
 }
 
 }  // namespace app_controller_mac