Simplify WebUI data sources. Currently WebUI data sources implement a URLDataSourceDelegate interfa…

chrome/browser/ui/webui/about_ui.cc

Index: chrome/browser/ui/webui/about_ui.cc
===================================================================
--- chrome/browser/ui/webui/about_ui.cc	(revision 176942)
+++ chrome/browser/ui/webui/about_ui.cc	(working copy)
@@ -37,7 +37,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/ui/browser_dialogs.h"
-#include "chrome/browser/ui/webui/chrome_url_data_manager.h"
+#include "chrome/browser/ui/webui/web_ui_util.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/jstemplate_builder.h"
 #include "chrome/common/render_messages.h"
@@ -116,9 +116,9 @@
 class AboutMemoryHandler : public MemoryDetails {
  public:
   AboutMemoryHandler(const base::WeakPtr<AboutUIHTMLSource>& source,
-                     int request_id)
+                     const content::URLDataSource::GotDataCallback& callback)
       : source_(source),
-        request_id_(request_id) {
+        callback_(callback) {
   }
 
   virtual void OnDetailsAvailable() OVERRIDE;
@@ -131,7 +131,7 @@
   void AppendProcess(ListValue* child_data, ProcessMemoryInformation* info);
 
   base::WeakPtr<AboutUIHTMLSource> source_;
-  int request_id_;
+  const content::URLDataSource::GotDataCallback& callback_;
 
   DISALLOW_COPY_AND_ASSIGN(AboutMemoryHandler);
 };
@@ -142,9 +142,9 @@
  public:
   static void Start(const base::WeakPtr<AboutUIHTMLSource>& source,
                     const std::string& path,
-                    int request_id) {
+                    const content::URLDataSource::GotDataCallback& callback) {
     scoped_refptr<ChromeOSTermsHandler> handler(
-        new ChromeOSTermsHandler(source, path, request_id));
+        new ChromeOSTermsHandler(source, path, callback));
     handler->StartOnUIThread();
   }
 
@@ -153,10 +153,10 @@
 
   ChromeOSTermsHandler(const base::WeakPtr<AboutUIHTMLSource>& source,
                        const std::string& path,
-                       int request_id)
+                       const content::URLDataSource::GotDataCallback& callback)
     : source_(source),
       path_(path),
-      request_id_(request_id),
+      callback_(callback),
       // Previously we were using "initial locale" http://crbug.com/145142
       locale_(g_browser_process->GetApplicationLocale()) {
   }
@@ -211,7 +211,7 @@
     // Do nothing if OEM EULA load failed.
     if (contents_.empty() && path_ != chrome::kOemEulaURLPath)
       contents_ = l10n_util::GetStringUTF8(IDS_TERMS_HTML);
-    source_->FinishDataRequest(contents_, request_id_);
+    source_->FinishDataRequest(contents_, callback_);
   }
 
   // Where the results are fed to.
@@ -220,8 +220,8 @@
   // Path in the URL.
   std::string path_;
 
-  // ID identifying the request.
-  int request_id_;
+  // Callback to run with the response.
+  const content::URLDataSource::GotDataCallback& callback_;
 
   // Locale of the EULA.
   std::string locale_;
@@ -504,9 +504,9 @@
 class AboutDnsHandler : public base::RefCountedThreadSafe<AboutDnsHandler> {
  public:
   static void Start(const base::WeakPtr<AboutUIHTMLSource>& source,
-                    int request_id) {
+                    const content::URLDataSource::GotDataCallback& callback) {
     scoped_refptr<AboutDnsHandler> handler(
-        new AboutDnsHandler(source, request_id));
+        new AboutDnsHandler(source, callback));
     handler->StartOnUIThread();
   }
 
@@ -514,9 +514,9 @@
   friend class base::RefCountedThreadSafe<AboutDnsHandler>;
 
   AboutDnsHandler(const base::WeakPtr<AboutUIHTMLSource>& source,
-                  int request_id)
+                  const content::URLDataSource::GotDataCallback& callback)
       : source_(source),
-        request_id_(request_id) {
+        callback_(callback) {
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   }
 
@@ -549,33 +549,34 @@
   void FinishOnUIThread(const std::string& data) {
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
     if (source_)
-      source_->FinishDataRequest(data, request_id_);
+      source_->FinishDataRequest(data, callback_);
   }
 
   // Where the results are fed to.
   base::WeakPtr<AboutUIHTMLSource> source_;
 
-  // ID identifying the request.
-  int request_id_;
+  // Callback to run with the response.
+  const content::URLDataSource::GotDataCallback& callback_;
 
   DISALLOW_COPY_AND_ASSIGN(AboutDnsHandler);
 };
 
