Keep URL in omnibox when navigating to chrome://bookmarks on iPhone

This is to fix the bug that when navigating to chrome://bookmarks, the
URL in omnibox was mistakenly cleared if chrome://bookmarks is disabled
(non-iPad or new bookmarks flag is ON).

Bug: 777510
Change-Id: I4d9dfd790c155e8351db9d9049b053c740d9f318
Reviewed-on: https://chromium-review.googlesource.com/737290
Commit-Queue: Marti Wong <martiw@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Gauthier Ambard <gambard@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512734}

ios/chrome/browser/ui/BUILD.gn

@@ -317,6 +317,7 @@ source_set("ui_internal") {
     "//ios/chrome/browser/ui/keyboard",
     "//ios/chrome/browser/ui/main:feature_flags",
     "//ios/chrome/browser/ui/ntp",
+    "//ios/chrome/browser/ui/ntp:modal_ntp",
     "//ios/chrome/browser/ui/ntp:ntp_controller",
     "//ios/chrome/browser/ui/ntp:ntp_internal",
     "//ios/chrome/browser/ui/ntp/recent_tabs",

ios/chrome/browser/ui/browser_view_controller.mm

@@ -3395,7 +3395,7 @@ bubblePresenterForFeature:(const base::Feature&)feature
 }
 
 - (BOOL)hasControllerForURL:(const GURL&)url {
-  std::string host(url.host());
+  base::StringPiece host = url.host_piece();
   if (host == kChromeUIOfflineHost) {
     // Only allow offline URL that are fully specified.
     return reading_list::IsOfflineURLValid(
@@ -3403,8 +3403,7 @@ bubblePresenterForFeature:(const base::Feature&)feature
   }
 
   if (host == kChromeUIBookmarksHost) {
-    // Only allow bookmark URL on iPad when Bookmarks is not shown modally.
-    return IsIPadIdiom() && !PresentNTPPanelModally();
+    return IsBookmarksHostEnabled();
   }
 
   return host == kChromeUINewTabHost;
@@ -3415,10 +3414,9 @@ bubblePresenterForFeature:(const base::Feature&)feature
   DCHECK(url.SchemeIs(kChromeUIScheme));
 
   id<CRWNativeContent> nativeController = nil;
-  std::string url_host = url.host();
+  base::StringPiece url_host = url.host_piece();
   if (url_host == kChromeUINewTabHost ||
-      (IsIPadIdiom() && url_host == kChromeUIBookmarksHost &&
-       !PresentNTPPanelModally())) {
+      (url_host == kChromeUIBookmarksHost && IsBookmarksHostEnabled())) {
     CGFloat fakeStatusBarHeight = _fakeStatusBarView.frame.size.height;
     UIEdgeInsets safeAreaInset = UIEdgeInsetsZero;
     if (@available(iOS 11.0, *)) {
@@ -3443,7 +3441,7 @@ bubblePresenterForFeature:(const base::Feature&)feature
     // Panel is always NTP for iPhone.
     ntp_home::PanelIdentifier panelType = ntp_home::HOME_PANEL;
 
-    if (IsIPadIdiom() && !PresentNTPPanelModally()) {
+    if (IsBookmarksHostEnabled()) {
       // New Tab Page can have multiple panels. Each panel is addressable
       // by a #fragment, e.g. chrome://newtab/#most_visited takes user to
       // the Most Visited page, chrome://newtab/#bookmarks takes user to

ios/chrome/browser/ui/ntp/BUILD.gn

@@ -49,6 +49,7 @@ source_set("ntp_controller") {
     "new_tab_page_controller.mm",
   ]
   deps = [
+    ":modal_ntp",
     ":ntp",
     ":ntp_internal",
     "//base",
@@ -91,8 +92,6 @@ source_set("ntp_internal") {
     "incognito_view_controller.mm",
     "metrics.h",
     "metrics.mm",
-    "modal_ntp.h",
-    "modal_ntp.mm",
     "most_visited_cell.h",
     "most_visited_cell.mm",
     "most_visited_layout.h",
@@ -117,6 +116,7 @@ source_set("ntp_internal") {
     ":ntp_header",
   ]
   deps = [
+    ":modal_ntp",
     ":ntp",
     ":ntp_tile",
     "resources:bookmarks_bar_bg",
@@ -224,6 +224,20 @@ source_set("ntp_tile") {
   ]
 }
 
+# TODO(crbug.com/768817): Remove the following once new bookmarks are enabled by
+# default.
+source_set("modal_ntp") {
+  configs += [ "//build/config/compiler:enable_arc" ]
+  sources = [
+    "modal_ntp.h",
+    "modal_ntp.mm",
+  ]
+  deps = [
+    "//ios/chrome/browser/bookmarks:features",
+    "//ios/chrome/browser/ui:ui_util",
+  ]
+}
+
 source_set("unit_tests") {
   configs += [ "//build/config/compiler:enable_arc" ]
   testonly = true
@@ -237,6 +251,7 @@ source_set("unit_tests") {
     "ntp_tile_saver_unittest.mm",
   ]
   deps = [
+    ":modal_ntp",
     ":ntp",
     ":ntp_controller",
     ":ntp_internal",

ios/chrome/browser/ui/ntp/modal_ntp.h

@@ -11,4 +11,8 @@
 // modally.
 BOOL PresentNTPPanelModally();
 
+// Whether chrome://bookmarks is enabled.  True when panel on the NTP isn't
+// presented modally.
+BOOL IsBookmarksHostEnabled();
+
 #endif  // IOS_CHROME_BROWSER_UI_NTP_MODAL_NTP_H_

ios/chrome/browser/ui/ntp/modal_ntp.mm

@@ -14,3 +14,7 @@
 BOOL PresentNTPPanelModally() {
   return !IsIPadIdiom() || base::FeatureList::IsEnabled(kBookmarkNewGeneration);
 }
+
+BOOL IsBookmarksHostEnabled() {
+  return !PresentNTPPanelModally();
+}

ios/chrome/browser/ui/toolbar/BUILD.gn

@@ -78,6 +78,7 @@ source_set("toolbar") {
     "//ios/chrome/browser/ui/history_popup/requirements",
     "//ios/chrome/browser/ui/keyboard",
     "//ios/chrome/browser/ui/ntp",
+    "//ios/chrome/browser/ui/ntp:modal_ntp",
     "//ios/chrome/browser/ui/omnibox",
     "//ios/chrome/browser/ui/popup_menu",
     "//ios/chrome/browser/ui/qr_scanner/requirements",

ios/chrome/browser/ui/toolbar/toolbar_model_delegate_ios.mm

@@ -14,6 +14,7 @@
 #include "ios/chrome/browser/chrome_url_constants.h"
 #include "ios/chrome/browser/pref_names.h"
 #include "ios/chrome/browser/ssl/ios_security_state_tab_helper.h"
+#import "ios/chrome/browser/ui/ntp/modal_ntp.h"
 #include "ios/chrome/browser/web_state_list/web_state_list.h"
 #import "ios/web/public/navigation_item.h"
 #import "ios/web/public/navigation_manager.h"
@@ -65,8 +66,10 @@ bool ToolbarModelDelegateIOS::ShouldDisplayURL() const {
         virtual_url.SchemeIs(kChromeUIScheme)) {
       if (!url.SchemeIs(kChromeUIScheme))
         url = virtual_url;
-      const std::string host = url.host();
-      return host != kChromeUIBookmarksHost && host != kChromeUINewTabHost;
+      base::StringPiece host = url.host_piece();
+      // Checking for Bookmarks Host is skipped if it is not enabled.
+      return (!IsBookmarksHostEnabled() || host != kChromeUIBookmarksHost) &&
+             host != kChromeUINewTabHost;
     }
   }
   return true;

ios/clean/chrome/browser/ui/ntp/BUILD.gn

@@ -48,6 +48,7 @@ source_set("ntp") {
     "//ios/chrome/browser/ui/coordinators",
     "//ios/chrome/browser/ui/metrics",
     "//ios/chrome/browser/ui/ntp",
+    "//ios/chrome/browser/ui/ntp:modal_ntp",
     "//ios/chrome/browser/ui/ntp:ntp_controller",
     "//ios/chrome/browser/ui/ntp:ntp_internal",
     "//ios/chrome/browser/ui/toolbar",
@@ -84,6 +85,7 @@ source_set("ntp_ui") {
     "//ios/chrome/browser/ui/content_suggestions:content_suggestions_constant",
     "//ios/chrome/browser/ui/content_suggestions:content_suggestions_util",
     "//ios/chrome/browser/ui/ntp",
+    "//ios/chrome/browser/ui/ntp:modal_ntp",
     "//ios/chrome/browser/ui/ntp:ntp_controller",
     "//ios/chrome/browser/ui/ntp:ntp_internal",
     "//ios/chrome/browser/ui/ntp/resources:ntp_google_search_box",