[iOS] Add Request Mobile Site cell to tools menu

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

Index: ios/chrome/browser/ui/tools_menu/tools_menu_view_controller.mm
diff --git a/ios/chrome/browser/ui/tools_menu/tools_menu_view_controller.mm b/ios/chrome/browser/ui/tools_menu/tools_menu_view_controller.mm
index e6157e493bbd1ddc227f61e6b8c924c80f2fa7a0..219af81ec0381c7b96e456234b05e145018385b1 100644
--- a/ios/chrome/browser/ui/tools_menu/tools_menu_view_controller.mm
+++ b/ios/chrome/browser/ui/tools_menu/tools_menu_view_controller.mm
@@ -21,7 +21,7 @@
 #import "ios/chrome/browser/ui/reading_list/reading_list_menu_notification_delegate.h"
 #import "ios/chrome/browser/ui/reading_list/reading_list_menu_notifier.h"
 #import "ios/chrome/browser/ui/tools_menu/reading_list_menu_view_item.h"
-#import "ios/chrome/browser/ui/tools_menu/tools_menu_context.h"
+#import "ios/chrome/browser/ui/tools_menu/tools_menu_configuration.h"
 #import "ios/chrome/browser/ui/tools_menu/tools_menu_view_item.h"
 #import "ios/chrome/browser/ui/tools_menu/tools_menu_view_tools_cell.h"
 #import "ios/chrome/browser/ui/tools_menu/tools_popup_controller.h"
@@ -32,6 +32,7 @@
 #include "ios/public/provider/chrome/browser/chrome_browser_provider.h"
 #import "ios/public/provider/chrome/browser/user_feedback/user_feedback_provider.h"
 #import "ios/third_party/material_components_ios/src/components/Ink/src/MaterialInk.h"
+#include "ios/web/public/user_agent.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/l10n/l10n_util_mac.h"
 
@@ -50,6 +51,7 @@
 NSString* const kToolsMenuFindInPageId = @"kToolsMenuFindInPageId";
 NSString* const kToolsMenuReaderMode = @"kToolsMenuReaderMode";
 NSString* const kToolsMenuRequestDesktopId = @"kToolsMenuRequestDesktopId";
+NSString* const kToolsMenuRequestMobileId = @"kToolsMenuRequestMobileId";
 NSString* const kToolsMenuSettingsId = @"kToolsMenuSettingsId";
 NSString* const kToolsMenuHelpId = @"kToolsMenuHelpId";
 NSString* const kToolsMenuSuggestionsId = @"kToolsMenuSuggestionsId";
@@ -98,7 +100,7 @@ typedef NS_OPTIONS(NSUInteger, kToolbarType) {
   // clang-format on
 };
 
