MD Settings/On Startup: ignore current tab when using current pages

chrome/browser/custom_home_pages_table_model.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 "chrome/browser/custom_home_pages_table_model.h"
 
 #include <stddef.h>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/i18n/rtl.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/settings_window_manager.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/history/core/browser/history_service.h"
 #include "components/url_formatter/url_formatter.h"
 #include "content/public/browser/web_contents.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/models/table_model_observer.h"
 #include "ui/gfx/codec/png_codec.h"
 #include "url/gurl.h"
 
-namespace {
-
-// Checks whether the given URL should count as one of the "current" pages.
-// Returns true for all pages except dev tools and settings.
-bool ShouldAddPage(const GURL& url) {
-  if (url.is_empty())
-    return false;
-
-  if (url.SchemeIs(content::kChromeDevToolsScheme))
-    return false;
-
-  if (url.SchemeIs(content::kChromeUIScheme)) {

[sky, 2017/06/07 16:08:48]

How come none of the following matters anymore?

[Dan Beam, 2017/06/07 22:49:14]

this code was a roundabout way to ignore the WebContents that started a "Use
current tabs" request.

I changed the code to just pass the requesting WebContents.  this is simpler,
saner, and has the mild mild benefit of working more expectedly in the case of
another settings page being open in your "current tabs" other than the one you
click "Use current tabs" to set your Startup pages from.

-    if (url.host_piece() == chrome::kChromeUISettingsHost ||
-        url.host_piece() == chrome::kChromeUISettingsFrameHost) {
-      return false;
-    }
-
-    // For a settings page, the path will start with "/settings" not "settings"
-    // so find() will return 1, not 0.
-    if (url.host_piece() == chrome::kChromeUIUberHost &&
-        url.path_piece().find(chrome::kChromeUISettingsHost) == 1) {
-      return false;
-    }
-  }
-
-  return true;
-}
-
-}  // namespace
-
 struct CustomHomePagesTableModel::Entry {
   Entry() : task_id(base::CancelableTaskTracker::kBadTaskId) {}
 
   // URL of the page.
   GURL url;
 
   // Page title.  If this is empty, we'll display the URL as the entry.
   base::string16 title;
 
   // If not |base::CancelableTaskTracker::kBadTaskId|, indicates we're loading
@@ skipping 102 matching lines @@
   }
   entries_.erase(entries_.begin() + static_cast<size_t>(index));
 }
 
 void CustomHomePagesTableModel::Remove(int index) {
   RemoveWithoutNotification(index);
   if (observer_)
     observer_->OnItemsRemoved(index, 1);
 }
 
-void CustomHomePagesTableModel::SetToCurrentlyOpenPages() {
+void CustomHomePagesTableModel::SetToCurrentlyOpenPages(
+    const content::WebContents* source_contents) {
   // Remove the current entries.
   while (RowCount())
     RemoveWithoutNotification(0);
 
   // Add tabs from appropriate browser windows.
   int add_index = 0;
   for (auto* browser : *BrowserList::GetInstance()) {
     if (!ShouldIncludeBrowser(browser))
       continue;
 
     for (int tab_index = 0;
          tab_index < browser->tab_strip_model()->count();
          ++tab_index) {
-      const GURL url =
-          browser->tab_strip_model()->GetWebContentsAt(tab_index)->GetURL();
-      if (ShouldAddPage(url))
+      const content::WebContents* contents =
+          browser->tab_strip_model()->GetWebContentsAt(tab_index);
+      if (contents == source_contents)
+        continue;
+      const GURL url = contents->GetURL();
+      if (!url.is_empty() && !url.SchemeIs(content::kChromeDevToolsScheme))

[Dan Beam, 2017/06/07 22:49:14]

and the empty and devtools check is still here

         AddWithoutNotification(add_index++, url);
     }
   }
   LoadAllTitles();
 }
 
 std::vector<GURL> CustomHomePagesTableModel::GetURLs() {
   std::vector<GURL> urls(entries_.size());
   for (size_t i = 0; i < entries_.size(); ++i)
     urls[i] = entries_[i].url;
@@ skipping 105 matching lines @@
     if (observer_ && observable)
       observer_->OnItemsChanged(static_cast<int>(entry_index), 1);
   }
 }
 
 base::string16 CustomHomePagesTableModel::FormattedURL(int row) const {
   base::string16 url = url_formatter::FormatUrl(entries_[row].url);
   url = base::i18n::GetDisplayStringInLTRDirectionality(url);
   return url;
 }