Cheer up spell-checking code

chrome/browser/spellchecker/feedback_sender.cc

-// Copyright (c) 2013 The Chromium Authors. All rights reserved.
+// Copyright 2013 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.
 //
 // The |FeedbackSender| object stores the user feedback to spellcheck
 // suggestions in a |Feedback| object.
 //
 // When spelling service returns spellcheck results, these results first arrive
 // in |FeedbackSender| to assign hash identifiers for each
 // misspelling-suggestion pair. If the spelling service identifies the same
 // misspelling as already displayed to the user, then |FeedbackSender| reuses
@@ skipping 74 matching lines @@
                                start + result.length,
                                text,
                                chrome::spellcheck_common::kContextWordCount);
   return Misspelling(context,
                      start,
                      result.length,
                      std::vector<base::string16>(1, result.replacement),
                      result.hash);
 }
 
-// Builds suggestion info from |suggestions|. The caller owns the result.
-base::ListValue* BuildSuggestionInfo(
-    const std::vector<Misspelling>& suggestions,
+// Builds suggestion info from |suggestions|.
+scoped_ptr<base::ListValue> BuildSuggestionInfo(
+    const std::vector<Misspelling>& misspellings,
     bool is_first_feedback_batch) {
-  base::ListValue* list = new base::ListValue;
-  for (std::vector<Misspelling>::const_iterator suggestion_it =
-           suggestions.begin();
-       suggestion_it != suggestions.end();
-       ++suggestion_it) {
-    base::DictionaryValue* suggestion = SerializeMisspelling(*suggestion_it);
-    suggestion->SetBoolean("isFirstInSession", is_first_feedback_batch);
-    suggestion->SetBoolean("isAutoCorrection", false);
-    list->Append(suggestion);
+  scoped_ptr<base::ListValue> list(new base::ListValue);
+  for (const auto& raw_misspelling : misspellings) {
+    scoped_ptr<base::DictionaryValue> misspelling(
+        SerializeMisspelling(raw_misspelling));
+    misspelling->SetBoolean("isFirstInSession", is_first_feedback_batch);
+    misspelling->SetBoolean("isAutoCorrection", false);
+    list->Append(misspelling.release());
   }
   return list;
 }
 
 // Builds feedback parameters from |suggestion_info|, |language|, and |country|.
-// Takes ownership of |suggestion_list|. The caller owns the result.
-base::DictionaryValue* BuildParams(base::ListValue* suggestion_info,
-                                   const std::string& language,
-                                   const std::string& country) {
-  base::DictionaryValue* params = new base::DictionaryValue;
-  params->Set("suggestionInfo", suggestion_info);
+// Takes ownership of |suggestion_list|.
+scoped_ptr<base::DictionaryValue> BuildParams(
+    scoped_ptr<base::ListValue> suggestion_info,
+    const std::string& language,
+    const std::string& country) {
+  scoped_ptr<base::DictionaryValue> params(new base::DictionaryValue);
+  params->Set("suggestionInfo", suggestion_info.release());
   params->SetString("key", google_apis::GetAPIKey());
   params->SetString("language", language);
   params->SetString("originCountry", country);
   params->SetString("clientName", "Chrome");
   return params;
 }
 
-// Builds feedback data from |params|. Takes ownership of |params|. The caller
-// owns the result.
-base::Value* BuildFeedbackValue(base::DictionaryValue* params,
-                                const std::string& api_version) {
-  base::DictionaryValue* result = new base::DictionaryValue;
-  result->Set("params", params);
+// Builds feedback data from |params|. Takes ownership of |params|.
+scoped_ptr<base::Value> BuildFeedbackValue(
+    scoped_ptr<base::DictionaryValue> params,
+    const std::string& api_version) {
+  scoped_ptr<base::DictionaryValue> result(new base::DictionaryValue);
+  result->Set("params", params.release());
   result->SetString("method", "spelling.feedback");
   result->SetString("apiVersion", api_version);
-  return result;
+  return std::move(result);

[groby-ooo-7-16, 2016/02/09 20:20:26]

Does this need to be std::move? The move ctor _should_ handle upcasts.

[Kevin Bailey, 2016/02/09 22:26:39]

../../chrome/browser/spellchecker/feedback_sender.cc:134:10: error: no viable
conversion from returned value of type 'scoped_ptr<base::DictionaryValue>' to
function return type 'scoped_ptr<base::Value>'
  return result;

 }
 
 // Returns true if the misspelling location is within text bounds.
 bool IsInBounds(int misspelling_location,
                 int misspelling_length,
                 size_t text_length) {
   return misspelling_location >= 0 && misspelling_length > 0 &&
          static_cast<size_t>(misspelling_location) < text_length &&
          static_cast<size_t>(misspelling_location + misspelling_length) <=
              text_length;
@@ skipping 53 matching lines @@
 void FeedbackSender::AddedToDictionary(uint32_t hash) {
   Misspelling* misspelling = feedback_.GetMisspelling(hash);
   // GetMisspelling() returns null for flushed feedback. Feedback is flushed
   // when the session expires every |kSessionHours| hours.
   if (!misspelling)
     return;
   misspelling->action.set_type(SpellcheckAction::TYPE_ADD_TO_DICT);
   misspelling->timestamp = base::Time::Now();
   const std::set<uint32_t>& hashes =
       feedback_.FindMisspellings(GetMisspelledString(*misspelling));
-  for (std::set<uint32_t>::const_iterator hash_it = hashes.begin();
-       hash_it != hashes.end(); ++hash_it) {
-    Misspelling* duplicate_misspelling = feedback_.GetMisspelling(*hash_it);
+  for (uint32_t hash : hashes) {
+    Misspelling* duplicate_misspelling = feedback_.GetMisspelling(hash);
     if (!duplicate_misspelling || duplicate_misspelling->action.IsFinal())
       continue;
     duplicate_misspelling->action.set_type(SpellcheckAction::TYPE_ADD_TO_DICT);
     duplicate_misspelling->timestamp = misspelling->timestamp;
   }
 }
 
 void FeedbackSender::RecordInDictionary(uint32_t hash) {
   Misspelling* misspelling = feedback_.GetMisspelling(hash);
   // GetMisspelling() returns null for flushed feedback. Feedback is flushed
@@ skipping 54 matching lines @@
     return;
 
   // Generate a map of marker offsets to marker hashes. This map helps to
   // efficiently lookup feedback data based on the position of the misspelling
   // in text.
   typedef std::map<size_t, uint32_t> MarkerMap;
   MarkerMap marker_map;
   for (size_t i = 0; i < markers.size(); ++i)
     marker_map[markers[i].offset] = markers[i].hash;
 
-  for (std::vector<SpellCheckResult>::iterator result_it = results->begin();
-       result_it != results->end();
-       ++result_it) {
-    if (!IsInBounds(result_it->location, result_it->length, text.length()))
+  for (auto& result : *results) {
+    if (!IsInBounds(result.location, result.length, text.length()))
       continue;
-    MarkerMap::const_iterator marker_it = marker_map.find(result_it->location);
+    MarkerMap::const_iterator marker_it = marker_map.find(result.location);
     if (marker_it != marker_map.end() &&
         feedback_.HasMisspelling(marker_it->second)) {
       // If the renderer already has a marker for this spellcheck result, then
       // set the hash of the spellcheck result to be the same as the marker.
-      result_it->hash = marker_it->second;
+      result.hash = marker_it->second;
     } else {
       // If the renderer does not yet have a marker for this spellcheck result,
       // then generate a new hash for the spellcheck result.
-      result_it->hash = BuildHash(session_start_, ++misspelling_counter_);
+      result.hash = BuildHash(session_start_, ++misspelling_counter_);
     }
     // Save the feedback data for the spellcheck result.
-    feedback_.AddMisspelling(renderer_process_id,
-                             BuildFeedback(*result_it, text));
+    feedback_.AddMisspelling(renderer_process_id, BuildFeedback(result, text));
   }
 }
 
 void FeedbackSender::OnLanguageCountryChange(const std::string& language,
                                              const std::string& country) {
   FlushFeedback();
   language_ = language;
   country_ = country;
 }
 
@@ skipping 53 matching lines @@
        it.Advance()) {
     alive_renderers.insert(it.GetCurrentValue()->GetID());
     it.GetCurrentValue()->Send(new SpellCheckMsg_RequestDocumentMarkers());
   }
 
   // Asynchronously send out the feedback for all the renderers that are no
   // longer alive.
   std::vector<int> known_renderers = feedback_.GetRendersWithMisspellings();
   std::sort(known_renderers.begin(), known_renderers.end());
   std::vector<int> dead_renderers =
-      base::STLSetDifference<std::vector<int> >(known_renderers,
-                                                alive_renderers);
-  for (std::vector<int>::const_iterator it = dead_renderers.begin();
-       it != dead_renderers.end();
-       ++it) {
+      base::STLSetDifference<std::vector<int>>(known_renderers,
+                                               alive_renderers);
+  for (int renderer : dead_renderers) {

[groby-ooo-7-16, 2016/02/09 20:20:26]

nit: renderer_process_id (I wish we had a type for this, but since we don't,
let's make clear what the int means)

[Kevin Bailey, 2016/02/09 22:26:39]

Done.

     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE, base::Bind(&FeedbackSender::OnReceiveDocumentMarkers,
-                              AsWeakPtr(), *it, std::vector<uint32_t>()));
+                              AsWeakPtr(), renderer, std::vector<uint32_t>()));
   }
 }
 
 void FeedbackSender::FlushFeedback() {
   if (feedback_.Empty())
     return;
   feedback_.FinalizeAllMisspellings();
   SendFeedback(feedback_.GetAllMisspellings(),
                renderers_sent_feedback_.empty());
   feedback_.Clear();
@@ skipping 17 matching lines @@
   net::URLFetcher* sender =
       net::URLFetcher::Create(kUrlFetcherId, feedback_service_url_,
                               net::URLFetcher::POST, this).release();
   data_use_measurement::DataUseUserData::AttachToFetcher(
       sender, data_use_measurement::DataUseUserData::SPELL_CHECKER);
   sender->SetLoadFlags(net::LOAD_DO_NOT_SEND_COOKIES |
                        net::LOAD_DO_NOT_SAVE_COOKIES);
   sender->SetUploadData("application/json", feedback);
   senders_.push_back(sender);
 
-  // Request context is NULL in testing.
+  // Request context is nullptr in testing.
   if (request_context_.get()) {
     sender->SetRequestContext(request_context_.get());
     sender->Start();
   }
 }
 
 }  // namespace spellcheck