Index: chrome/browser/extensions/extension_context_menu_model_unittest.cc |
diff --git a/chrome/browser/extensions/extension_context_menu_model_unittest.cc b/chrome/browser/extensions/extension_context_menu_model_unittest.cc |
index f02d6e3390af570bd1ba1a856140fd1e659d1253..b0f40778d131d43971c504a14307bc1a52adb0ea 100644 |
--- a/chrome/browser/extensions/extension_context_menu_model_unittest.cc |
+++ b/chrome/browser/extensions/extension_context_menu_model_unittest.cc |
@@ -4,8 +4,11 @@ |
#include "chrome/browser/extensions/extension_context_menu_model.h" |
+#include "chrome/browser/extensions/context_menu_matcher.h" |
#include "chrome/browser/extensions/extension_service.h" |
#include "chrome/browser/extensions/extension_service_test_base.h" |
+#include "chrome/browser/extensions/menu_manager.h" |
+#include "chrome/browser/extensions/menu_manager_factory.h" |
#include "chrome/browser/ui/browser.h" |
#include "chrome/browser/ui/host_desktop.h" |
#include "chrome/test/base/test_browser_window.h" |
@@ -17,25 +20,64 @@ |
#include "testing/gtest/include/gtest/gtest.h" |
namespace extensions { |
-namespace { |
+namespace api { |
+namespace context_menus { |
+ |
+extern const int ACTION_MENU_TOP_LEVEL_LIMIT; |
Devlin
2014/07/24 21:51:28
why not just include the .h?
gpdavis
2014/07/28 21:08:22
Done.
|
+ |
+} // namespace context_menus |
+} // namespace api |
class ExtensionContextMenuModelTest : public ExtensionServiceTestBase { |
+ public: |
+ // Build an extension to pass to the menu constructor. It needs an |
+ // ExtensionAction. |
+ scoped_refptr<Extension> BuildExtension(std::string action_type) { |
Devlin
2014/07/24 21:51:27
If you take my suggestion on line 80, this should
Devlin
2014/07/24 21:51:27
even in tests, we *should* have the practice of no
gpdavis
2014/07/28 21:08:22
Sure thing! I wasn't sure if this was still proto
gpdavis
2014/07/28 21:08:22
Done (parameter change).
Yoyo Zhou
2014/07/28 23:23:25
You don't need a separate header file unless you n
|
+ return ExtensionBuilder() |
+ .SetManifest( |
+ DictionaryBuilder() |
+ .Set("name", "Page Action Extension") |
+ .Set("version", "1") |
+ .Set("manifest_version", 2) |
+ .Set(action_type, |
+ DictionaryBuilder().Set("default_title", "Hello"))) |
+ .Build(); |
+ } |
+ |
+ // Creates an extension menu item for |extension| with the given |context| |
+ // and adds it to |manager|. |
+ void AddContextItem(MenuManager* manager, |
+ Extension* extension, |
+ MenuItem::Context context) { |
+ MenuItem::Type type = MenuItem::NORMAL; |
+ MenuItem::ContextList contexts(context); |
+ const MenuItem::ExtensionKey key(extension->id()); |
+ MenuItem::Id id(false, key); |
+ id.uid = ++cur_id; |
+ manager->AddContextItem( |
+ extension, new MenuItem(id, "test", false, true, type, contexts)); |
Devlin
2014/07/24 21:51:28
doc anon bools
gpdavis
2014/07/28 21:08:22
Done.
|
+ } |
+ |
+ // Reinitializes the given |model|. |
+ void RefreshMenu(ExtensionContextMenuModel* model) { |
+ model->InitMenu(model->GetExtension()); |
+ } |
+ |
+ // Returns the number of extension menu items that show up in |model|. |
+ int CountExtensionItems(ExtensionContextMenuModel* model) { |
+ return model->extension_items_count_; |
+ } |
+ |
+ int cur_id = 0; |
Devlin
2014/07/24 21:51:28
Similarly, we should not have public class variabl
gpdavis
2014/07/28 21:08:22
Done.
|
}; |
+namespace { |
+ |
// Tests that applicable menu items are disabled when a ManagementPolicy |
// prohibits them. |
TEST_F(ExtensionContextMenuModelTest, PolicyDisablesItems) { |
InitializeEmptyExtensionService(); |
- // Build an extension to pass to the menu constructor. It needs an |
- // ExtensionAction. |
- scoped_refptr<Extension> extension = ExtensionBuilder() |
- .SetManifest(DictionaryBuilder() |
- .Set("name", "Page Action Extension") |
- .Set("version", "1") |
- .Set("manifest_version", 2) |
- .Set("page_action", DictionaryBuilder() |
- .Set("default_title", "Hello"))) |
- .Build(); |
+ scoped_refptr<Extension> extension = BuildExtension("page_action"); |
Devlin
2014/07/24 21:51:28
Why don't we define these as const char[]s kPageAc
gpdavis
2014/07/28 21:08:22
Done. I left out kBrowserAction for now, since it
|
ASSERT_TRUE(extension.get()); |
service_->AddExtension(extension.get()); |
@@ -66,5 +108,65 @@ TEST_F(ExtensionContextMenuModelTest, PolicyDisablesItems) { |
system->management_policy()->UnregisterAllProviders(); |
} |
+TEST_F(ExtensionContextMenuModelTest, ExtensionItemTest) { |
+ InitializeEmptyExtensionService(); |
+ scoped_refptr<Extension> extension = BuildExtension("page_action"); |
+ ASSERT_TRUE(extension.get()); |
+ service_->AddExtension(extension.get()); |
+ |
+ // Create a Browser for the ExtensionContextMenuModel to use. |
+ Browser::CreateParams params(profile_.get(), chrome::GetActiveDesktop()); |
+ TestBrowserWindow test_window; |
+ params.window = &test_window; |
+ Browser browser(params); |
+ |
+ // Create a MenuManager for adding context items. |
+ MenuManager* manager = static_cast<MenuManager*>( |
Yoyo Zhou
2014/07/24 22:21:03
What about creating a MenuManager without using th
gpdavis
2014/07/28 21:08:22
MenuManager::Get(profile_.get()) returns null if I
|
+ (MenuManagerFactory::GetInstance()->SetTestingFactoryAndUse( |
+ profile_.get(), &MenuManagerFactory::BuildInstanceFor))); |
+ ASSERT_TRUE(manager); |
+ |
+ scoped_refptr<ExtensionContextMenuModel> menu( |
+ new ExtensionContextMenuModel(extension.get(), &browser, NULL)); |
Devlin
2014/07/24 21:51:28
doc NULL
gpdavis
2014/07/28 21:08:22
I can actually just use the other constructor, whi
|
+ |
+ // There should be no extension items yet. |
+ ASSERT_TRUE(CountExtensionItems(menu) == 0); |
Devlin
2014/07/24 21:51:28
Some of these can be EXPECTs, and you should also
gpdavis
2014/07/28 21:08:22
Done.
|
+ |
+ // Add a browser action menu item for |extension| to |manager|. |
+ AddContextItem(manager, extension.get(), MenuItem::BROWSER_ACTION); |
+ RefreshMenu(menu); |
+ |
+ // Since |extension| has a page action, the browser action menu item should |
+ // not be present. |
+ ASSERT_TRUE(CountExtensionItems(menu) == 0); |
+ |
+ // Add a page action menu item and reset the context menu. |
+ AddContextItem(manager, extension.get(), MenuItem::PAGE_ACTION); |
+ RefreshMenu(menu); |
+ |
+ // The page action item should be present because |extension| has a page |
+ // action. |
+ ASSERT_TRUE(CountExtensionItems(menu) == 1); |
+ |
+ // Create more page action items to test top level menu item limitations. |
+ for (int i = 0; i < api::context_menus::ACTION_MENU_TOP_LEVEL_LIMIT; ++i) { |
Devlin
2014/07/24 21:51:28
nit: no brackets.
gpdavis
2014/07/28 21:08:22
Done.
|
+ AddContextItem(manager, extension.get(), MenuItem::PAGE_ACTION); |
+ } |
+ RefreshMenu(menu); |
+ |
+ // The menu should only have a limited number of extension items, since they |
+ // are all top level items, and we limit the number of top level extension |
+ // items. |
+ ASSERT_TRUE(CountExtensionItems(menu) == |
+ api::context_menus::ACTION_MENU_TOP_LEVEL_LIMIT); |
+ |
+ AddContextItem(manager, extension.get(), MenuItem::PAGE_ACTION); |
+ RefreshMenu(menu); |
+ |
+ // Adding another top level item should not increase the count. |
+ ASSERT_TRUE(CountExtensionItems(menu) == |
+ api::context_menus::ACTION_MENU_TOP_LEVEL_LIMIT); |
+} |
+ |
} // namespace |
} // namespace extensions |