Add Read Later omnibox star entry point.

Add a new menu triggered by the StarView which gives the option to
either bookmark the tab or save it for later. If bookmark is selected
the user flow matches previous behavior. If save to read later is
selected the tab will be saved to the reading list model. Note this
change is hidden behind the read later feature flag.

Follow-up work: Update the read later icon in the menu when new icons
are available. Update the omnibox star tooltip. Update StarView/bookmark metrics and
add metrics for read later. Update menu size.

Bug: 1117023
Change-Id: I09c6cf4973338faa272bc6ec6da06e26f0f9aa8f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2359550
Commit-Queue: Caroline Rising <corising@chromium.org>
Reviewed-by: Peter Boström <pbos@chromium.org>
Reviewed-by: Connie Wan <connily@chromium.org>
Cr-Commit-Position: refs/heads/master@{#801449}

chrome/app/generated_resources.grd

@@ -5652,6 +5652,20 @@ Keep your key file in a safe place. You will need it to create new versions of y
         </message>
       </if>
 
+      <!-- Star View menu -->
+      <message name="IDS_STAR_VIEW_MENU_ADD_BOOKMARK" desc="The item label of the menu triggered from the star icon in the location bar for adding a bookmark.">
+        Add Bookmark
+      </message>
+      <message name="IDS_STAR_VIEW_MENU_EDIT_BOOKMARK" desc="The item label of the menu triggered from the star icon in the location bar for editing a bookmark.">
+        Edit Bookmark
+      </message>
+      <message name="IDS_STAR_VIEW_MENU_MOVE_TO_READ_LATER" desc="The item label of the menu triggered from the star icon in the location bar for moving the current tab to read later.">
+        Add to Read later
+      </message>
+      <message name="IDS_STAR_VIEW_MENU_MARK_AS_READ" desc="The item label of the menu triggered from the star icon in the location bar for marking the current tab's read later entry as read.">
+        Mark as Read
+      </message>
+
       <!--Tooltip strings-->
       <message name="IDS_TOOLTIP_BACK" desc="The tooltip for back button">
         Click to go back, hold to see history

chrome/app/generated_resources_grd/IDS_STAR_VIEW_MENU_ADD_BOOKMARK.png.sha1

+cd19bd38c1c505428e1b952dff29ea2dcc23a749
\ No newline at end of file

chrome/app/generated_resources_grd/IDS_STAR_VIEW_MENU_EDIT_BOOKMARK.png.sha1

+ca95daa02a7ba6b6dcc742fa2359f58058ac8025
\ No newline at end of file

chrome/app/generated_resources_grd/IDS_STAR_VIEW_MENU_MARK_AS_READ.png.sha1

+ca95daa02a7ba6b6dcc742fa2359f58058ac8025
\ No newline at end of file

chrome/app/generated_resources_grd/IDS_STAR_VIEW_MENU_MOVE_TO_READ_LATER.png.sha1

+cd19bd38c1c505428e1b952dff29ea2dcc23a749
\ No newline at end of file

chrome/browser/ui/BUILD.gn

@@ -3571,6 +3571,8 @@ static_library("ui") {
       "views/location_bar/permission_chip.h",
       "views/location_bar/selected_keyword_view.cc",
       "views/location_bar/selected_keyword_view.h",
+      "views/location_bar/star_menu_model.cc",
+      "views/location_bar/star_menu_model.h",
       "views/location_bar/star_view.cc",
       "views/location_bar/star_view.h",
       "views/location_bar/zoom_bubble_view.cc",

chrome/browser/ui/browser_commands.cc

@@ -57,6 +57,7 @@
 #include "chrome/browser/ui/location_bar/location_bar.h"
 #include "chrome/browser/ui/passwords/manage_passwords_ui_controller.h"
 #include "chrome/browser/ui/qrcode_generator/qrcode_generator_bubble_controller.h"
+#include "chrome/browser/ui/read_later/reading_list_model_factory.h"
 #include "chrome/browser/ui/scoped_tabbed_browser_displayer.h"
 #include "chrome/browser/ui/send_tab_to_self/send_tab_to_self_bubble_controller.h"
 #include "chrome/browser/ui/status_bubble.h"
@@ -87,6 +88,8 @@
 #include "components/google/core/common/google_util.h"
 #include "components/omnibox/browser/omnibox_prefs.h"
 #include "components/prefs/pref_service.h"
+#include "components/reading_list/core/reading_list_entry.h"
+#include "components/reading_list/core/reading_list_model.h"
 #include "components/services/app_service/public/mojom/types.mojom.h"
 #include "components/sessions/core/live_tab_context.h"
 #include "components/sessions/core/tab_restore_service.h"
@@ -208,6 +211,27 @@ void CreateAndShowNewWindowWithContents(
                                                  TabStripModel::ADD_ACTIVE);
 }
 
+bool GetActiveTabURLAndTitleToSave(Browser* browser,
+                                   GURL* url,
+                                   base::string16* title) {
+  content::WebContents* web_contents =
+      browser->tab_strip_model()->GetActiveWebContents();
+  // |web_contents| can be nullptr if the last tab in the browser was closed
+  // but the browser wasn't closed yet. https://crbug.com/799668
+  if (!web_contents)
+    return false;
+  chrome::GetURLAndTitleToBookmark(web_contents, url, title);
+  return true;
+}
+
+ReadingListModel* GetReadingListModel(Browser* browser) {
+  ReadingListModel* model =
+      ReadingListModelFactory::GetForBrowserContext(browser->profile());
+  if (!model || !model->loaded())
+    return nullptr;  // Ignore requests until model has loaded.
+  return model;
+}
+
 }  // namespace
 
 using base::UserMetricsAction;
