Fixes issue in extending menu'

* Changed the way to pass DataSource from subclass. CreateDataSource was called in the constructor, so virtual function does not work. Moved to constructor argument.
* extra JS source has to be embedded.
* removed unnecessary padding-left

BUG=none
TEST=none

Review URL: http://codereview.chromium.org/3595019

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@62015 0039d316-1c4b-4281-b951-d872f2087c98

9 files changed, 104 insertions, 46 deletions

diff --git a/chrome/browser/browser_resources.grd b/chrome/browser/browser_resources.grd
index f2db76b..f3a1f53 100644
--- a/chrome/browser/browser_resources.grd
+++ b/chrome/browser/browser_resources.grd
@@ -78,6 +78,7 @@ without changes to the corresponding grd file. eter -->
         <include name="IDR_SLIDESHOW_HTML" file="resources\slideshow.html" flattenhtml="true" type="BINDATA" />
         <include name="IDR_TALK_APP_MANIFEST" file="resources\chat_manager\manifest.json" type="BINDATA" />
         <include name="IDR_MENU_HTML" file="resources\menu.html" flattenhtml="true" type="BINDATA" />
+        <include name="IDR_WRENCH_MENU_JS" file="resources\wrench_menu.js" flattenhtml="true" type="BINDATA" />
       </if>
     </includes>
   </release>
