Remove AutocompleteMatch::is_starred

chrome/browser/ui/webui/omnibox/omnibox_ui_handler.cc

 // Copyright (c) 2012 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.
 
 #include "chrome/browser/ui/webui/omnibox/omnibox_ui_handler.h"
 
 #include <string>
 
 #include "base/auto_reset.h"
 #include "base/bind.h"
 #include "base/strings/string16.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
 #include "base/values.h"
 #include "chrome/browser/autocomplete/autocomplete_classifier.h"
 #include "chrome/browser/autocomplete/autocomplete_controller.h"
 #include "chrome/browser/autocomplete/autocomplete_match.h"
 #include "chrome/browser/autocomplete/autocomplete_provider.h"
 #include "chrome/browser/autocomplete/chrome_autocomplete_scheme_classifier.h"
+#include "chrome/browser/bookmarks/bookmark_model_factory.h"
 #include "chrome/browser/history/history_service.h"
 #include "chrome/browser/history/history_service_factory.h"
 #include "chrome/browser/search/search.h"
 #include "chrome/browser/search_engines/template_url_service_factory.h"
+#include "components/bookmarks/browser/bookmark_model.h"
 #include "components/history/core/browser/url_database.h"
 #include "components/metrics/proto/omnibox_event.pb.h"
 #include "components/search_engines/template_url.h"
 #include "content/public/browser/web_ui.h"
 #include "mojo/common/common_type_converters.h"
 
 namespace mojo {
 
 template <>
 class TypeConverter<mojo::Array<AutocompleteAdditionalInfoPtr>,
@@ skipping 40 matching lines @@
     result->transition = input.transition;
     result->is_history_what_you_typed_match =
         input.is_history_what_you_typed_match;
     result->allowed_to_be_default_match = input.allowed_to_be_default_match;
     result->type = AutocompleteMatchType::ToString(input.type);
     if (input.associated_keyword.get() != NULL) {
       result->associated_keyword =
           mojo::String::From(input.associated_keyword->keyword);
     }
     result->keyword = mojo::String::From(input.keyword);
-    result->starred = input.starred;
     result->duplicates = static_cast<int32>(input.duplicate_matches.size());
     result->from_previous = input.from_previous;
 
     result->additional_info =
         mojo::Array<AutocompleteAdditionalInfoPtr>::From(input.additional_info);
     return result.Pass();
   }
 };
 
 template <>
@@ skipping 38 matching lines @@
     // Copy to an ACMatches to make conversion easier. Since this isn't
     // performance critical we don't worry about the cost here.
     ACMatches matches(controller_->result().begin(),
                       controller_->result().end());
     result->combined_results =
         mojo::Array<AutocompleteMatchMojoPtr>::From(matches);
   }
   result->results_by_provider =
       mojo::Array<AutocompleteResultsForProviderMojoPtr>::From(
           controller_->providers());
+
+  // Fill AutocompleteMatchMojo::starred.
+  BookmarkModel* bookmark_model = BookmarkModelFactory::GetForProfile(profile_);
+  if (bookmark_model) {
+    for (size_t i =  0; i < result->combined_results.size(); ++i) {

[Peter Kasting, 2014/07/26 02:12:08]

Nit: Use an iterator instead of an index here and in the other loops.

[hashimoto, 2014/07/28 06:49:43]

IIUC mojo::Array doesn't provide iterators.
(https://code.google.com/p/chromium/codesearch#chromium/src/mojo/public/cpp/bi...)

[Peter Kasting, 2014/07/28 11:11:02]

Then unless it is infeasible to do so, can you add iterators (might have to be
just const_iterators?) to that class?  If the underlying storage is an actual
array, this should just be a matter of returning some pointers.

[hashimoto, 2014/07/29 02:03:49]

If we are to use iterators here, it should be non-const as we are modifying the
contents of the array.
mojo::Array is basically an array, but not when the stored type is a move-only
type.
(https://code.google.com/p/chromium/codesearch#chromium/src/mojo/public/cpp/bi...)
Using a pointer as an iterator doesn't work for this case ((&a[0] + 1) != &a[1]
may be true with mojo::Array) so adding iterators for mojo::Array should require
a certain amount of hassle.

[Peter Kasting, 2014/07/29 02:19:50]

That sounds scary given that the header file for Mojo::Array says that it
provides "read-only access" to the array data.  Maybe that comment is incorrect?

It seems from array_internal.h like ultimately the underlying type ArrayTraits
deals with is always a vector.  Can't we just typedef a Traits::iterator type to
the underlying vector iterator and add begin() and end() functions that return
the iterators of that vector?

+      result->combined_results[i]->starred = bookmark_model->IsBookmarked(
+          GURL(result->combined_results[i]->destination_url));
+    }
+    for (size_t i = 0; i < result->results_by_provider.size(); ++i) {
+      const AutocompleteResultsForProviderMojo& result_by_provider =
+          *result->results_by_provider[i];
+      for (size_t j = 0; j < result_by_provider.results.size(); ++j) {
+        result_by_provider.results[j]->starred = bookmark_model->IsBookmarked(
+            GURL(result_by_provider.results[j]->destination_url));
+      }
+    }
+  }
+
   client()->HandleNewAutocompleteResult(result.Pass());
 }
 
 bool OmniboxUIHandler::LookupIsTypedHost(const base::string16& host,
                                          bool* is_typed_host) const {
   HistoryService* const history_service =
       HistoryServiceFactory::GetForProfile(profile_,
                                            Profile::EXPLICIT_ACCESS);
   if (!history_service)
     return false;
@@ skipping 25 matching lines @@
       ChromeAutocompleteSchemeClassifier(profile_));
   controller_->Start(input_);
 }
 
 void OmniboxUIHandler::ResetController() {
   controller_.reset(new AutocompleteController(profile_,
           TemplateURLServiceFactory::GetForProfile(profile_),
           this,
           AutocompleteClassifier::kDefaultOmniboxProviders));
 }