Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(382)

Unified Diff: chrome/browser/app_menu_model.cc

Issue 523147: Made MenuController handle dynamic labels. (Closed)
Patch Set: Addressed pinkerton's comments. Created 10 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « chrome/browser/app_menu_model.h ('k') | chrome/browser/cocoa/menu_controller.mm » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/app_menu_model.cc
diff --git a/chrome/browser/app_menu_model.cc b/chrome/browser/app_menu_model.cc
index 38aca1511d404f4287ddcfa9e32c60e2c1bd1e77..f42c92089d1d2f21705220990a17c60335bd90ac 100644
--- a/chrome/browser/app_menu_model.cc
+++ b/chrome/browser/app_menu_model.cc
@@ -15,16 +15,33 @@
#include "grit/chromium_strings.h"
#include "grit/generated_resources.h"
+// TODO(akalin): Now that AppMenuModel handles the sync item
+// dynamically, we don't need to refresh the menu on Windows/Linux.
+// Remove that code and make sure it works.
+
AppMenuModel::AppMenuModel(menus::SimpleMenuModel::Delegate* delegate,
Browser* browser)
: menus::SimpleMenuModel(delegate),
- browser_(browser) {
+ browser_(browser),
+ // For now, we assume that sync cannot be enabled/disabled after
+ // launch.
+ sync_item_enabled_(ProfileSyncService::IsSyncEnabled()),
+ sync_item_index_(-1) {
Build();
}
AppMenuModel::~AppMenuModel() {
}
+bool AppMenuModel::IsLabelDynamicAt(int index) const {
+ return IsSyncItem(index) || SimpleMenuModel::IsLabelDynamicAt(index);
+}
+
+string16 AppMenuModel::GetLabelAt(int index) const {
+ return IsSyncItem(index) ?
+ GetSyncMenuLabel() : SimpleMenuModel::GetLabelAt(index);
+}
+
void AppMenuModel::Build() {
AddItemWithStringId(IDC_NEW_TAB, IDS_NEW_TAB);
AddItemWithStringId(IDC_NEW_WINDOW, IDS_NEW_WINDOW);
@@ -56,10 +73,11 @@ void AppMenuModel::Build() {
AddItemWithStringId(IDC_MANAGE_EXTENSIONS, IDS_SHOW_EXTENSIONS);
AddSeparator();
- if (ProfileSyncService::IsSyncEnabled()) {
- string16 label = sync_ui_util::GetSyncMenuLabel(
- browser_->profile()->GetOriginalProfile()->GetProfileSyncService());
- AddItem(IDC_SYNC_BOOKMARKS, label);
+ if (sync_item_enabled_) {
+ AddItem(IDC_SYNC_BOOKMARKS, GetSyncMenuLabel());
+ // TODO(akalin): Make it possible to get the index in a less
+ // hackish way.
+ sync_item_index_ = GetItemCount() - 1;
AddSeparator();
}
#if defined(OS_MACOSX)
@@ -109,3 +127,14 @@ void AppMenuModel::BuildProfileSubMenu() {
IDC_NEW_PROFILE,
IDS_SELECT_PROFILE_DIALOG_NEW_PROFILE_ENTRY);
}
+
+string16 AppMenuModel::GetSyncMenuLabel() const {
+ DCHECK(sync_item_enabled_);
+ return sync_ui_util::GetSyncMenuLabel(
+ browser_->profile()->GetOriginalProfile()->GetProfileSyncService());
+}
+
+bool AppMenuModel::IsSyncItem(int index) const {
+ return sync_item_enabled_ && (index == sync_item_index_);
+}
+
« no previous file with comments | « chrome/browser/app_menu_model.h ('k') | chrome/browser/cocoa/menu_controller.mm » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698