diff --git a/chrome/browser/chromeos/dom_ui/menu_ui.cc b/chrome/browser/chromeos/dom_ui/menu_ui.cc
index 24fc964..e6adb5d 100644
--- a/chrome/browser/chromeos/dom_ui/menu_ui.cc
+++ b/chrome/browser/chromeos/dom_ui/menu_ui.cc
@@ -38,6 +38,9 @@
 
 namespace {
 
+// a fake resource id for not loading extra resource.
+const int kNoExtraSource = -1;
+
 // A utility function that generates css font property from gfx::Font.
 std::wstring GetFontShorthand(const gfx::Font* font) {
   std::wstring out;
@@ -114,14 +117,14 @@ const std::string& GetImageDataUrlForRadio(bool on) {
 
 /**
  * Generates a html file that uses |menu_class| as a menu implementation.
- * |menu_source| specifies the source that contains the definition of the
+ * |menu_source_id| specifies the source that contains the definition of the
  * |menu_class|, or empty string to use plain "Menu".
  */
 std::string GetMenuUIHTMLSourceFromString(
     const chromeos::MenuUI& menu_ui,
     const base::StringPiece& menu_template,
     const std::string& menu_class,
-    const std::string& menu_source) {
+    int menu_source_id) {
 #define SET_INTEGER_PROPERTY(prop) \
   value_config.SetInteger(#prop, menu_config.prop)
 
@@ -168,11 +171,19 @@ std::string GetMenuUIHTMLSourceFromString(
   base::JSONWriter::Write(&value_config, false, &json_config);
 
   DictionaryValue strings;
+
   strings.SetString(
       "init_script",
       menu_class + ".decorate(document.getElementById('viewport'));" +
       " init(" + json_config + ");");
-  strings.SetString("menu_source", menu_source);
+
+  if (menu_source_id == kNoExtraSource) {
+    strings.SetString("menu_source", "");
+  } else {
+    base::StringPiece menu_source(
+        ResourceBundle::GetSharedInstance().GetRawDataResource(menu_source_id));
+    strings.SetString("menu_source", menu_source.as_string());
+  }
 
   return jstemplate_builder::GetI18nTemplateHtml(menu_template, &strings);
 }
@@ -181,9 +192,9 @@ class MenuUIHTMLSource : public ChromeURLDataManager::DataSource,
                          public URLFetcher::Delegate {
  public:
   MenuUIHTMLSource(const chromeos::MenuUI& menu_ui,
-                   Profile* profile,
+                   const std::string& source_name,
                    const std::string& menu_class,
-                   const std::string& menu_source);
+                   int menu_source_id);
 
   // Called when the network layer has requested a resource underneath
   // the path we registered.
@@ -211,11 +222,10 @@ class MenuUIHTMLSource : public ChromeURLDataManager::DataSource,
   // The name of JS Menu class to use.
   const std::string menu_class_;
 
-  // The source file of the menu subclass.
-  const std::string menu_source_;
+  // The resource id of the file of the menu subclass.
+  int menu_source_id_;
 #ifndef NDEBUG
   int request_id_;
-  Profile* profile_;
 #endif
   DISALLOW_COPY_AND_ASSIGN(MenuUIHTMLSource);
 };
@@ -289,18 +299,19 @@ class MenuHandler : public chromeos::MenuHandlerBase,
 ////////////////////////////////////////////////////////////////////////////////
 
 MenuUIHTMLSource::MenuUIHTMLSource(const chromeos::MenuUI& menu_ui,
-                                   Profile* profile,
+                                   const std::string& source_name,
                                    const std::string& menu_class,
-                                   const std::string& menu_source)
-    : DataSource(chrome::kChromeUIMenu, MessageLoop::current()),
+                                   int menu_source_id)
+    : DataSource(source_name, MessageLoop::current()),
       menu_ui_(menu_ui),
       menu_class_(menu_class),
-      menu_source_(menu_source)
+      menu_source_id_(menu_source_id)
 #ifndef NDEBUG
-    , request_id_(-1),
-      profile_(profile)
+    , request_id_(-1)
 #endif
-     {
+{
+  // Important !
+  // Don't call any method on menu_ui. MenuUI object is not yet initialized
 }
 
 void MenuUIHTMLSource::StartDataRequest(const std::string& path,
@@ -312,7 +323,7 @@ void MenuUIHTMLSource::StartDataRequest(const std::string& path,
   if (!url.empty()) {
     request_id_ = request_id;
     URLFetcher* fetcher = new URLFetcher(GURL(url), URLFetcher::GET, this);
-    fetcher->set_request_context(profile_->GetRequestContext());
+    fetcher->set_request_context(menu_ui_.GetProfile()->GetRequestContext());
     fetcher->Start();
     return;
   }
@@ -323,7 +334,7 @@ void MenuUIHTMLSource::StartDataRequest(const std::string& path,
   // The resource string should be pure code and should not contain
   // i18n string.
   const std::string menu_html = GetMenuUIHTMLSourceFromString(
-      menu_ui_, menu_template, menu_class_, menu_source_);
+      menu_ui_, menu_template, menu_class_, menu_source_id_);
 
   scoped_refptr<RefCountedBytes> html_bytes(new RefCountedBytes);
 
@@ -342,8 +353,8 @@ void MenuUIHTMLSource::OnURLFetchComplete(const URLFetcher* source,
                                           const std::string& data) {
 #ifndef NDEBUG
   // This should not be called in release build.
-  const std::string menu_html =
-      GetMenuUIHTMLSourceFromString(menu_ui_, data, menu_class_, menu_source_);
+  const std::string menu_html = GetMenuUIHTMLSourceFromString(
+      menu_ui_, data, menu_class_, menu_source_id_);
 
   scoped_refptr<RefCountedBytes> html_bytes(new RefCountedBytes);
 
@@ -559,6 +570,19 @@ MenuUI::MenuUI(TabContents* contents) : DOMUI(contents) {
           make_scoped_refptr(CreateDataSource())));
 }
 
+MenuUI::MenuUI(TabContents* contents, ChromeURLDataManager::DataSource* source)
+    : DOMUI(contents) {
+  MenuHandler* handler = new MenuHandler();
+  AddMessageHandler((handler)->Attach(this));
+
+  ChromeThread::PostTask(
+      ChromeThread::IO, FROM_HERE,
+      NewRunnableMethod(
+          Singleton<ChromeURLDataManager>::get(),
+          &ChromeURLDataManager::AddDataSource,
+          make_scoped_refptr(source)));
+}
+
 void MenuUI::ModelUpdated(const menus::MenuModel* model) {
   DictionaryValue json_model;
   ListValue* items = new ListValue();
@@ -631,17 +655,17 @@ DictionaryValue* MenuUI::CreateMenuItem(const menus::MenuModel* model,
 
 ChromeURLDataManager::DataSource* MenuUI::CreateDataSource() {
   return CreateMenuUIHTMLSource(*this,
-                                GetProfile(),
+                                chrome::kChromeUIMenu,
                                 "Menu" /* class name */,
-                                "" /* no extra source */);
+                                kNoExtraSource);
 }
 
 ChromeURLDataManager::DataSource* MenuUI::CreateMenuUIHTMLSource(
     const MenuUI& menu_ui,
-    Profile* profile,
+    const std::string& source_name,
     const std::string& menu_class,
-    const std::string& menu_source) {
-  return new MenuUIHTMLSource(menu_ui, profile, menu_class, menu_source);
+    int menu_source_id) {
+  return new MenuUIHTMLSource(menu_ui, source_name, menu_class, menu_source_id);
 }
 
 }  // namespace chromeos
diff --git a/chrome/browser/chromeos/dom_ui/menu_ui.h b/chrome/browser/chromeos/dom_ui/menu_ui.h
index ea505a9f..66cdf7f 100644
--- a/chrome/browser/chromeos/dom_ui/menu_ui.h
+++ b/chrome/browser/chromeos/dom_ui/menu_ui.h
@@ -38,19 +38,26 @@ class MenuUI : public DOMUI {
   // Subclass can add extra parameters or replaces default configuration.
   virtual void AddCustomConfigValues(DictionaryValue* config) const {};
 
-  // Create HTML Data source for the menu.  Extended menu
-  // implementation may provide its own menu implmentation.
-  virtual ChromeURLDataManager::DataSource* CreateDataSource();
-
-  // A utility function that create a concrete html file from
-  // template for given |menu_class|.
+  // A utility function which creates a concrete html file from
+  // template file |menu_resource_id| for given |menu_class|.
+  // The resource_name is the host part of DOMUI's url.
+  // Caution: This calls MenuUI::GetProfile() when creating the data source,
+  // thus, it has to be initialized.
   static ChromeURLDataManager::DataSource* CreateMenuUIHTMLSource(
       const MenuUI& menu_ui,
-      Profile* profile,
+      const std::string& source_name,
       const std::string& menu_class,
-      const std::string& menu_source);
+      int menu_source_res_id);
+
+ protected:
+  // A constructor for subclass to initialize the MenuUI with
+  // different data source.
+  MenuUI(TabContents* contents, ChromeURLDataManager::DataSource* source);
 
  private:
+  // Create HTML Data source for the menu.
+  ChromeURLDataManager::DataSource* CreateDataSource();
+
   DISALLOW_COPY_AND_ASSIGN(MenuUI);
 };
 
diff --git a/chrome/browser/chromeos/dom_ui/wrench_menu_ui.cc b/chrome/browser/chromeos/dom_ui/wrench_menu_ui.cc
index b7a849c..c79316b5 100644
--- a/chrome/browser/chromeos/dom_ui/wrench_menu_ui.cc
+++ b/chrome/browser/chromeos/dom_ui/wrench_menu_ui.cc
@@ -5,7 +5,13 @@
 #include "chrome/browser/chromeos/dom_ui/wrench_menu_ui.h"
 
 #include "base/values.h"
+#include "base/string_util.h"
 #include "chrome/app/chrome_dll_resource.h"
+#include "chrome/browser/chromeos/views/native_menu_domui.h"
+#include "chrome/common/url_constants.h"
+#include "googleurl/src/gurl.h"
+#include "grit/browser_resources.h"
+#include "views/controls/menu/menu_2.h"
 
 namespace chromeos {
 
@@ -15,7 +21,14 @@ namespace chromeos {
 //
 ////////////////////////////////////////////////////////////////////////////////
 
-WrenchMenuUI::WrenchMenuUI(TabContents* contents) : chromeos::MenuUI(contents) {
+WrenchMenuUI::WrenchMenuUI(TabContents* contents)
+    : chromeos::MenuUI(
+        contents,
+        ALLOW_THIS_IN_INITIALIZER_LIST(
+            CreateMenuUIHTMLSource(*this,
+                                   chrome::kChromeUIWrenchMenu,
+                                   "Menu",
+                                   IDR_WRENCH_MENU_JS))) {
 }
 
 void WrenchMenuUI::AddCustomConfigValues(DictionaryValue* config) const {
@@ -24,9 +37,11 @@ void WrenchMenuUI::AddCustomConfigValues(DictionaryValue* config) const {
   config->SetInteger("IDC_ZOOM_MINUS", IDC_ZOOM_MINUS);
 }
 
-ChromeURLDataManager::DataSource* WrenchMenuUI::CreateDataSource() {
-  return CreateMenuUIHTMLSource(*this, GetProfile(),
-                                "WrenchMenu", "wrench_menu.js");
+views::Menu2* WrenchMenuUI::CreateMenu2(menus::MenuModel* model) {
+  views::Menu2* menu = new views::Menu2(model);
+  NativeMenuDOMUI::SetMenuURL(
+      menu, GURL(StringPrintf("chrome://%s", chrome::kChromeUIWrenchMenu)));
+  return menu;
 }
 
 }  // namespace chromeos
diff --git a/chrome/browser/chromeos/dom_ui/wrench_menu_ui.h b/chrome/browser/chromeos/dom_ui/wrench_menu_ui.h
index c3f053b..04fd44e 100644
--- a/chrome/browser/chromeos/dom_ui/wrench_menu_ui.h
+++ b/chrome/browser/chromeos/dom_ui/wrench_menu_ui.h
@@ -8,6 +8,14 @@
 
 #include "chrome/browser/chromeos/dom_ui/menu_ui.h"
 
+namespace views {
+class Menu2;
+}  // namespace views
+
+namespace menus {
+class MenuModel;
+} // namespace menus
+
 namespace chromeos {
 
 class WrenchMenuUI : public MenuUI {
@@ -17,9 +25,8 @@ class WrenchMenuUI : public MenuUI {
   // MenuUI overrides:
   virtual void AddCustomConfigValues(DictionaryValue* config) const;
 
-  // Create HTML Data source for the menu.  Extended menu
-  // implementation may provide its own menu implmentation.
-  virtual ChromeURLDataManager::DataSource* CreateDataSource();
+  // A convenient factory method to create Menu2 for wrench menu.
+  static views::Menu2* CreateMenu2(menus::MenuModel* model);
 
  private:
   DISALLOW_COPY_AND_ASSIGN(WrenchMenuUI);
diff --git a/chrome/browser/resources/menu.css b/chrome/browser/resources/menu.css
index 1396017..a279618 100644
--- a/chrome/browser/resources/menu.css
+++ b/chrome/browser/resources/menu.css
@@ -11,7 +11,6 @@ body {
   white-space: nowrap;
   margin: 0;
   padding-top: 0;
-  padding-right: 19px;
   padding-bottom: 0;
   background-repeat: no-repeat;
   -webkit-padding-end: 19px;
diff --git a/chrome/browser/resources/menu.html b/chrome/browser/resources/menu.html
index 8380577..f35e2fd 100644
--- a/chrome/browser/resources/menu.html
+++ b/chrome/browser/resources/menu.html
@@ -6,7 +6,7 @@
     <script src="shared/js/cr.js"></script>
     <script src="shared/js/cr/ui.js"></script>
     <script src="menu.js"></script>
-    <script i18n-values=".src:menu_source"></script>
+    <script i18n-content="menu_source"></script>
   </head>
   <body>
     <div id="scroll-up" class="scroll-button"></div>
diff --git a/chrome/browser/resources/menu.js b/chrome/browser/resources/menu.js
index 7cf5f57..66d205e 100644
--- a/chrome/browser/resources/menu.js
+++ b/chrome/browser/resources/menu.js
@@ -38,10 +38,10 @@ MenuItem.prototype = {
   /**
    * Initialize the MenuItem.
    * @param {Menu} menu A {@code Menu} object to which this menu item
-   *   will be added to.
+   *    will be added to.
    * @param {Object} attrs JSON object that represents this menu items
-   *   properties.  This is created from menu model in C code.  See
-   *   chromeos/views/native_menu_domui.cc.
+   *    properties.  This is created from menu model in C code.  See
+   *    chromeos/views/native_menu_domui.cc.
    * @param {number} leftIconWidth The left icon's width. 0 if no icon.
    */
   init: function(menu, attrs, leftIconWidth) {
@@ -127,7 +127,7 @@ MenuItem.prototype = {
         url = this.menu_.config_.checkUrl;
       }
       if (url) {
-        this.style.backgroundImage = "url(" + url + ")";
+        this.style.backgroundImage = 'url(' + url + ')';
       }
       // TODO(oshima): figure out how to update left padding in rule.
       // 4 is the padding on left side of icon.
@@ -158,7 +158,7 @@ MenuItem.prototype = {
       // This overrides left-icon's position, but it's OK as submenu
       // shoudln't have left-icon.
       this.classList.add('right-icon');
-      this.style.backgroundImage = "url(" + this.menu_.config_.arrowUrl + ")";
+      this.style.backgroundImage = 'url(' + this.menu_.config_.arrowUrl + ')';
     }
   },
 };
diff --git a/chrome/browser/resources/wrench_menu.js b/chrome/browser/resources/wrench_menu.js
new file mode 100644
index 0000000..c45e432
--- /dev/null
+++ b/chrome/browser/resources/wrench_menu.js
@@ -0,0 +1,5 @@
+// Copyright (c) 2010 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.
+
+// T.B.D.