@@ -1006,6 +1030,45 @@ bool CanBookmarkAllTabs(const Browser* browser) {
          CanBookmarkCurrentTab(browser);
 }
 
+bool MoveCurrentTabToReadLater(Browser* browser) {
+  GURL url;
+  base::string16 title;
+  ReadingListModel* model = GetReadingListModel(browser);
+  if (!model || !GetActiveTabURLAndTitleToSave(browser, &url, &title))
+    return false;
+  model->AddEntry(url, base::UTF16ToUTF8(title),
+                  reading_list::EntrySource::ADDED_VIA_CURRENT_APP);
+  // Close current tab.
+  int index = browser->tab_strip_model()->active_index();
+  browser->tab_strip_model()->CloseWebContentsAt(
+      index, TabStripModel::CLOSE_CREATE_HISTORICAL_TAB |
+                 TabStripModel::CLOSE_USER_GESTURE);
+  return true;
+}
+
+bool MarkCurrentTabAsReadInReadLater(Browser* browser) {
+  GURL url;
+  base::string16 title;
+  ReadingListModel* model = GetReadingListModel(browser);
+  if (!model || !GetActiveTabURLAndTitleToSave(browser, &url, &title))
+    return false;
+  const ReadingListEntry* entry = model->GetEntryByURL(url);
+  // Mark current tab as read.
+  if (entry && !entry->IsRead())
+    model->SetReadStatus(url, true);
+  return entry != nullptr;
+}
+
+bool IsCurrentTabUnreadInReadLater(Browser* browser) {
+  GURL url;
+  base::string16 title;
+  ReadingListModel* model = GetReadingListModel(browser);
+  if (!model || !GetActiveTabURLAndTitleToSave(browser, &url, &title))
+    return false;
+  const ReadingListEntry* entry = model->GetEntryByURL(url);
+  return entry && !entry->IsRead();
+}
+
 void SaveCreditCard(Browser* browser) {
   WebContents* web_contents =
       browser->tab_strip_model()->GetActiveWebContents();

chrome/browser/ui/browser_commands.h

@@ -140,6 +140,9 @@ void BookmarkCurrentTab(Browser* browser);
 bool CanBookmarkCurrentTab(const Browser* browser);
 void BookmarkAllTabs(Browser* browser);
 bool CanBookmarkAllTabs(const Browser* browser);
+bool MoveCurrentTabToReadLater(Browser* browser);
+bool MarkCurrentTabAsReadInReadLater(Browser* browser);
+bool IsCurrentTabUnreadInReadLater(Browser* browser);
 void SaveCreditCard(Browser* browser);
 void MigrateLocalCards(Browser* browser);
 void MaybeShowSaveLocalCardSignInPromo(Browser* browser);

chrome/browser/ui/views/location_bar/star_menu_model.cc

+// Copyright 2020 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/ui/views/location_bar/star_menu_model.h"
+
+#include "chrome/grit/generated_resources.h"
+#include "components/omnibox/browser/vector_icons.h"
+#include "components/vector_icons/vector_icons.h"
+#include "ui/base/models/image_model.h"
+
+StarMenuModel::StarMenuModel(ui::SimpleMenuModel::Delegate* delegate,
+                             bool bookmarked,
+                             bool exists_as_unread_in_read_later)
+    : SimpleMenuModel(delegate) {
+  Build(bookmarked, exists_as_unread_in_read_later);
+}
+
+StarMenuModel::~StarMenuModel() = default;
+
+void StarMenuModel::Build(bool bookmarked,
+                          bool exists_as_unread_in_read_later) {
+  AddItemWithStringIdAndIcon(
+      CommandBookmark,
+      bookmarked ? IDS_STAR_VIEW_MENU_EDIT_BOOKMARK
+                 : IDS_STAR_VIEW_MENU_ADD_BOOKMARK,
+      ui::ImageModel::FromVectorIcon(omnibox::kStarIcon));
+  // TODO(corising): Replace placeholder folder icon with read-later icon once
+  // available.
+  AddItemWithStringIdAndIcon(
+      exists_as_unread_in_read_later ? CommandMarkAsRead
+                                     : CommandMoveToReadLater,
+      exists_as_unread_in_read_later ? IDS_STAR_VIEW_MENU_MARK_AS_READ
+                                     : IDS_STAR_VIEW_MENU_MOVE_TO_READ_LATER,
+      ui::ImageModel::FromVectorIcon(vector_icons::kFolderIcon));
+}

chrome/browser/ui/views/location_bar/star_menu_model.h

+// Copyright 2020 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.
+
+#ifndef CHROME_BROWSER_UI_VIEWS_LOCATION_BAR_STAR_MENU_MODEL_H_
+#define CHROME_BROWSER_UI_VIEWS_LOCATION_BAR_STAR_MENU_MODEL_H_
+
+#include "base/macros.h"
+#include "ui/base/models/simple_menu_model.h"
+
+class StarMenuModel : public ui::SimpleMenuModel {
+ public:
+  StarMenuModel(ui::SimpleMenuModel::Delegate* delegate,
+                bool bookmarked,
+                bool exists_as_unread_in_read_later);
+  StarMenuModel(const StarMenuModel&) = delete;
+  StarMenuModel& operator=(const StarMenuModel&) = delete;
+  ~StarMenuModel() override;
+
+  enum StarMenuCommand {
+    CommandBookmark,
+    CommandMoveToReadLater,
+    CommandMarkAsRead
+  };
+
+ private:
+  void Build(bool bookmarked, bool exists_as_unread_in_read_later);
+};
+
+#endif  // CHROME_BROWSER_UI_VIEWS_LOCATION_BAR_STAR_MENU_MODEL_H_

chrome/browser/ui/views/location_bar/star_view.cc

@@ -15,9 +15,11 @@
 #include "chrome/browser/ui/bookmarks/bookmark_stats.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
+#include "chrome/browser/ui/ui_features.h"
 #include "chrome/browser/ui/view_ids.h"
 #include "chrome/browser/ui/views/bookmarks/bookmark_bubble_view.h"
 #include "chrome/browser/ui/views/in_product_help/feature_promo_bubble_view.h"
+#include "chrome/browser/ui/views/location_bar/star_menu_model.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/bookmarks/common/bookmark_pref_names.h"
 #include "components/omnibox/browser/vector_icons.h"
@@ -27,6 +29,7 @@
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/color_utils.h"
 #include "ui/gfx/paint_vector_icon.h"
+#include "ui/views/controls/menu/menu_runner.h"
 
 StarView::StarView(CommandUpdater* command_updater,
                    Browser* browser,
@@ -38,6 +41,7 @@ StarView::StarView(CommandUpdater* command_updater,
                          page_action_icon_delegate),
       browser_(browser) {
   DCHECK(browser_);
+
   edit_bookmarks_enabled_.Init(
       bookmarks::prefs::kEditBookmarksEnabled, browser_->profile()->GetPrefs(),
       base::BindRepeating(&StarView::EditBookmarksPrefUpdated,
@@ -71,8 +75,19 @@ void StarView::OnExecuting(PageActionIconView::ExecuteSource execute_source) {
 }
 
 void StarView::ExecuteCommand(ExecuteSource source) {
-  OnExecuting(source);
-  chrome::BookmarkCurrentTab(browser_);
+  if (base::FeatureList::IsEnabled(features::kReadLater)) {
+    menu_model_ = std::make_unique<StarMenuModel>(
+        this, active(), chrome::IsCurrentTabUnreadInReadLater(browser_));
+    menu_runner_ = std::make_unique<views::MenuRunner>(
+        menu_model_.get(),
+        views::MenuRunner::HAS_MNEMONICS | views::MenuRunner::FIXED_ANCHOR);
+    menu_runner_->RunMenuAt(GetWidget(), nullptr, GetAnchorBoundsInScreen(),
+                            views::MenuAnchorPosition::kTopRight,
+                            ui::MENU_SOURCE_NONE);
+  } else {
+    OnExecuting(source);
+    chrome::BookmarkCurrentTab(browser_);
+  }
 }
 
 views::BubbleDialogDelegate* StarView::GetBubble() const {
@@ -95,3 +110,27 @@ const char* StarView::GetClassName() const {
 void StarView::EditBookmarksPrefUpdated() {
   Update();
 }
+
+void StarView::ExecuteCommand(int command_id, int event_flags) {
+  switch (command_id) {
+    case StarMenuModel::CommandBookmark:
+      chrome::BookmarkCurrentTab(browser_);
+      break;
+    case StarMenuModel::CommandMoveToReadLater:
+      chrome::MoveCurrentTabToReadLater(browser_);
+      break;
+    case StarMenuModel::CommandMarkAsRead:
+      chrome::MarkCurrentTabAsReadInReadLater(browser_);
+      break;
+    default:
+      NOTREACHED();
+  }
+}
+
+void StarView::MenuClosed(ui::SimpleMenuModel* source) {
+  if (!GetBubble() || !GetBubble()->GetWidget() ||
+      !GetBubble()->GetWidget()->IsVisible()) {
+    SetHighlighted(false);
+  }
+  menu_runner_.reset();
+}

chrome/browser/ui/views/location_bar/star_view.h

@@ -5,16 +5,21 @@
 #ifndef CHROME_BROWSER_UI_VIEWS_LOCATION_BAR_STAR_VIEW_H_
 #define CHROME_BROWSER_UI_VIEWS_LOCATION_BAR_STAR_VIEW_H_
 
+#include <memory>
+
 #include "base/macros.h"
 #include "base/scoped_observer.h"
 #include "chrome/browser/ui/views/page_action/page_action_icon_view.h"
 #include "components/prefs/pref_member.h"
+#include "ui/base/models/simple_menu_model.h"
 
 class Browser;
 class CommandUpdater;
+class StarMenuModel;
 
 // The star icon to show a bookmark bubble.
-class StarView : public PageActionIconView {
+class StarView : public PageActionIconView,
+                 public ui::SimpleMenuModel::Delegate {
  public:
   StarView(CommandUpdater* command_updater,
            Browser* browser,
@@ -22,6 +27,8 @@ class StarView : public PageActionIconView {
            PageActionIconView::Delegate* page_action_icon_delegate);
   ~StarView() override;
 
+  StarMenuModel* menu_model_for_test() { return menu_model_.get(); }
+
  protected:
   // PageActionIconView:
   void UpdateImpl() override;
@@ -36,8 +43,15 @@ class StarView : public PageActionIconView {
   void EditBookmarksPrefUpdated();
   bool IsBookmarkStarHiddenByExtension() const;
 
+  // ui::SimpleMenuModel::Delegate:
+  void ExecuteCommand(int command_id, int event_flags) override;
+  void MenuClosed(ui::SimpleMenuModel* source) override;
+
   Browser* const browser_;
 
+  std::unique_ptr<views::MenuRunner> menu_runner_;
+  std::unique_ptr<StarMenuModel> menu_model_;
+
   BooleanPrefMember edit_bookmarks_enabled_;
 
   DISALLOW_COPY_AND_ASSIGN(StarView);

chrome/browser/ui/views/location_bar/star_view_interactive_uitest.cc

@@ -10,9 +10,13 @@
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
 #include "chrome/browser/bookmarks/bookmark_model_factory.h"
+#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/browser/ui/ui_features.h"
 #include "chrome/browser/ui/views/bookmarks/bookmark_bubble_view.h"
 #include "chrome/browser/ui/views/frame/browser_view.h"
+#include "chrome/browser/ui/views/location_bar/star_menu_model.h"
 #include "chrome/browser/ui/views/page_action/page_action_icon_view.h"
 #include "chrome/browser/ui/views/toolbar/toolbar_view.h"
 #include "chrome/common/pref_names.h"
@@ -23,12 +27,15 @@
 #include "components/bookmarks/browser/bookmark_utils.h"
 #include "components/bookmarks/test/bookmark_test_helpers.h"
 #include "components/prefs/pref_service.h"
+#include "components/reading_list/core/reading_list_model.h"
+#include "components/reading_list/core/reading_list_model_observer.h"
 #include "content/public/test/browser_test.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/test_utils.h"
 #include "ui/base/ui_base_switches.h"
 #include "ui/events/event_utils.h"
 #include "ui/views/animation/test/ink_drop_host_view_test_api.h"
+#include "ui/views/test/button_test_api.h"
 
 namespace {
 
@@ -120,4 +127,109 @@ IN_PROC_BROWSER_TEST_F(StarViewTest, InkDropHighlighted) {
   }
 }
 
+class StarViewTestWithReadLaterEnabled : public InProcessBrowserTest {
+ public:
+  StarViewTestWithReadLaterEnabled() {
+    feature_list_.InitAndEnableFeature(features::kReadLater);
+  }
+  StarViewTestWithReadLaterEnabled(const StarViewTestWithReadLaterEnabled&) =
+      delete;
+  StarViewTestWithReadLaterEnabled& operator=(
+      const StarViewTestWithReadLaterEnabled&) = delete;
+  ~StarViewTestWithReadLaterEnabled() override = default;
+
+  StarView* GetStarIcon() {
+    return static_cast<StarView*>(
+        BrowserView::GetBrowserViewForBrowser(browser())
+            ->toolbar_button_provider()
+            ->GetPageActionIconView(PageActionIconType::kBookmarkStar));
+  }
+
+  void OpenStarViewMenu(StarView* star_icon) {
+    ui::MouseEvent pressed_event(
+        ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(), ui::EventTimeForNow(),
+        ui::EF_LEFT_MOUSE_BUTTON, ui::EF_LEFT_MOUSE_BUTTON);
+    ui::MouseEvent released_event(ui::ET_MOUSE_RELEASED, gfx::Point(),
+                                  gfx::Point(), ui::EventTimeForNow(),
+                                  ui::EF_LEFT_MOUSE_BUTTON,
+                                  ui::EF_LEFT_MOUSE_BUTTON);
+
+    views::test::ButtonTestApi(star_icon).NotifyClick(pressed_event);
+    views::test::ButtonTestApi(star_icon).NotifyClick(released_event);
+  }
+
+ private:
+  base::test::ScopedFeatureList feature_list_;
+};
+
+// Verifies clicking the bookmark button in the StarView's menu bookmarks the
+// page.
+IN_PROC_BROWSER_TEST_F(StarViewTestWithReadLaterEnabled,
+                       AddBookmarkFromStarViewMenuBookmarksUrl) {
+  bookmarks::BookmarkModel* bookmark_model =
+      BookmarkModelFactory::GetForBrowserContext(browser()->profile());
+  bookmarks::test::WaitForBookmarkModelToLoad(bookmark_model);
+
+  StarView* star_icon = GetStarIcon();
+  const GURL current_url =
+      browser()->tab_strip_model()->GetActiveWebContents()->GetVisibleURL();
+
+  // The page should not initially be bookmarked.
+  EXPECT_FALSE(bookmark_model->IsBookmarked(current_url));
+  EXPECT_FALSE(star_icon->active());
+
+  OpenStarViewMenu(star_icon);
+
+  // The page should not be bookmarked when the menu is opened.
+  EXPECT_FALSE(bookmark_model->IsBookmarked(current_url));
+  EXPECT_FALSE(star_icon->active());
+
+  StarMenuModel* menu_model = star_icon->menu_model_for_test();
+
+  // Activate "Add Bookmark" button in the menu and verify the bookmark is
+  // added.
+  const int bookmark_command_index =
+      menu_model->GetIndexOfCommandId(StarMenuModel::CommandBookmark);
+  menu_model->ActivatedAt(bookmark_command_index);
+
+  EXPECT_TRUE(bookmark_model->IsBookmarked(current_url));
+  EXPECT_TRUE(star_icon->active());
+}
+
+// Verifies clicking the Read Later button in the StarView's menu saves the page
+// to the read later model.
+IN_PROC_BROWSER_TEST_F(StarViewTestWithReadLaterEnabled,
+                       AddToReadLaterFromStarViewMenuSavesUrlToReadLater) {
+  ReadingListModel* reading_list_model =
+      ReadingListModelFactory::GetForBrowserContext(browser()->profile());
+  test::ReadingListLoadObserver(reading_list_model).Wait();
+  GURL url("http://www.test.com/");
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  StarView* star_icon = GetStarIcon();
+  const GURL current_url =
+      browser()->tab_strip_model()->GetActiveWebContents()->GetVisibleURL();
+
+  // The page should not initially be in model.
+  EXPECT_EQ(reading_list_model->GetEntryByURL(current_url), nullptr);
+  EXPECT_FALSE(star_icon->active());
+
+  OpenStarViewMenu(star_icon);
+
+  // The page should not be bookmarked when the menu is opened.
+  EXPECT_EQ(reading_list_model->GetEntryByURL(current_url), nullptr);
+  EXPECT_FALSE(star_icon->active());
+
+  StarMenuModel* menu_model = star_icon->menu_model_for_test();
+
+  // // Activate "Add to Read later" button in the menu and verify the entry is
+  // added.
+  const int read_later_command_index =
+      menu_model->GetIndexOfCommandId(StarMenuModel::CommandMoveToReadLater);
+  menu_model->ActivatedAt(read_later_command_index);
+
+  EXPECT_NE(reading_list_model->GetEntryByURL(current_url), nullptr);
+  EXPECT_FALSE(star_icon->active());
+}
+
 }  // namespace

chrome/test/BUILD.gn

@@ -6194,6 +6194,8 @@ if (!is_android) {
 
     if (toolkit_views) {
       sources += [
+        "../browser/ui/read_later/read_later_test_utils.cc",
+        "../browser/ui/read_later/read_later_test_utils.h",
         "../browser/ui/views/bookmarks/bookmark_bar_view_test.cc",
         "../browser/ui/views/bookmarks/bookmark_bar_view_test_helper.h",
         "../browser/ui/views/certificate_selector_browsertest.cc",