-// Declare all the possible items.
+// Declares all the possible items.
 static MenuItemInfo itemInfoList[] = {
     // clang-format off
   { IDS_IOS_TOOLS_MENU_NEW_TAB,           kToolsMenuNewTabId,
@@ -138,6 +140,9 @@ typedef NS_OPTIONS(NSUInteger, kToolbarType) {
   { IDS_IOS_TOOLS_MENU_REQUEST_DESKTOP_SITE, kToolsMenuRequestDesktopId,
     IDC_REQUEST_DESKTOP_SITE,             kToolbarTypeWebAll,
     0,                                    nil },
+  { IDS_IOS_TOOLS_MENU_REQUEST_MOBILE_SITE, kToolsMenuRequestMobileId,
+    IDC_REQUEST_MOBILE_SITE,             kToolbarTypeWebAll,
+    0,                                    nil },
   { IDS_IOS_TOOLS_MENU_READER_MODE,       kToolsMenuReaderMode,
     IDC_READER_MODE,                      kToolbarTypeWebAll,
     0,                                    nil },
@@ -151,15 +156,15 @@ typedef NS_OPTIONS(NSUInteger, kToolbarType) {
 };
 
 NS_INLINE BOOL ItemShouldBeVisible(const MenuItemInfo& item,
-                                   BOOL incognito,
-                                   kToolbarType toolbarType) {
+                                   kToolbarType toolbarType,
+                                   ToolsMenuConfiguration* configuration) {
   if (!(item.toolbar_types & toolbarType))
     return NO;
 
-  if (incognito && (item.visibility & kVisibleNotIncognitoOnly))
+  if (configuration.inIncognito && (item.visibility & kVisibleNotIncognitoOnly))
     return NO;
 
-  if (!incognito && (item.visibility & kVisibleIncognitoOnly))
+  if (!configuration.inIncognito && (item.visibility & kVisibleIncognitoOnly))
     return NO;
 
   if (item.title_id == IDS_IOS_TOOLBAR_SHOW_TABS) {
@@ -194,6 +199,21 @@ NS_INLINE BOOL ItemShouldBeVisible(const MenuItemInfo& item,
     }
   }
 
+  // TODO(crbug.com/696676): Talk to UI/UX people to decide the correct behavior
+  // of "Requestion Desktop/Mobile Site" (e.g. Whether user agent flag should
+  // stick when going backward and which cell should be visible when navigating
+  // to native pages).
+  if (item.title_id == IDS_IOS_TOOLS_MENU_REQUEST_DESKTOP_SITE) {
+    if (configuration.userAgentType == web::UserAgentType::DESKTOP)
+      return NO;
+  }
+
+  if (item.title_id == IDS_IOS_TOOLS_MENU_REQUEST_MOBILE_SITE) {
+    if (configuration.userAgentType == web::UserAgentType::NONE ||

[Eugene But (OOO till 7-30), 2017/03/03 02:30:25]

Maybe |configuration.userAgentType ! web::UserAgentType::DESKTOP|?

[liaoyuke, 2017/03/03 23:39:42]

Done.

+        configuration.userAgentType == web::UserAgentType::MOBILE)
+      return NO;
+  }
+
   return YES;
 }
 
@@ -264,7 +284,7 @@ @interface ToolsMenuViewController ()<UICollectionViewDelegateFlowLayout,
 @property(nonatomic, retain) NSMutableArray* menuItems;
 @property(nonatomic, assign) kToolbarType toolbarType;
 
-// Get the reading list cell.
+// Gets the reading list cell.

[Eugene But (OOO till 7-30), 2017/03/03 02:30:25]

nit: s/Gets/Returns since you touching this :)

[liaoyuke, 2017/03/03 23:39:42]

Done.

 - (ReadingListMenuViewCell*)readingListCell;
 @end
 
@@ -328,10 +348,6 @@ - (void)setCanUseReaderMode:(BOOL)enabled {
   [self setItemEnabled:enabled withTag:IDC_READER_MODE];
 }
 
-- (void)setCanUseDesktopUserAgent:(BOOL)enabled {
-  [self setItemEnabled:enabled withTag:IDC_REQUEST_DESKTOP_SITE];
-}
-
 - (void)setCanShowFindBar:(BOOL)enabled {
   [self setItemEnabled:enabled withTag:IDC_FIND];
 }
@@ -361,23 +377,23 @@ - (void)setIsTabLoading:(BOOL)isTabLoading {
   [[toolsCell reloadButton] setHidden:isTabLoading];
 }
 
-- (void)initializeMenu:(ToolsMenuContext*)context {
-  if (context.readingListMenuNotifier) {
-    _readingListMenuNotifier.reset(context.readingListMenuNotifier);
-    [context.readingListMenuNotifier setDelegate:self];
+- (void)initializeMenuWithConfiguration:(ToolsMenuConfiguration*)configuration {
+  if (configuration.readingListMenuNotifier) {
+    _readingListMenuNotifier.reset(configuration.readingListMenuNotifier);
+    [configuration.readingListMenuNotifier setDelegate:self];
   }
 
   if (IsIPadIdiom()) {
-    _toolbarType = context.hasNoOpenedTabs
+    _toolbarType = configuration.hasNoOpenedTabs
                        ? kToolbarTypeNoTabsiPad
                        : (!IsCompactTablet() ? kToolbarTypeWebiPad
                                              : kToolbarTypeWebiPhone);
   } else {
     // kOptionInTabSwitcher option must be enabled on iPhone with
     // no opened tabs.
-    DCHECK(!context.hasNoOpenedTabs || context.isInTabSwitcher);
-    _toolbarType = context.isInTabSwitcher ? kToolbarTypeSwitcheriPhone
-                                           : kToolbarTypeWebiPhone;
+    DCHECK(!configuration.hasNoOpenedTabs || configuration.isInTabSwitcher);
+    _toolbarType = configuration.isInTabSwitcher ? kToolbarTypeSwitcheriPhone
+                                                 : kToolbarTypeWebiPhone;
   }
 
   // Build the menu, adding all relevant items.
@@ -385,7 +401,7 @@ - (void)initializeMenu:(ToolsMenuContext*)context {
 
   for (size_t i = 0; i < arraysize(itemInfoList); ++i) {
     const MenuItemInfo& item = itemInfoList[i];
-    if (!ItemShouldBeVisible(item, context.isInIncognito, _toolbarType))
+    if (!ItemShouldBeVisible(item, _toolbarType, configuration))
       continue;
 
     NSString* title = l10n_util::GetNSStringWithFixup(item.title_id);
@@ -411,9 +427,19 @@ - (void)initializeMenu:(ToolsMenuContext*)context {
 
   [self setMenuItems:menu];
 
+  // Decide the enabled state of the currently visible item between
+  // "Request Desktop Site" and "Request Mobile Site".
+  if (configuration.userAgentType == web::UserAgentType::NONE)

[Eugene But (OOO till 7-30), 2017/03/03 02:30:25]

Should this be a switch case?

[liaoyuke, 2017/03/03 23:39:43]

Done.

+    [self setItemEnabled:NO withTag:IDC_REQUEST_DESKTOP_SITE];

[kkhorimoto, 2017/03/03 18:42:43]

Can we add a TODO referencing your bug to decide what the correct behavior
should be for NONE?  I'm not sure whether it should always be a disabled desktop
option, or if it should be the disabled version of whichever option would be
shown for the previous non-native NavigationItem.

[liaoyuke, 2017/03/03 23:39:42]

I have a TODO at the 202th line in this file, and have send a document to
mardini@
 and UI/UX folks to ask their opinions ASAP.

+  else if (configuration.userAgentType == web::UserAgentType::MOBILE)
+    [self setItemEnabled:YES withTag:IDC_REQUEST_DESKTOP_SITE];
+  else
+    [self setItemEnabled:YES withTag:IDC_REQUEST_MOBILE_SITE];
+
   // Disable IDC_CLOSE_ALL_TABS menu item if on phone with no tabs.
   if (!IsIPadIdiom()) {
-    [self setItemEnabled:!context.hasNoOpenedTabs withTag:IDC_CLOSE_ALL_TABS];
+    [self setItemEnabled:!configuration.hasNoOpenedTabs
+                 withTag:IDC_CLOSE_ALL_TABS];
   }
 }