[ios] Creates ToolsMenuModel Class

ios/chrome/browser/ui/tools_menu/tools_menu_model.mm

Index: ios/chrome/browser/ui/tools_menu/tools_menu_model.mm
diff --git a/ios/chrome/browser/ui/tools_menu/tools_menu_model.mm b/ios/chrome/browser/ui/tools_menu/tools_menu_model.mm
new file mode 100644
index 0000000000000000000000000000000000000000..9c7c7cf5c63cb11a02e001b81150be8059ccc450
--- /dev/null
+++ b/ios/chrome/browser/ui/tools_menu/tools_menu_model.mm
@@ -0,0 +1,136 @@
+// Copyright 2017 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.
+
+#include "ios/chrome/browser/ui/tools_menu/tools_menu_model.h"
+
+#include "components/reading_list/core/reading_list_switches.h"
+#include "components/strings/grit/components_strings.h"
+#include "ios/chrome/browser/experimental_flags.h"
+#include "ios/chrome/browser/ui/commands/ios_command_ids.h"
+#import "ios/chrome/browser/ui/tools_menu/reading_list_menu_view_item.h"
+#include "ios/chrome/browser/ui/ui_util.h"
+#include "ios/chrome/grit/ios_strings.h"
+#include "ios/public/provider/chrome/browser/chrome_browser_provider.h"
+#import "ios/public/provider/chrome/browser/user_feedback/user_feedback_provider.h"
+
+NSString* const kToolsMenuNewTabId = @"kToolsMenuNewTabId";
+NSString* const kToolsMenuNewIncognitoTabId = @"kToolsMenuNewIncognitoTabId";
+NSString* const kToolsMenuCloseAllTabsId = @"kToolsMenuCloseAllTabsId";
+NSString* const kToolsMenuCloseAllIncognitoTabsId =
+    @"kToolsMenuCloseAllIncognitoTabsId";
+NSString* const kToolsMenuBookmarksId = @"kToolsMenuBookmarksId";
+NSString* const kToolsMenuReadingListId = @"kToolsMenuReadingListId";
+NSString* const kToolsMenuOtherDevicesId = @"kToolsMenuOtherDevicesId";
+NSString* const kToolsMenuHistoryId = @"kToolsMenuHistoryId";
+NSString* const kToolsMenuReportAnIssueId = @"kToolsMenuReportAnIssueId";
+NSString* const kToolsMenuFindInPageId = @"kToolsMenuFindInPageId";
+NSString* const kToolsMenuReaderMode = @"kToolsMenuReaderMode";
+NSString* const kToolsMenuRequestDesktopId = @"kToolsMenuRequestDesktopId";
+NSString* const kToolsMenuSettingsId = @"kToolsMenuSettingsId";
+NSString* const kToolsMenuHelpId = @"kToolsMenuHelpId";
+NSString* const kToolsMenuSuggestionsId = @"kToolsMenuSuggestionsId";
+
+// Menu items can be marked as visible or not when Incognito is enabled.
+// The following bits are used for |visibility| field in |MenuItemInfo|.
+const NSInteger kVisibleIncognitoOnly = 1 << 0;
+const NSInteger kVisibleNotIncognitoOnly = 1 << 1;
+
+// Declare all the possible items.
+const MenuItemInfo itemInfoList[] = {
+    // clang-format off
+  { IDS_IOS_TOOLS_MENU_NEW_TAB,           kToolsMenuNewTabId,
+    IDC_NEW_TAB,                          kToolbarTypeAll,

[rohitrao (ping after 24h), 2017/02/22 12:17:33]

Can you put selectors into structs?  We could expand this to include both IDC
constants and selectors:

{ IDS_IOS_TOOLS_MENU_NEW_INCOGNITO_TAB, kToolsMenuNewIncognitoTabId,
  @selector(newIncognitoTab),
  IDC_NEW_INCOGNITO_TAB,                kToolbarTypeAll,
  0,                                    nil },

If we end up using selectors to pass action messages, this could act as a model
for both the old and new architectures.  Eventually we'd just drop the IDC
constants.

[marq (ping after 24h), 2017/02/22 12:29:07]

Yes, that would work. We would need to think about how this would be shared in
that case, since so far our rationale for shared/ has been that it wouldn't have
chrome/ or clean/ dependencies. This class would use chrome command_ids and
clean action selectors. We could move both of those into shared/of course.

Selectors in the structs can be added in a future CL.

[sczs, 2017/02/22 18:05:12]

That's exactly what I want to try doing in a next CL, I might even sub-class
this from a shared model class so we can support the old VC and still have a
clean model class for the new VC.

+    0,                                    nil },
+  { IDS_IOS_TOOLS_MENU_NEW_INCOGNITO_TAB, kToolsMenuNewIncognitoTabId,
+    IDC_NEW_INCOGNITO_TAB,                kToolbarTypeAll,
+    0,                                    nil },
+  { IDS_IOS_TOOLS_MENU_CLOSE_ALL_TABS,    kToolsMenuCloseAllTabsId,
+    IDC_CLOSE_ALL_TABS,                   kToolbarTypeSwitcheriPhone,
+    kVisibleNotIncognitoOnly,             nil },
+  { IDS_IOS_TOOLS_MENU_CLOSE_ALL_INCOGNITO_TABS,
+    kToolsMenuCloseAllIncognitoTabsId,
+    IDC_CLOSE_ALL_INCOGNITO_TABS,         kToolbarTypeSwitcheriPhone,
+    kVisibleIncognitoOnly,                nil },
+  { IDS_IOS_TOOLS_MENU_BOOKMARKS,         kToolsMenuBookmarksId,
+    IDC_SHOW_BOOKMARK_MANAGER,            kToolbarTypeWebAll,
+    0,                                    nil },
+  { IDS_IOS_TOOLS_MENU_READING_LIST,      kToolsMenuReadingListId,
+    IDC_SHOW_READING_LIST,                kToolbarTypeWebAll,
+    0,                                    [ReadingListMenuViewItem class] },
+  { IDS_IOS_TOOLS_MENU_SUGGESTIONS,       kToolsMenuSuggestionsId,
+    IDC_SHOW_SUGGESTIONS,                 kToolbarTypeWebAll,
+    0,                                    nil },
+  { IDS_IOS_TOOLS_MENU_RECENT_TABS,       kToolsMenuOtherDevicesId,
+    IDC_SHOW_OTHER_DEVICES,               kToolbarTypeWebAll,
+    kVisibleNotIncognitoOnly,             nil },
+  { IDS_HISTORY_SHOW_HISTORY,             kToolsMenuHistoryId,
+    IDC_SHOW_HISTORY,                     kToolbarTypeWebAll,
+    0,                                    nil },
+  { IDS_IOS_OPTIONS_REPORT_AN_ISSUE,      kToolsMenuReportAnIssueId,
+    IDC_REPORT_AN_ISSUE,                  kToolbarTypeAll,
+    0,                                    nil },
+  { IDS_IOS_TOOLS_MENU_FIND_IN_PAGE,      kToolsMenuFindInPageId,
+    IDC_FIND,                             kToolbarTypeWebAll,
+    0,                                    nil },
+  { IDS_IOS_TOOLS_MENU_REQUEST_DESKTOP_SITE, kToolsMenuRequestDesktopId,
+    IDC_REQUEST_DESKTOP_SITE,             kToolbarTypeWebAll,
+    0,                                    nil },
+  { IDS_IOS_TOOLS_MENU_READER_MODE,       kToolsMenuReaderMode,
+    IDC_READER_MODE,                      kToolbarTypeWebAll,
+    0,                                    nil },
+  { IDS_IOS_TOOLS_MENU_SETTINGS,          kToolsMenuSettingsId,
+    IDC_OPTIONS,                          kToolbarTypeAll,
+    0,                                    nil },
+  { IDS_IOS_TOOLS_MENU_HELP_MOBILE,           kToolsMenuHelpId,
+    IDC_HELP_PAGE_VIA_MENU,               kToolbarTypeWebAll,
+    0,                                    nil },
+    // clang-format on
+};
+
+bool ToolsMenuModel::ItemShouldBeVisible(const MenuItemInfo& item,

[edchin, 2017/02/22 07:22:29]

Just out of curiosity, why did you omit NS_INLINE as well as change BOOL to
bool?

[marq (ping after 24h), 2017/02/22 11:15:57]

Either is OK, but if you change to bool you need to return bools (true/false)
and not BOOLs (YES/NO).

[rohitrao (ping after 24h), 2017/02/22 12:17:33]

I have no idea why the NS_INLINE was in here in the first place.
Doesn't matter much, but bool/true/false is probably a better fit for a C++
function.

[sczs, 2017/02/22 18:05:12]

Done.

+                                         bool incognito,
+                                         kToolbarType toolbarType) {
+  if (!(item.toolbar_types & toolbarType))
+    return NO;
+
+  if (incognito && (item.visibility & kVisibleNotIncognitoOnly))
+    return NO;
+
+  if (!incognito && (item.visibility & kVisibleIncognitoOnly))
+    return NO;
+
+  if (item.title_id == IDS_IOS_TOOLBAR_SHOW_TABS) {

[marq (ping after 24h), 2017/02/22 11:15:57]

I realize this is a direct lift from the old code, but from here on down this
can just be a switch:

switch (item.title_id) {
  case IDS_IOS_TOOLBAR_SHOW_TABS: 
    return IsIPadIdiom();
  case IDS_IOS_TOOLS_MENU_READER_MODE:
    return experimental_flags::IsReaderModeEnabled();
  case IDS_IOS_TOOLS_MENU_READING_LIST:                                  
    return reading_list::switches::IsReadingListEnabled();  
  case IDS_IOS_TOOLS_MENU_SUGGESTIONS: 
    return experimental_flags::IsSuggestionsUIEnabled();
  case IDS_IOS_OPTIONS_REPORT_AN_ISSUE: 
    return ios::GetChromeBrowserProvider()->GetUserFeedbackProvider()
        ->IsUserFeedbackEnabled();
  default:
    return YES;
}

Even if you don't do that, you should collapse this pattern:

if (!Condition()) {
  return NO;
}

into 

return Condition();

[sczs, 2017/02/22 18:05:12]

Done.

+    if (!IsIPadIdiom()) {
+      return NO;
+    }
+  }
+
+  if (item.title_id == IDS_IOS_TOOLS_MENU_READER_MODE) {
+    if (!experimental_flags::IsReaderModeEnabled()) {
+      return NO;
+    }
+  }
+
+  if (item.title_id == IDS_IOS_TOOLS_MENU_READING_LIST) {
+    if (!reading_list::switches::IsReadingListEnabled()) {
+      return NO;
+    }
+  }
+
+  if (item.title_id == IDS_IOS_TOOLS_MENU_SUGGESTIONS) {
+    if (!experimental_flags::IsSuggestionsUIEnabled()) {
+      return NO;
+    }
+  }
+
+  if (item.title_id == IDS_IOS_OPTIONS_REPORT_AN_ISSUE) {
+    if (!ios::GetChromeBrowserProvider()
+             ->GetUserFeedbackProvider()
+             ->IsUserFeedbackEnabled()) {
+      return NO;
+    }
+  }
+
+  return YES;
+}