diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd index d5858006a30d6..39633639e9d83 100644 --- a/chrome/app/generated_resources.grd +++ b/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 + + + Add Bookmark + + + Edit Bookmark + + + Add to Read later + + + Mark as Read + + Click to go back, hold to see history diff --git a/chrome/app/generated_resources_grd/IDS_STAR_VIEW_MENU_ADD_BOOKMARK.png.sha1 b/chrome/app/generated_resources_grd/IDS_STAR_VIEW_MENU_ADD_BOOKMARK.png.sha1 new file mode 100644 index 0000000000000..7d33700ffb2f0 --- /dev/null +++ b/chrome/app/generated_resources_grd/IDS_STAR_VIEW_MENU_ADD_BOOKMARK.png.sha1 @@ -0,0 +1 @@ +cd19bd38c1c505428e1b952dff29ea2dcc23a749 \ No newline at end of file diff --git a/chrome/app/generated_resources_grd/IDS_STAR_VIEW_MENU_EDIT_BOOKMARK.png.sha1 b/chrome/app/generated_resources_grd/IDS_STAR_VIEW_MENU_EDIT_BOOKMARK.png.sha1 new file mode 100644 index 0000000000000..5140577e3c4e3 --- /dev/null +++ b/chrome/app/generated_resources_grd/IDS_STAR_VIEW_MENU_EDIT_BOOKMARK.png.sha1 @@ -0,0 +1 @@ +ca95daa02a7ba6b6dcc742fa2359f58058ac8025 \ No newline at end of file diff --git a/chrome/app/generated_resources_grd/IDS_STAR_VIEW_MENU_MARK_AS_READ.png.sha1 b/chrome/app/generated_resources_grd/IDS_STAR_VIEW_MENU_MARK_AS_READ.png.sha1 new file mode 100644 index 0000000000000..5140577e3c4e3 --- /dev/null +++ b/chrome/app/generated_resources_grd/IDS_STAR_VIEW_MENU_MARK_AS_READ.png.sha1 @@ -0,0 +1 @@ +ca95daa02a7ba6b6dcc742fa2359f58058ac8025 \ No newline at end of file diff --git a/chrome/app/generated_resources_grd/IDS_STAR_VIEW_MENU_MOVE_TO_READ_LATER.png.sha1 b/chrome/app/generated_resources_grd/IDS_STAR_VIEW_MENU_MOVE_TO_READ_LATER.png.sha1 new file mode 100644 index 0000000000000..7d33700ffb2f0 --- /dev/null +++ b/chrome/app/generated_resources_grd/IDS_STAR_VIEW_MENU_MOVE_TO_READ_LATER.png.sha1 @@ -0,0 +1 @@ +cd19bd38c1c505428e1b952dff29ea2dcc23a749 \ No newline at end of file diff --git a/chrome/browser/ui/BUILD.gn b/chrome/browser/ui/BUILD.gn index de7954c5e0212..ff9eaefeda75c 100644 --- a/chrome/browser/ui/BUILD.gn +++ b/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", diff --git a/chrome/browser/ui/browser_commands.cc b/chrome/browser/ui/browser_commands.cc index dc3969368afc3..d332f3913135b 100644 --- a/chrome/browser/ui/browser_commands.cc +++ b/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(); diff --git a/chrome/browser/ui/browser_commands.h b/chrome/browser/ui/browser_commands.h index 07c068bb5f07a..a153d33c24153 100644 --- a/chrome/browser/ui/browser_commands.h +++ b/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); diff --git a/chrome/browser/ui/views/location_bar/star_menu_model.cc b/chrome/browser/ui/views/location_bar/star_menu_model.cc new file mode 100644 index 0000000000000..e987992294e2b --- /dev/null +++ b/chrome/browser/ui/views/location_bar/star_menu_model.cc @@ -0,0 +1,36 @@ +// 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)); +} diff --git a/chrome/browser/ui/views/location_bar/star_menu_model.h b/chrome/browser/ui/views/location_bar/star_menu_model.h new file mode 100644 index 0000000000000..e9e0bb73581d4 --- /dev/null +++ b/chrome/browser/ui/views/location_bar/star_menu_model.h @@ -0,0 +1,30 @@ +// 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_ diff --git a/chrome/browser/ui/views/location_bar/star_view.cc b/chrome/browser/ui/views/location_bar/star_view.cc index 05edc8ec06fab..2f89bca50aa12 100644 --- a/chrome/browser/ui/views/location_bar/star_view.cc +++ b/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( + this, active(), chrome::IsCurrentTabUnreadInReadLater(browser_)); + menu_runner_ = std::make_unique( + 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(); +} diff --git a/chrome/browser/ui/views/location_bar/star_view.h b/chrome/browser/ui/views/location_bar/star_view.h index 2c8c611e9c45d..40824d7e12bf0 100644 --- a/chrome/browser/ui/views/location_bar/star_view.h +++ b/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 + #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 menu_runner_; + std::unique_ptr menu_model_; + BooleanPrefMember edit_bookmarks_enabled_; DISALLOW_COPY_AND_ASSIGN(StarView); diff --git a/chrome/browser/ui/views/location_bar/star_view_interactive_uitest.cc b/chrome/browser/ui/views/location_bar/star_view_interactive_uitest.cc index 5794245310513..3b277b436a556 100644 --- a/chrome/browser/ui/views/location_bar/star_view_interactive_uitest.cc +++ b/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( + 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 diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn index 5ba36d5fa024b..c6c9da690a317 100644 --- a/chrome/test/BUILD.gn +++ b/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",