Disable read later star menu option for non-http/https urls.

This restriction comes from ReadingListModel's implementation.

Bug: 1117023
Change-Id: Ibc3f05b7d9523e1dde3661a9559936adb12c52f3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2399015
Commit-Queue: Caroline Rising <corising@chromium.org>
Reviewed-by: Peter Boström <pbos@chromium.org>
Reviewed-by: Olivier Robin <olivierrobin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#805372}

chrome/browser/ui/browser_commands.cc

@@ -1030,6 +1030,15 @@ bool CanBookmarkAllTabs(const Browser* browser) {
          CanBookmarkCurrentTab(browser);
 }
 
+bool CanMoveActiveTabToReadLater(Browser* browser) {
+  GURL url =
+      GetURLToBookmark(browser->tab_strip_model()->GetActiveWebContents());
+  ReadingListModel* model = GetReadingListModel(browser);
+  if (!model)
+    return false;
+  return model->IsUrlSupported(url);
+}
+
 bool MoveCurrentTabToReadLater(Browser* browser) {
   GURL url;
   base::string16 title;

chrome/browser/ui/browser_commands.h

@@ -140,6 +140,7 @@ void BookmarkCurrentTab(Browser* browser);
 bool CanBookmarkCurrentTab(const Browser* browser);
 void BookmarkAllTabs(Browser* browser);
 bool CanBookmarkAllTabs(const Browser* browser);
+bool CanMoveActiveTabToReadLater(Browser* browser);
 bool MoveCurrentTabToReadLater(Browser* browser);
 bool MarkCurrentTabAsReadInReadLater(Browser* browser);
 bool IsCurrentTabUnreadInReadLater(Browser* browser);

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

@@ -8,23 +8,27 @@
 #include "components/omnibox/browser/vector_icons.h"
 #include "components/vector_icons/vector_icons.h"
 #include "ui/base/models/image_model.h"
+#include "ui/native_theme/native_theme.h"
 
 StarMenuModel::StarMenuModel(ui::SimpleMenuModel::Delegate* delegate,
                              bool bookmarked,
+                             bool can_move_to_read_later,
                              bool exists_as_unread_in_read_later)
     : SimpleMenuModel(delegate) {
-  Build(bookmarked, exists_as_unread_in_read_later);
+  Build(bookmarked, can_move_to_read_later, exists_as_unread_in_read_later);
 }
 
 StarMenuModel::~StarMenuModel() = default;
 
 void StarMenuModel::Build(bool bookmarked,
+                          bool can_move_to_read_later,
                           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));
+      ui::ImageModel::FromVectorIcon(
+          omnibox::kStarIcon, ui::NativeTheme::kColorId_DefaultIconColor));
   // TODO(corising): Replace placeholder folder icon with read-later icon once
   // available.
   AddItemWithStringIdAndIcon(
@@ -32,5 +36,13 @@ void StarMenuModel::Build(bool bookmarked,
                                      : 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));
+      ui::ImageModel::FromVectorIcon(
+          vector_icons::kFolderIcon,
+          can_move_to_read_later
+              ? ui::NativeTheme::kColorId_DefaultIconColor
+              : ui::NativeTheme::kColorId_DisabledIconColor));
+  int index = GetIndexOfCommandId(CommandMoveToReadLater);
+  if (index != -1) {
+    SetEnabledAt(index, can_move_to_read_later);
+  }
 }

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

@@ -12,6 +12,7 @@ class StarMenuModel : public ui::SimpleMenuModel {
  public:
   StarMenuModel(ui::SimpleMenuModel::Delegate* delegate,
                 bool bookmarked,
+                bool can_move_to_read_later,
                 bool exists_as_unread_in_read_later);
   StarMenuModel(const StarMenuModel&) = delete;
   StarMenuModel& operator=(const StarMenuModel&) = delete;
@@ -24,7 +25,9 @@ class StarMenuModel : public ui::SimpleMenuModel {
   };
 
  private:
-  void Build(bool bookmarked, bool exists_as_unread_in_read_later);
+  void Build(bool bookmarked,
+             bool can_move_to_read_later,
+             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

@@ -77,7 +77,8 @@ void StarView::OnExecuting(PageActionIconView::ExecuteSource execute_source) {
 void StarView::ExecuteCommand(ExecuteSource source) {
   if (base::FeatureList::IsEnabled(features::kReadLater)) {
     menu_model_ = std::make_unique<StarMenuModel>(
-        this, active(), chrome::IsCurrentTabUnreadInReadLater(browser_));
+        this, active(), chrome::CanMoveActiveTabToReadLater(browser_),
+        chrome::IsCurrentTabUnreadInReadLater(browser_));
     menu_runner_ = std::make_unique<views::MenuRunner>(
         menu_model_.get(),
         views::MenuRunner::HAS_MNEMONICS | views::MenuRunner::FIXED_ANCHOR);

components/reading_list/core/reading_list_model.h

@@ -92,6 +92,9 @@ class ReadingListModel {
   // entries available offline.
   virtual const ReadingListEntry* GetFirstUnreadEntry(bool distilled) const = 0;
 
+  // Returns true if |url| can be added to the reading list.
+  virtual bool IsUrlSupported(const GURL& url) = 0;
+
   // Adds |url| at the top of the unread entries, and removes entries with the
   // same |url| from everywhere else if they exist. The entry title will be a
   // trimmed copy of |title|.

components/reading_list/core/reading_list_model_impl.cc

@@ -318,13 +318,17 @@ void ReadingListModelImpl::RemoveEntryByURLImpl(const GURL& url,
     observer.ReadingListDidApplyChanges(this);
 }
 
+bool ReadingListModelImpl::IsUrlSupported(const GURL& url) {
+  return url.SchemeIsHTTPOrHTTPS();
+}
+
 const ReadingListEntry& ReadingListModelImpl::AddEntry(
     const GURL& url,
     const std::string& title,
     reading_list::EntrySource source) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(loaded());
-  DCHECK(url.SchemeIsHTTPOrHTTPS());
+  DCHECK(IsUrlSupported(url));
   std::unique_ptr<ReadingListModel::ScopedReadingListBatchUpdate>
       scoped_model_batch_updates = nullptr;
   if (GetEntryByURL(url)) {

components/reading_list/core/reading_list_model_impl.h

@@ -65,6 +65,8 @@ class ReadingListModelImpl : public ReadingListModel,
 
   void RemoveEntryByURL(const GURL& url) override;
 
+  bool IsUrlSupported(const GURL& url) override;
+
   const ReadingListEntry& AddEntry(const GURL& url,
                                    const std::string& title,
                                    reading_list::EntrySource source) override;

ios/chrome/browser/reading_list/offline_page_tab_helper_unittest.mm

@@ -89,6 +89,11 @@ class FakeReadingListModel : public ReadingListModel {
     return nullptr;
   }
 
+  bool IsUrlSupported(const GURL& url) override {
+    NOTREACHED();
+    return false;
+  }
+
   const ReadingListEntry& AddEntry(const GURL& url,
                                    const std::string& title,
                                    reading_list::EntrySource source) override {