Commit d14d3ce7 authored by edchin's avatar edchin Committed by Commit Bot

[ios] Handle EditBookmarks pref in ui/popup_menu/

EditBookmarksEnabled is an enterprise policy that determines whether
the user can edit any bookmarks. The policy is mapped to a user pref.

This CL is part of a series of CLs that add support for this
enterprise policy on the iOS platform.

Specifically in this CL, the add/edit bookmark tools menu item is
disabled when EditBookmarksEnabled is false.

Bug: 1072326
Change-Id: I3e9ce72e887518503867e661ecffacc076612ad8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2156142Reviewed-by: default avatarGauthier Ambard <gambard@chromium.org>
Commit-Queue: edchin <edchin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#761879}
parent b23214c2
...@@ -43,9 +43,12 @@ source_set("popup_menu") { ...@@ -43,9 +43,12 @@ source_set("popup_menu") {
"resources:popup_menu_voice_search", "resources:popup_menu_voice_search",
"//base", "//base",
"//components/bookmarks/browser", "//components/bookmarks/browser",
"//components/bookmarks/common",
"//components/feature_engagement/public", "//components/feature_engagement/public",
"//components/language/ios/browser", "//components/language/ios/browser",
"//components/open_from_clipboard", "//components/open_from_clipboard",
"//components/prefs",
"//components/prefs/ios",
"//components/translate/core/browser", "//components/translate/core/browser",
"//ios/chrome/app/strings", "//ios/chrome/app/strings",
"//ios/chrome/browser", "//ios/chrome/browser",
...@@ -55,6 +58,7 @@ source_set("popup_menu") { ...@@ -55,6 +58,7 @@ source_set("popup_menu") {
"//ios/chrome/browser/find_in_page", "//ios/chrome/browser/find_in_page",
"//ios/chrome/browser/main:public", "//ios/chrome/browser/main:public",
"//ios/chrome/browser/overlays", "//ios/chrome/browser/overlays",
"//ios/chrome/browser/policy:feature_flags",
"//ios/chrome/browser/reading_list", "//ios/chrome/browser/reading_list",
"//ios/chrome/browser/search_engines", "//ios/chrome/browser/search_engines",
"//ios/chrome/browser/translate", "//ios/chrome/browser/translate",
...@@ -108,15 +112,22 @@ source_set("unit_tests") { ...@@ -108,15 +112,22 @@ source_set("unit_tests") {
":popup_menu", ":popup_menu",
"//base", "//base",
"//base/test:test_support", "//base/test:test_support",
"//components/bookmarks/browser",
"//components/bookmarks/common",
"//components/bookmarks/test",
"//components/feature_engagement/test:test_support", "//components/feature_engagement/test:test_support",
"//components/language/ios/browser", "//components/language/ios/browser",
"//components/prefs",
"//components/prefs:test_support",
"//components/reading_list/core", "//components/reading_list/core",
"//components/translate/core/browser", "//components/translate/core/browser",
"//ios/chrome/browser/bookmarks",
"//ios/chrome/browser/browser_state:test_support", "//ios/chrome/browser/browser_state:test_support",
"//ios/chrome/browser/main:test_support", "//ios/chrome/browser/main:test_support",
"//ios/chrome/browser/overlays", "//ios/chrome/browser/overlays",
"//ios/chrome/browser/overlays/public/web_content_area", "//ios/chrome/browser/overlays/public/web_content_area",
"//ios/chrome/browser/overlays/test", "//ios/chrome/browser/overlays/test",
"//ios/chrome/browser/policy:feature_flags",
"//ios/chrome/browser/ui/popup_menu/cells", "//ios/chrome/browser/ui/popup_menu/cells",
"//ios/chrome/browser/ui/popup_menu/public:popup_menu_ui", "//ios/chrome/browser/ui/popup_menu/public:popup_menu_ui",
"//ios/chrome/browser/ui/toolbar/test", "//ios/chrome/browser/ui/toolbar/test",
......
...@@ -239,6 +239,7 @@ PopupMenuCommandType CommandTypeFromPopupType(PopupMenuType type) { ...@@ -239,6 +239,7 @@ PopupMenuCommandType CommandTypeFromPopupType(PopupMenuType type) {
static_cast<id<BrowserCommands>>(self.browser->GetCommandDispatcher()); static_cast<id<BrowserCommands>>(self.browser->GetCommandDispatcher());
self.mediator.bookmarkModel = ios::BookmarkModelFactory::GetForBrowserState( self.mediator.bookmarkModel = ios::BookmarkModelFactory::GetForBrowserState(
self.browser->GetBrowserState()); self.browser->GetBrowserState());
self.mediator.prefService = self.browser->GetBrowserState()->GetPrefs();
self.mediator.templateURLService = self.mediator.templateURLService =
ios::TemplateURLServiceFactory::GetForBrowserState( ios::TemplateURLServiceFactory::GetForBrowserState(
self.browser->GetBrowserState()); self.browser->GetBrowserState());
......
...@@ -19,6 +19,7 @@ class Tracker; ...@@ -19,6 +19,7 @@ class Tracker;
@protocol BrowserCommands; @protocol BrowserCommands;
class OverlayPresenter; class OverlayPresenter;
@protocol PopupMenuConsumer; @protocol PopupMenuConsumer;
class PrefService;
class ReadingListModel; class ReadingListModel;
class TemplateURLService; class TemplateURLService;
class WebStateList; class WebStateList;
...@@ -54,6 +55,8 @@ class WebStateList; ...@@ -54,6 +55,8 @@ class WebStateList;
@property(nonatomic, assign) feature_engagement::Tracker* engagementTracker; @property(nonatomic, assign) feature_engagement::Tracker* engagementTracker;
// The bookmarks model to know if the page is bookmarked. // The bookmarks model to know if the page is bookmarked.
@property(nonatomic, assign) bookmarks::BookmarkModel* bookmarkModel; @property(nonatomic, assign) bookmarks::BookmarkModel* bookmarkModel;
// Pref service to retrieve preference values.
@property(nonatomic, assign) PrefService* prefService;
// The template url service to use for checking whether search by image is // The template url service to use for checking whether search by image is
// available. // available.
@property(nonatomic, assign) TemplateURLService* templateURLService; @property(nonatomic, assign) TemplateURLService* templateURLService;
......
...@@ -10,11 +10,15 @@ ...@@ -10,11 +10,15 @@
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
#include "base/strings/sys_string_conversions.h" #include "base/strings/sys_string_conversions.h"
#include "components/bookmarks/browser/bookmark_model.h" #include "components/bookmarks/browser/bookmark_model.h"
#include "components/bookmarks/common/bookmark_pref_names.h"
#include "components/feature_engagement/public/feature_constants.h" #include "components/feature_engagement/public/feature_constants.h"
#include "components/feature_engagement/public/tracker.h" #include "components/feature_engagement/public/tracker.h"
#include "components/language/ios/browser/ios_language_detection_tab_helper.h" #include "components/language/ios/browser/ios_language_detection_tab_helper.h"
#import "components/language/ios/browser/ios_language_detection_tab_helper_observer_bridge.h" #import "components/language/ios/browser/ios_language_detection_tab_helper_observer_bridge.h"
#include "components/open_from_clipboard/clipboard_recent_content.h" #include "components/open_from_clipboard/clipboard_recent_content.h"
#import "components/prefs/ios/pref_observer_bridge.h"
#include "components/prefs/pref_change_registrar.h"
#include "components/prefs/pref_service.h"
#include "components/translate/core/browser/translate_manager.h" #include "components/translate/core/browser/translate_manager.h"
#include "components/translate/core/browser/translate_prefs.h" #include "components/translate/core/browser/translate_prefs.h"
#import "ios/chrome/browser/browser_state/chrome_browser_state.h" #import "ios/chrome/browser/browser_state/chrome_browser_state.h"
...@@ -22,6 +26,7 @@ ...@@ -22,6 +26,7 @@
#import "ios/chrome/browser/find_in_page/find_tab_helper.h" #import "ios/chrome/browser/find_in_page/find_tab_helper.h"
#import "ios/chrome/browser/overlays/public/overlay_presenter.h" #import "ios/chrome/browser/overlays/public/overlay_presenter.h"
#import "ios/chrome/browser/overlays/public/overlay_presenter_observer_bridge.h" #import "ios/chrome/browser/overlays/public/overlay_presenter_observer_bridge.h"
#include "ios/chrome/browser/policy/policy_features.h"
#import "ios/chrome/browser/search_engines/search_engines_util.h" #import "ios/chrome/browser/search_engines/search_engines_util.h"
#import "ios/chrome/browser/translate/chrome_ios_translate_client.h" #import "ios/chrome/browser/translate/chrome_ios_translate_client.h"
#import "ios/chrome/browser/ui/activity_services/canonical_url_retriever.h" #import "ios/chrome/browser/ui/activity_services/canonical_url_retriever.h"
...@@ -87,6 +92,7 @@ PopupMenuToolsItem* CreateTableViewItem(int titleID, ...@@ -87,6 +92,7 @@ PopupMenuToolsItem* CreateTableViewItem(int titleID,
CRWWebStateObserver, CRWWebStateObserver,
IOSLanguageDetectionTabHelperObserving, IOSLanguageDetectionTabHelperObserving,
OverlayPresenterObserving, OverlayPresenterObserving,
PrefObserverDelegate,
ReadingListMenuNotificationDelegate, ReadingListMenuNotificationDelegate,
WebStateListObserving> { WebStateListObserving> {
std::unique_ptr<web::WebStateObserverBridge> _webStateObserver; std::unique_ptr<web::WebStateObserverBridge> _webStateObserver;
...@@ -97,6 +103,10 @@ PopupMenuToolsItem* CreateTableViewItem(int titleID, ...@@ -97,6 +103,10 @@ PopupMenuToolsItem* CreateTableViewItem(int titleID,
std::unique_ptr<language::IOSLanguageDetectionTabHelperObserverBridge> std::unique_ptr<language::IOSLanguageDetectionTabHelperObserverBridge>
_iOSLanguageDetectionTabHelperObserverBridge; _iOSLanguageDetectionTabHelperObserverBridge;
std::unique_ptr<OverlayPresenterObserver> _overlayPresenterObserver; std::unique_ptr<OverlayPresenterObserver> _overlayPresenterObserver;
// Pref observer to track changes to prefs.
std::unique_ptr<PrefObserverBridge> _prefObserverBridge;
// Registrar for pref changes notifications.
std::unique_ptr<PrefChangeRegistrar> _prefChangeRegistrar;
} }
// Items to be displayed in the popup menu. // Items to be displayed in the popup menu.
...@@ -201,6 +211,9 @@ PopupMenuToolsItem* CreateTableViewItem(int titleID, ...@@ -201,6 +211,9 @@ PopupMenuToolsItem* CreateTableViewItem(int titleID,
_readingListMenuNotifier = nil; _readingListMenuNotifier = nil;
_bookmarkModelBridge.reset(); _bookmarkModelBridge.reset();
_iOSLanguageDetectionTabHelperObserverBridge.reset(); _iOSLanguageDetectionTabHelperObserverBridge.reset();
_prefChangeRegistrar.reset();
_prefObserverBridge.reset();
} }
#pragma mark - CRWWebStateObserver #pragma mark - CRWWebStateObserver
...@@ -309,6 +322,13 @@ PopupMenuToolsItem* CreateTableViewItem(int titleID, ...@@ -309,6 +322,13 @@ PopupMenuToolsItem* CreateTableViewItem(int titleID,
self.webContentAreaShowingOverlay = NO; self.webContentAreaShowingOverlay = NO;
} }
#pragma mark - PrefObserverDelegate
- (void)onPreferenceChanged:(const std::string&)preferenceName {
if (preferenceName == bookmarks::prefs::kEditBookmarksEnabled)
[self updateBookmarkItem];
}
#pragma mark - Properties #pragma mark - Properties
- (void)setWebState:(web::WebState*)webState { - (void)setWebState:(web::WebState*)webState {
...@@ -471,6 +491,15 @@ PopupMenuToolsItem* CreateTableViewItem(int titleID, ...@@ -471,6 +491,15 @@ PopupMenuToolsItem* CreateTableViewItem(int titleID,
[self updatePopupMenu]; [self updatePopupMenu];
} }
- (void)setPrefService:(PrefService*)prefService {
_prefService = prefService;
_prefChangeRegistrar = std::make_unique<PrefChangeRegistrar>();
_prefChangeRegistrar->Init(prefService);
_prefObserverBridge.reset(new PrefObserverBridge(self));
_prefObserverBridge->ObserveChangesForPreference(
bookmarks::prefs::kEditBookmarksEnabled, _prefChangeRegistrar.get());
}
#pragma mark - PopupMenuActionHandlerCommands #pragma mark - PopupMenuActionHandlerCommands
- (void)readPageLater { - (void)readPageLater {
...@@ -556,7 +585,8 @@ PopupMenuToolsItem* CreateTableViewItem(int titleID, ...@@ -556,7 +585,8 @@ PopupMenuToolsItem* CreateTableViewItem(int titleID,
if (!self.bookmarkItem) if (!self.bookmarkItem)
return; return;
self.bookmarkItem.enabled = [self isCurrentURLWebURL]; self.bookmarkItem.enabled =
[self isCurrentURLWebURL] && [self isEditBookmarksEnabled];
if (self.webState && self.bookmarkModel && if (self.webState && self.bookmarkModel &&
self.bookmarkModel->IsBookmarked(self.webState->GetVisibleURL())) { self.bookmarkModel->IsBookmarked(self.webState->GetVisibleURL())) {
...@@ -980,4 +1010,14 @@ PopupMenuToolsItem* CreateTableViewItem(int titleID, ...@@ -980,4 +1010,14 @@ PopupMenuToolsItem* CreateTableViewItem(int titleID,
return visibleItem->GetUserAgentType(self.webState->GetView()); return visibleItem->GetUserAgentType(self.webState->GetView());
} }
#pragma mark - Other private methods
// Returns YES if user is allowed to edit any bookmarks.
- (BOOL)isEditBookmarksEnabled {
if (IsEditBookmarksIOSEnabled())
return self.prefService->GetBoolean(
bookmarks::prefs::kEditBookmarksEnabled);
return YES;
}
@end @end
...@@ -4,12 +4,20 @@ ...@@ -4,12 +4,20 @@
#import "ios/chrome/browser/ui/popup_menu/popup_menu_mediator.h" #import "ios/chrome/browser/ui/popup_menu/popup_menu_mediator.h"
#include "base/strings/sys_string_conversions.h"
#include "base/test/scoped_feature_list.h" #include "base/test/scoped_feature_list.h"
#include "base/time/default_clock.h" #include "base/time/default_clock.h"
#include "components/bookmarks/browser/bookmark_model.h"
#include "components/bookmarks/browser/bookmark_utils.h"
#include "components/bookmarks/common/bookmark_pref_names.h"
#include "components/bookmarks/test/bookmark_test_helpers.h"
#include "components/feature_engagement/test/mock_tracker.h" #include "components/feature_engagement/test/mock_tracker.h"
#include "components/language/ios/browser/ios_language_detection_tab_helper.h" #include "components/language/ios/browser/ios_language_detection_tab_helper.h"
#include "components/prefs/pref_registry_simple.h"
#include "components/prefs/testing_pref_service.h"
#include "components/reading_list/core/reading_list_model_impl.h" #include "components/reading_list/core/reading_list_model_impl.h"
#include "components/translate/core/browser/translate_prefs.h" #include "components/translate/core/browser/translate_prefs.h"
#include "ios/chrome/browser/bookmarks/bookmark_model_factory.h"
#import "ios/chrome/browser/browser_state/test_chrome_browser_state.h" #import "ios/chrome/browser/browser_state/test_chrome_browser_state.h"
#import "ios/chrome/browser/main/test_browser.h" #import "ios/chrome/browser/main/test_browser.h"
#import "ios/chrome/browser/overlays/public/overlay_presenter.h" #import "ios/chrome/browser/overlays/public/overlay_presenter.h"
...@@ -17,6 +25,7 @@ ...@@ -17,6 +25,7 @@
#import "ios/chrome/browser/overlays/public/overlay_request_queue.h" #import "ios/chrome/browser/overlays/public/overlay_request_queue.h"
#import "ios/chrome/browser/overlays/public/web_content_area/java_script_alert_overlay.h" #import "ios/chrome/browser/overlays/public/web_content_area/java_script_alert_overlay.h"
#include "ios/chrome/browser/overlays/test/fake_overlay_presentation_context.h" #include "ios/chrome/browser/overlays/test/fake_overlay_presentation_context.h"
#include "ios/chrome/browser/policy/policy_features.h"
#import "ios/chrome/browser/ui/popup_menu/cells/popup_menu_tools_item.h" #import "ios/chrome/browser/ui/popup_menu/cells/popup_menu_tools_item.h"
#import "ios/chrome/browser/ui/popup_menu/popup_menu_constants.h" #import "ios/chrome/browser/ui/popup_menu/popup_menu_constants.h"
#import "ios/chrome/browser/ui/popup_menu/public/popup_menu_table_view_controller.h" #import "ios/chrome/browser/ui/popup_menu/public/popup_menu_table_view_controller.h"
...@@ -48,6 +57,8 @@ ...@@ -48,6 +57,8 @@
#error "This file requires ARC support." #error "This file requires ARC support."
#endif #endif
using bookmarks::BookmarkModel;
@interface FakePopupMenuConsumer : NSObject <PopupMenuConsumer> @interface FakePopupMenuConsumer : NSObject <PopupMenuConsumer>
@property(nonatomic, strong) @property(nonatomic, strong)
NSArray<NSArray<TableViewItem<PopupMenuItem>*>*>* popupMenuItems; NSArray<NSArray<TableViewItem<PopupMenuItem>*>*>* popupMenuItems;
...@@ -115,6 +126,21 @@ class PopupMenuMediatorTest : public ChromeWebTest { ...@@ -115,6 +126,21 @@ class PopupMenuMediatorTest : public ChromeWebTest {
return mediator_; return mediator_;
} }
void CreatePrefs() {
prefs_ = std::make_unique<TestingPrefServiceSimple>();
prefs_->registry()->RegisterBooleanPref(
bookmarks::prefs::kEditBookmarksEnabled,
/*default_value=*/true);
}
void SetUpBookmarks() {
browser_state_->CreateBookmarkModel(false);
bookmark_model_ =
ios::BookmarkModelFactory::GetForBrowserState(browser_state_.get());
bookmarks::test::WaitForBookmarkModelToLoad(bookmark_model_);
mediator_.bookmarkModel = bookmark_model_;
}
void SetUpWebStateList() { void SetUpWebStateList() {
auto navigation_manager = std::make_unique<ToolbarTestNavigationManager>(); auto navigation_manager = std::make_unique<ToolbarTestNavigationManager>();
navigation_manager_ = navigation_manager.get(); navigation_manager_ = navigation_manager.get();
...@@ -197,6 +223,8 @@ class PopupMenuMediatorTest : public ChromeWebTest { ...@@ -197,6 +223,8 @@ class PopupMenuMediatorTest : public ChromeWebTest {
std::unique_ptr<TestChromeBrowserState> browser_state_; std::unique_ptr<TestChromeBrowserState> browser_state_;
std::unique_ptr<Browser> browser_; std::unique_ptr<Browser> browser_;
PopupMenuMediator* mediator_; PopupMenuMediator* mediator_;
BookmarkModel* bookmark_model_;
std::unique_ptr<TestingPrefServiceSimple> prefs_;
std::unique_ptr<ReadingListModelImpl> reading_list_model_; std::unique_ptr<ReadingListModelImpl> reading_list_model_;
ToolbarTestWebState* web_state_; ToolbarTestWebState* web_state_;
ToolbarTestNavigationManager* navigation_manager_; ToolbarTestNavigationManager* navigation_manager_;
...@@ -414,3 +442,49 @@ TEST_F(PopupMenuMediatorTest, TestTextZoomDisabledIPad) { ...@@ -414,3 +442,49 @@ TEST_F(PopupMenuMediatorTest, TestTextZoomDisabledIPad) {
SetUpActiveWebState(); SetUpActiveWebState();
EXPECT_FALSE(HasItem(consumer, kToolsMenuTextZoom, /*enabled=*/YES)); EXPECT_FALSE(HasItem(consumer, kToolsMenuTextZoom, /*enabled=*/YES));
} }
// Tests that 1) the tools menu has an enabled 'Add to Bookmarks' button when
// the current URL is not in bookmarks 2) the bookmark button changes to an
// enabled 'Edit bookmark' button when navigating to a bookmarked URL, 3) the
// bookmark button changes to 'Add to Bookmarks' when the bookmark is removed.
TEST_F(PopupMenuMediatorTest, TestBookmarksToolsMenuButtons) {
const GURL url("https://bookmarked.url");
web_state_->SetCurrentURL(url);
CreateMediator(PopupMenuTypeToolsMenu, /*is_incognito=*/NO,
/*trigger_incognito_hint=*/NO);
SetUpBookmarks();
bookmarks::AddIfNotBookmarked(bookmark_model_, url,
base::SysNSStringToUTF16(@"Test bookmark"));
mediator_.webStateList = web_state_list_.get();
FakePopupMenuConsumer* consumer = [[FakePopupMenuConsumer alloc] init];
mediator_.popupMenu = consumer;
EXPECT_TRUE(HasItem(consumer, kToolsMenuAddToBookmarks, /*enabled=*/YES));
SetUpActiveWebState();
EXPECT_FALSE(HasItem(consumer, kToolsMenuAddToBookmarks, /*enabled=*/YES));
EXPECT_TRUE(HasItem(consumer, kToolsMenuEditBookmark, /*enabled=*/YES));
bookmark_model_->RemoveAllUserBookmarks();
EXPECT_TRUE(HasItem(consumer, kToolsMenuAddToBookmarks, /*enabled=*/YES));
EXPECT_FALSE(HasItem(consumer, kToolsMenuEditBookmark, /*enabled=*/YES));
}
// Tests that the bookmark button is disabled when EditBookmarksEnabled pref is
// changed to false.
TEST_F(PopupMenuMediatorTest, TestDisableBookmarksButton) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeature(kEditBookmarksIOS);
CreateMediator(PopupMenuTypeToolsMenu, /*is_incognito=*/NO,
/*trigger_incognito_hint=*/NO);
CreatePrefs();
FakePopupMenuConsumer* consumer = [[FakePopupMenuConsumer alloc] init];
mediator_.popupMenu = consumer;
mediator_.prefService = prefs_.get();
EXPECT_TRUE(HasItem(consumer, kToolsMenuAddToBookmarks, /*enabled=*/YES));
prefs_->SetBoolean(bookmarks::prefs::kEditBookmarksEnabled, false);
EXPECT_TRUE(HasItem(consumer, kToolsMenuAddToBookmarks, /*enabled=*/NO));
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment