C++ readability review

Original CL: https://chromiumcodereview.appspot.com/9253024/

BUG=none
TEST=CWSIntentsRegistryTest.*

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

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

5 files changed, 58 insertions, 36 deletions

diff --git a/chrome/browser/intents/cws_intents_registry.cc b/chrome/browser/intents/cws_intents_registry.cc
index 99dbbc9f..58d4c35 100644
--- a/chrome/browser/intents/cws_intents_registry.cc
+++ b/chrome/browser/intents/cws_intents_registry.cc
@@ -29,13 +29,22 @@ const char kCWSIntentServiceURL[] =
 
 // Internal object representing all data associated with a single query.
 struct CWSIntentsRegistry::IntentsQuery {
+  IntentsQuery();
+  ~IntentsQuery();
+
   // Underlying URL request query.
-  scoped_ptr<content::URLFetcher> url_fetcher_;
+  scoped_ptr<content::URLFetcher> url_fetcher;
 
   // The callback - invoked on completed retrieval.
-  ResultsCallback callback_;
+  ResultsCallback callback;
 };
 
+CWSIntentsRegistry::IntentsQuery::IntentsQuery() {
+}
+
+CWSIntentsRegistry::IntentsQuery::~IntentsQuery() {
+}
+
 CWSIntentsRegistry::IntentExtensionInfo::IntentExtensionInfo()
     : num_ratings(0),
       average_rating(0) {
@@ -60,7 +69,7 @@ void CWSIntentsRegistry::OnURLFetchComplete(const content::URLFetcher* source) {
   QueryMap::iterator it = queries_.find(handle);
   DCHECK(it != queries_.end());
   scoped_ptr<IntentsQuery> query(it->second);
-  DCHECK(query != NULL);
+  DCHECK(query.get() != NULL);
   queries_.erase(it);
 
   std::string response;
@@ -74,7 +83,7 @@ void CWSIntentsRegistry::OnURLFetchComplete(const content::URLFetcher* source) {
   scoped_ptr<Value> parsed_response;
   JSONStringValueSerializer serializer(response);
   parsed_response.reset(serializer.Deserialize(NULL, &error));
-  if (parsed_response == NULL)
+  if (parsed_response.get() == NULL)
     return;
 
   DictionaryValue* response_dict;
@@ -82,7 +91,7 @@ void CWSIntentsRegistry::OnURLFetchComplete(const content::URLFetcher* source) {
   if (!response_dict)
     return;
   ListValue* items;
-  if (!response_dict->GetList("items",&items))
+  if (!response_dict->GetList("items", &items))
     return;
 
   IntentExtensionList intents;
@@ -108,7 +117,7 @@ void CWSIntentsRegistry::OnURLFetchComplete(const content::URLFetcher* source) {
     JSONStringValueSerializer manifest_serializer(manifest_utf8);
     scoped_ptr<Value> manifest_value;
     manifest_value.reset(manifest_serializer.Deserialize(NULL, &error));
-    if (manifest_value == NULL)
+    if (manifest_value.get() == NULL)
       continue;
 
     DictionaryValue* manifest_dict;
@@ -124,29 +133,29 @@ void CWSIntentsRegistry::OnURLFetchComplete(const content::URLFetcher* source) {
     intents.push_back(info);
   }
 
-  if (!query->callback_.is_null())
-    query->callback_.Run(intents);
+  if (!query->callback.is_null())
+    query->callback.Run(intents);
 }
 
-void CWSIntentsRegistry::GetIntentServices(
-    const string16& action, const string16& mimetype,
-    const ResultsCallback& cb) {
+void CWSIntentsRegistry::GetIntentServices(const string16& action,
+                                           const string16& mimetype,
+                                           const ResultsCallback& cb) {
   scoped_ptr<IntentsQuery> query(new IntentsQuery);
-  query->callback_ = cb;
-  query->url_fetcher_.reset(content::URLFetcher::Create(
+  query->callback = cb;
+  query->url_fetcher.reset(content::URLFetcher::Create(
       0, BuildQueryURL(action,mimetype), content::URLFetcher::GET, this));
 
-  if (query->url_fetcher_ == NULL)
+  if (query->url_fetcher.get() == NULL)
     return;
 
-  query->url_fetcher_->SetRequestContext(request_context_);
-  query->url_fetcher_->SetLoadFlags(
+  query->url_fetcher->SetRequestContext(request_context_);
+  query->url_fetcher->SetLoadFlags(
       net::LOAD_DO_NOT_SEND_COOKIES | net::LOAD_DO_NOT_SAVE_COOKIES);
 
   URLFetcherHandle handle = reinterpret_cast<URLFetcherHandle>(
-      query->url_fetcher_.get());
+      query->url_fetcher.get());
   queries_[handle] = query.release();
-  queries_[handle]->url_fetcher_->Start();
+  queries_[handle]->url_fetcher->Start();
 }
 
 // static
diff --git a/chrome/browser/intents/cws_intents_registry.h b/chrome/browser/intents/cws_intents_registry.h
index 1c5f541..ec01eb7 100644
--- a/chrome/browser/intents/cws_intents_registry.h
+++ b/chrome/browser/intents/cws_intents_registry.h
@@ -60,21 +60,21 @@ class CWSIntentsRegistry : public ProfileKeyedService,
   FRIEND_TEST_ALL_PREFIXES(CWSIntentsRegistryTest, ValidQuery);
   FRIEND_TEST_ALL_PREFIXES(CWSIntentsRegistryTest, InvalidQuery);
 
-  // content::URLFetcherDelegate implementation.
-  virtual void OnURLFetchComplete(const content::URLFetcher* source) OVERRIDE;
-
-  // |context| is a profile-dependent URL request context. Must not be NULL.
-  explicit CWSIntentsRegistry(net::URLRequestContextGetter* context);
-  virtual ~CWSIntentsRegistry();
-
   struct IntentsQuery;
 
   // This is an opaque version of URLFetcher*, so we can use it as a hash key.
   typedef intptr_t URLFetcherHandle;
 
-  // Maps URL fetchers to queries.
+  // Maps URL fetchers to queries. IntentsQuery objects are owned by the map.
   typedef base::hash_map<URLFetcherHandle, IntentsQuery*> QueryMap;
 
+  // |context| is a profile-dependent URL request context. Must not be NULL.
+  explicit CWSIntentsRegistry(net::URLRequestContextGetter* context);
+  virtual ~CWSIntentsRegistry();
+
+  // content::URLFetcherDelegate implementation.
+  virtual void OnURLFetchComplete(const content::URLFetcher* source) OVERRIDE;
+
   // Map for all in-flight web data requests/intent queries.
   QueryMap queries_;
 
diff --git a/chrome/browser/intents/cws_intents_registry_factory.cc b/chrome/browser/intents/cws_intents_registry_factory.cc
index b833521..6420627 100644
--- a/chrome/browser/intents/cws_intents_registry_factory.cc
+++ b/chrome/browser/intents/cws_intents_registry_factory.cc
@@ -2,6 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+
 #include "chrome/browser/intents/cws_intents_registry.h"
 #include "chrome/browser/intents/cws_intents_registry_factory.h"
 #include "chrome/browser/profiles/profile.h"
diff --git a/chrome/browser/intents/cws_intents_registry_factory.h b/chrome/browser/intents/cws_intents_registry_factory.h
index 28bff47..0c27439 100644
--- a/chrome/browser/intents/cws_intents_registry_factory.h
+++ b/chrome/browser/intents/cws_intents_registry_factory.h
@@ -10,10 +10,10 @@
 #include "base/memory/singleton.h"
 #include "chrome/browser/profiles/profile_keyed_service_factory.h"
 
-class Profile;
 class CWSIntentsRegistry;
+class Profile;
 
-// Singleton that owns all CWSIntentsRegistrys and associates each with
+// Singleton that owns all CWSIntentsRegistry objects and associates each with
 // their respective profile. Listens for the profile's destruction notification
 // and cleans up the associated CWSIntentsRegistry.
 class CWSIntentsRegistryFactory : public ProfileKeyedServiceFactory {
diff --git a/chrome/browser/intents/cws_intents_registry_unittest.cc b/chrome/browser/intents/cws_intents_registry_unittest.cc
index 2c258f5..690a542 100644
--- a/chrome/browser/intents/cws_intents_registry_unittest.cc
+++ b/chrome/browser/intents/cws_intents_registry_unittest.cc
@@ -28,11 +28,16 @@ const char kCWSQueryValid[] =
     "/chromewebstore/v1.1b/items/intent"
       "?intent=http%3A%2F%2Fwebintents.org%2Fedit&mime_types=*%2Fpng";
 const char kCWSResponseValid[] =
-  "{\"kind\":\"chromewebstore#itemList\",\"total_items\":1,"
-  "\"start_index\":0,\"items\":[{\"kind\":\"chromewebstore#item\","
-  "\"id\":\"nhkckhebbbncbkefhcpcgepcgfaclehe\",\"type\":\"APPLICATION\","
-  "\"num_ratings\":0,\"average_rating\":0.0,\"manifest\":"
-  "\"{\\n\\\"update_url\\\":\\"
+  "{\"kind\":\"chromewebstore#itemList\","
+  " \"total_items\":1,"
+  " \"start_index\":0,"
+  " \"items\":[  "
+  "  {\"kind\":\"chromewebstore#item\","
+  "   \"id\":\"nhkckhebbbncbkefhcpcgepcgfaclehe\","
+  "   \"type\":\"APPLICATION\","
+  "   \"num_ratings\":0,"
+  "   \"average_rating\":0.0,"
+  "   \"manifest\":\"{\\n\\\"update_url\\\":\\"
   "\"http://0.tbhome_staging.dserver.download-qa.td.borg.google.com/"
   "service/update2/crx\\\",\\n  \\\"name\\\": \\\"Sidd's Intent App\\\""
   ",\\n  \\\"description\\\": \\\"Do stuff\\\",\\n  \\\"version\\\": "
@@ -49,8 +54,9 @@ const char kCWSResponseValid[] =
   "{\\n      \\\"type\\\" : [\\\"text/plain\\\", \\\"image/jpg\\\"],"
   "\\n      \\\"path\\\" : \\\"//services/share\\\",\\n      \\\"title\\\" : "
   "\\\"Sample sharing Intent\\\",\\n      \\\"disposition\\\" : "
-  "\\\"inline\\\"\\n    }\\n  }\\n}\\n\",\"family_safe\":true,\"icon_url\":"
-  "\"http://qa-lighthouse.sandbox.google.com/image/"
+  "\\\"inline\\\"\\n    }\\n  }\\n}\\n\","
+  "   \"family_safe\":true,"
+  "   \"icon_url\":\"http://qa-lighthouse.sandbox.google.com/image/"
   "QzPnRCYCBbBGI99ZkGxkp-NNJ488IkkiTyCgynFEeDTJHcw4tHl3csmjTQ\"}]}";
 const char kValidIconURL[]=
     "http://qa-lighthouse.sandbox.google.com/image/"
@@ -73,7 +79,9 @@ const char kValidManifest[]=
 
 class CWSIntentsRegistryTest : public testing::Test {
  public:
-  CWSIntentsRegistryTest() : test_factory_(NULL) {}
+  CWSIntentsRegistryTest() : test_factory_(NULL) {
+  }
+
   virtual void TearDown() {
     // Pump messages posted by the main thread.
     ui_loop_.RunAllPending();
@@ -114,6 +122,10 @@ TEST_F(CWSIntentsRegistryTest, ValidQuery) {
   EXPECT_EQ(0, extensions_[0].num_ratings);
   EXPECT_EQ(0.0, extensions_[0].average_rating);
   EXPECT_EQ(std::string(kValidManifest), UTF16ToUTF8(extensions_[0].manifest));
+  EXPECT_EQ(std::string("nhkckhebbbncbkefhcpcgepcgfaclehe"),
+            UTF16ToUTF8(extensions_[0].id) );
+  EXPECT_EQ(std::string("Sidd's Intent App"),
+            UTF16ToUTF8(extensions_[0].name));
   EXPECT_EQ(std::string(kValidIconURL), extensions_[0].icon_url.spec());
 }