Remove old webui History page on desktop and mobile

chrome/browser/sessions/better_session_restore_browsertest.cc

 // 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.
 
 #include <stddef.h>
 
 #include <string>
 
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/lazy_instance.h"
 #include "base/macros.h"
 #include "base/path_service.h"
 #include "base/strings/utf_string_conversions.h"
-#include "base/test/scoped_feature_list.h"
 #include "build/build_config.h"
 #include "chrome/browser/background/background_mode_manager.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/content_settings/cookie_settings_factory.h"
 #include "chrome/browser/defaults.h"
 #include "chrome/browser/infobars/infobar_service.h"
 #include "chrome/browser/lifetime/keep_alive_types.h"
 #include "chrome/browser/lifetime/scoped_keep_alive.h"
 #include "chrome/browser/prefs/session_startup_pref.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_impl.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/sessions/session_restore_test_helper.h"
 #include "chrome/browser/sessions/session_service_factory.h"
 #include "chrome/browser/sessions/session_service_test_helper.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/startup/startup_browser_creator.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/webui/md_history_ui.h"
-#include "chrome/common/chrome_features.h"
-#include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/content_settings/core/browser/cookie_settings.h"
 #include "components/content_settings/core/common/content_settings.h"
 #include "components/infobars/core/confirm_infobar_delegate.h"
 #include "components/prefs/pref_service.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/url_constants.h"
@@ skipping 442 matching lines @@
 IN_PROC_BROWSER_TEST_F(ContinueWhereILeftOffTest, SessionCookiesBrowserClose) {
   // Set the startup preference to "continue where I left off" and visit a page
   // which stores a session cookie.
   StoreDataWithPage("session_cookies.html");
   Browser* new_browser = QuitBrowserAndRestore(browser(), false);
   // The browsing session will be continued; just wait for the page to reload
   // and check the stored data.
   CheckReloadedPageRestored(new_browser);
 }
 
-// Test that switching MD History on behaves correctly with session restore.

[sky, 2017/04/27 22:41:04]

Is there already a test that ensures the old history urls are mapped to the new?

[Dan Beam, 2017/04/28 03:17:45]

I assume the main entry point to history is Chrome app menu -> History or
Ctrl/Cmd+h.

You probably already know this, but: the URL I'm removing (history-frame) is
kinda of hidden on desktop.  when you navigate to history on desktop, it shows
chrome://history.  behind the covers, it's actually:

  top frame: chrome://chrome/history
    left nav frame: chrome://uber-frame
    right nav frame: chrome://history-frame <-- I'm removing this

so the user never sees the "history-frame" URL unless they type it directly. 
bookmarks would be on "chrome://history".

on ChromeOS, the URL visited in a window is chrome://history-frame but the URL
bar is hidden.

the URL does kinda show on mobile, until you scroll (of course) and again, the
main entry point I'd imagine (due to how annoying and error-prone it is to type)
is Chrome app menu -> History.


On legacy URLs:

In settings, we changed some URLs.  We updated all the places in native UI that
these URLs were used (i.e. native UI opens a tab to
chrome://settings/someSubpage that's now chrome://settings/some/subpage).  We
did not attempt to keep backward compatible URLs forever.  I don't really see a
use to that.  The only things affected are maybe bookmarks or users that type
the URL, and those can both be updated.

As far as I know, no help center articles use chrome:// URLs.  Instead they use
a set of instructions specific to the UI (show the X, scroll to the Y, click the
Z), which also needs to be updated.

I think chrome:// URLs are weird.  They're internal.  We don't let people click
them by default, and they're mainly just echoing internal state to the user.  If
we could hide them, I'd be happier.


tl;dr - i don't think it's worth continuing to hold on to URLs forever for back
compatibility.  do you disagree?  maybe hold on them longer?

[sky, 2017/04/28 17:43:54]

I was specifically thinking of an old URL entering the system via session
restore, or history. If you say that isn't possible, or doesn't cause problems.
Ok.

[Dan Beam, 2017/04/29 02:39:43]

it's possible in some cases, so i restored some legacy redirection for now

-IN_PROC_BROWSER_TEST_F(ContinueWhereILeftOffTest, MDHistoryUpgrade) {
-  MdHistoryUI::use_test_title_ = true;
-  Browser* current_browser = browser();
-  {
-    base::test::ScopedFeatureList feature_list;
-    feature_list.InitAndDisableFeature(features::kMaterialDesignHistory);
-    content::WebContents* web_contents =
-        current_browser->tab_strip_model()->GetActiveWebContents();
-    content::TitleWatcher title_watcher(web_contents,
-                                        base::ASCIIToUTF16("History"));
-    ui_test_utils::NavigateToURL(current_browser, GURL("chrome://history"));
-    base::string16 final_title = title_watcher.WaitAndGetTitle();
-    EXPECT_EQ(3u, current_browser->tab_strip_model()
-                     ->GetActiveWebContents()
-                     ->GetAllFrames()
-                     .size());
-  }
-  {
-    base::test::ScopedFeatureList feature_list;
-    feature_list.InitAndEnableFeature(features::kMaterialDesignHistory);
-    current_browser = QuitBrowserAndRestore(browser(), false);
-    // The new history page should have loaded.
-    CheckTitle(current_browser, base::ASCIIToUTF16("MD History"));
-    EXPECT_EQ(1u, current_browser->tab_strip_model()
-                     ->GetActiveWebContents()
-                     ->GetAllFrames()
-                     .size());
-  }
-  {
-    base::test::ScopedFeatureList feature_list;
-    feature_list.InitAndDisableFeature(features::kMaterialDesignHistory);
-    current_browser = QuitBrowserAndRestore(current_browser, false);
-    // The old history page should have loaded.
-    CheckTitle(current_browser, base::ASCIIToUTF16("History"));
-    EXPECT_EQ(3u, current_browser->tab_strip_model()
-                     ->GetActiveWebContents()
-                     ->GetAllFrames()
-                     .size());
-  }
-}
-
 // Test that leaving a popup open will not prevent session restore.
 IN_PROC_BROWSER_TEST_F(ContinueWhereILeftOffTest,
                        SessionCookiesBrowserCloseWithPopupOpen) {
   // Set the startup preference to "continue where I left off" and visit a page
   // which stores a session cookie.
   StoreDataWithPage("session_cookies.html");
   Browser* popup = new Browser(
       Browser::CreateParams(Browser::TYPE_POPUP, browser()->profile(), true));
   popup->window()->Show();
 
@@ skipping 348 matching lines @@
   CookieSettingsFactory::GetForProfile(new_browser->profile())
       ->SetDefaultCookieSetting(CONTENT_SETTING_SESSION_ONLY);
   // ... even if background mode is active.
   EnableBackgroundMode();
   new_browser = QuitBrowserAndRestore(new_browser, true);
   StoreDataWithPage(new_browser, "cookies.html");
   DisableBackgroundMode();
   new_browser = QuitBrowserAndRestore(new_browser, true);
   StoreDataWithPage(new_browser, "cookies.html");
 }