Read later: open reading list item on active tab if it is NTP.

Currently reading list items are always opened in a new tab. Instead we
want the reading list item to be opened in the active tab if it is the
NTP.

Bug: 1181416
Change-Id: I455a9ea911d5783e5a68df748d3ae58a14f88693
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2716163
Commit-Queue: Caroline Rising <corising@chromium.org>
Reviewed-by: Connie Wan <connily@chromium.org>
Cr-Commit-Position: refs/heads/master@{#857249}

chrome/browser/ui/webui/read_later/read_later_page_handler.cc

@@ -18,10 +18,13 @@
 #include "chrome/browser/ui/browser_finder.h"
 #include "chrome/browser/ui/read_later/reading_list_model_factory.h"
 #include "chrome/browser/ui/webui/read_later/read_later_ui.h"
+#include "chrome/common/webui_url_constants.h"
 #include "components/reading_list/core/reading_list_entry.h"
 #include "components/reading_list/core/reading_list_model.h"
 #include "components/url_formatter/url_formatter.h"
+#include "content/public/browser/web_contents.h"
 #include "ui/base/l10n/time_format.h"
+#include "url/gurl.h"
 
 namespace {
 
@@ -37,6 +40,21 @@ int64_t TimeToUS(const base::Time& time) {
   return (time - base::Time::UnixEpoch()).InMicroseconds();
 }
 
+bool IsActiveTabNTP(Browser* browser) {
+  content::WebContents* web_contents =
+      browser->tab_strip_model()->GetActiveWebContents();
+  if (web_contents) {
+    const GURL site_origin = web_contents->GetLastCommittedURL().GetOrigin();
+    // These are also the NTP urls checked for showing the bookmark bar on the
+    // NTP.
+    if (site_origin == GURL(chrome::kChromeUINewTabURL).GetOrigin() ||
+        site_origin == GURL(chrome::kChromeUINewTabPageURL).GetOrigin()) {
+      return true;
+    }
+  }
+  return false;
+}
+
 }  // namespace
 
 ReadLaterPageHandler::ReadLaterPageHandler(
@@ -62,8 +80,12 @@ void ReadLaterPageHandler::GetReadLaterEntries(
 }
 
 void ReadLaterPageHandler::OpenSavedEntry(const GURL& url) {
-  content::OpenURLParams params(url, content::Referrer(),
-                                WindowOpenDisposition::NEW_FOREGROUND_TAB,
+  // Open in active tab if the user is on the NTP.
+  WindowOpenDisposition open_location =
+      IsActiveTabNTP(browser_) ? WindowOpenDisposition::CURRENT_TAB
+                               : WindowOpenDisposition::NEW_FOREGROUND_TAB;
+
+  content::OpenURLParams params(url, content::Referrer(), open_location,
                                 ui::PAGE_TRANSITION_AUTO_BOOKMARK, false);
   browser_->OpenURL(params);
   reading_list_model_->SetReadStatus(url, true);

chrome/browser/ui/webui/read_later/read_later_page_handler_unittest.cc

@@ -14,6 +14,7 @@
 #include "chrome/browser/ui/read_later/read_later_test_utils.h"
 #include "chrome/browser/ui/read_later/reading_list_model_factory.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/common/webui_url_constants.h"
 #include "chrome/test/base/browser_with_test_window_test.h"
 #include "chrome/test/base/test_browser_window.h"
 #include "components/reading_list/core/reading_list_model.h"
@@ -135,8 +136,35 @@ TEST_F(TestReadLaterPageHandlerTest, GetReadLaterEntries) {
   handler()->GetReadLaterEntries(std::move(callback1));
 }
 
-TEST_F(TestReadLaterPageHandlerTest, OpenSavedEntry) {
-  // Check that OpenSavedEntry opens a new tab.
+TEST_F(TestReadLaterPageHandlerTest, OpenSavedEntryOnNTP) {
+  // Open and navigate to NTP.
+  AddTabWithTitle(browser(), GURL(chrome::kChromeUINewTabURL), "NTP");
+
+  // Check that OpenSavedEntry from the NTP does not open a new tab.
+  EXPECT_EQ(browser()->tab_strip_model()->count(), 5);
+  handler()->OpenSavedEntry(GURL(kTabUrl3));
+  EXPECT_EQ(browser()->tab_strip_model()->count(), 5);
+
+  // Get Read later entries.
+  read_later::mojom::PageHandler::GetReadLaterEntriesCallback callback1 =
+      base::BindLambdaForTesting(
+          [&](read_later::mojom::ReadLaterEntriesByStatusPtr
+                  entries_by_status) {
+            ASSERT_EQ(1u, entries_by_status->unread_entries.size());
+            ASSERT_EQ(1u, entries_by_status->read_entries.size());
+
+            auto* entry1 = entries_by_status->unread_entries[0].get();
+            ExpectNewReadLaterEntry(entry1, GURL(kTabUrl1), kTabName1);
+
+            auto* entry2 = entries_by_status->read_entries[0].get();
+            ExpectNewReadLaterEntry(entry2, GURL(kTabUrl3), kTabName3);
+          });
+
+  handler()->GetReadLaterEntries(std::move(callback1));
+}
+
+TEST_F(TestReadLaterPageHandlerTest, OpenSavedEntryNotOnNTP) {
+  // Check that OpenSavedEntry opens a new tab when not on the NTP.
   EXPECT_EQ(browser()->tab_strip_model()->count(), 4);
   handler()->OpenSavedEntry(GURL(kTabUrl3));
   EXPECT_EQ(browser()->tab_strip_model()->count(), 5);