-void FinishMemoryDataRequest(const std::string& path,
-                             AboutUIHTMLSource* source,
-                             int request_id) {
+void FinishMemoryDataRequest(
+    const std::string& path,
+    AboutUIHTMLSource* source,
+    const content::URLDataSource::GotDataCallback& callback) {
   if (path == kStringsJsPath) {
     // The AboutMemoryHandler cleans itself up, but |StartFetch()| will want
     // the refcount to be greater than 0.
     scoped_refptr<AboutMemoryHandler>
-        handler(new AboutMemoryHandler(source->AsWeakPtr(), request_id));
+        handler(new AboutMemoryHandler(source->AsWeakPtr(), callback));
     // TODO(jamescook): Maybe this shouldn't update UMA?
     handler->StartFetch(MemoryDetails::UPDATE_USER_METRICS);
   } else {
     source->FinishDataRequest(
         ResourceBundle::GetSharedInstance().GetRawDataResource(
             path == kMemoryJsPath ? IDR_ABOUT_MEMORY_JS :
-            IDR_ABOUT_MEMORY_HTML).as_string(), request_id);
+            IDR_ABOUT_MEMORY_HTML).as_string(), callback);
   }
 }
 
@@ -947,13 +948,13 @@
   load_time_data.SetString(
       "summary_desc",
       l10n_util::GetStringUTF16(IDS_MEMORY_USAGE_SUMMARY_DESC));
-  URLDataSource::SetFontAndTextDirection(&load_time_data);
+  web_ui_util::SetFontAndTextDirection(&load_time_data);
   load_time_data.Set("jstemplateData", root.release());
 
   jstemplate_builder::UseVersion2 version2;
   std::string data;
   jstemplate_builder::AppendJsonJS(&load_time_data, &data);
-  source_->FinishDataRequest(data, request_id_);
+  source_->FinishDataRequest(data, callback_);
 }
 
 }  // namespace
@@ -971,9 +972,10 @@
   return source_name_;
 }
 
-void AboutUIHTMLSource::StartDataRequest(const std::string& path,
-                                         bool is_incognito,
-                                         int request_id) {
+void AboutUIHTMLSource::StartDataRequest(
+    const std::string& path,
+    bool is_incognito,
+    const content::URLDataSource::GotDataCallback& callback) {
   std::string response;
   // Add your data source here, in alphabetical order.
   if (source_name_ == chrome::kChromeUIChromeURLsHost) {
@@ -991,7 +993,7 @@
     response = AboutTransparency(path);
 #endif
   } else if (source_name_ == chrome::kChromeUIDNSHost) {
-    AboutDnsHandler::Start(AsWeakPtr(), request_id);
+    AboutDnsHandler::Start(AsWeakPtr(), callback);
     return;
 #if defined(OS_LINUX) || defined(OS_OPENBSD)
   } else if (source_name_ == chrome::kChromeUILinuxProxyConfigHost) {
@@ -1000,7 +1002,7 @@
   } else if (source_name_ == chrome::kChromeUIMemoryHost) {
     response = GetAboutMemoryRedirectResponse(profile());
   } else if (source_name_ == chrome::kChromeUIMemoryRedirectHost) {
-    FinishMemoryDataRequest(path, this, request_id);
+    FinishMemoryDataRequest(path, this, callback);
     return;
 #if defined(OS_CHROMEOS)
   } else if (source_name_ == chrome::kChromeUINetworkHost) {
@@ -1017,21 +1019,21 @@
     response = AboutStats(path);
   } else if (source_name_ == chrome::kChromeUITermsHost) {
 #if defined(OS_CHROMEOS)
-    ChromeOSTermsHandler::Start(AsWeakPtr(), path, request_id);
+    ChromeOSTermsHandler::Start(AsWeakPtr(), path, callback);
     return;
 #else
     response = l10n_util::GetStringUTF8(IDS_TERMS_HTML);
 #endif
   }
 
-  FinishDataRequest(response, request_id);
+  FinishDataRequest(response, callback);
 }
 
-void AboutUIHTMLSource::FinishDataRequest(const std::string& html,
-                                          int request_id) {
+void AboutUIHTMLSource::FinishDataRequest(
+    const std::string& html,
+    const content::URLDataSource::GotDataCallback& callback) {
   std::string html_copy(html);
-  url_data_source()->SendResponse(
-      request_id, base::RefCountedString::TakeString(&html_copy));
+  callback.Run(base::RefCountedString::TakeString(&html_copy));
 }
 
 std::string AboutUIHTMLSource::GetMimeType(const std::string& path) const {
@@ -1054,6 +1056,6 @@
   ChromeURLDataManager::AddDataSource(profile, theme);
 #endif
 
-  content::URLDataSourceDelegate* source = new AboutUIHTMLSource(name, profile);
+  AboutUIHTMLSource* source = new AboutUIHTMLSource(name, profile);
   ChromeURLDataManager::AddDataSource(profile, source);

[Evan Stade, 2013/01/15 20:49:34]

combine these lines?

[jam, 2013/01/15 21:09:08]

(just tried it) that would take two lines

[Evan Stade, 2013/01/15 21:33:43]

yea, but I still think it's better. For one thing, you wouldn't have had to make
this change at all if it were already inline.

[jam, 2013/01/15 21:39:47]

Done. btw this is my style preference as well; but I've been resisting changing
all the callsites as I go through the code :)

 }