[AppList] Add a Suggestions Provider to applist.

chrome/browser/ui/app_list/search/suggestions/suggestions_result.cc

Index: chrome/browser/ui/app_list/search/suggestions/suggestions_result.cc
diff --git a/chrome/browser/ui/app_list/search/suggestions/suggestions_result.cc b/chrome/browser/ui/app_list/search/suggestions/suggestions_result.cc
new file mode 100644
index 0000000000000000000000000000000000000000..c1bb86595c11f050bbd748053063c63aeaa2fd76
--- /dev/null
+++ b/chrome/browser/ui/app_list/search/suggestions/suggestions_result.cc
@@ -0,0 +1,73 @@
+// Copyright 2014 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/app_list/search/suggestions/suggestions_result.h"
+
+#include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/app_list/app_list_controller_delegate.h"
+#include "chrome/browser/ui/app_list/search/search_util.h"
+#include "components/suggestions/suggestions_service.h"
+#include "third_party/skia/include/core/SkBitmap.h"
+#include "ui/base/page_transition_types.h"
+#include "ui/gfx/image/image_skia.h"
+#include "url/gurl.h"
+
+namespace app_list {
+
+SuggestionsResult::SuggestionsResult(
+    Profile* profile, AppListControllerDelegate* list_controller,
+    suggestions::SuggestionsService* suggestions_service,
+    const suggestions::ChromeSuggestion& suggestion, int index)
+    : profile_(profile),
+      list_controller_(list_controller),
+      suggestions_service_(suggestions_service),
+      suggestion_(suggestion),
+      index_(index),
+      weak_ptr_factory_(this) {
+  set_id(suggestion_.url());
+  set_title(base::UTF8ToUTF16(suggestion_.title()));
+  set_display_type(DISPLAY_TILE);
+
+  // The index of the suggestion is an indication of its relevance.
+  if (index_) set_relevance(1.0 / index_);

[calamity, 2014/12/05 04:01:58]

label public_relevance:
I think that you should be setting the relevance in
suggestions_search_provider.cc by making set_relevance() public for this class.

Since this class isn't intrinsically calculating its relevance, it's confusing
to work out from this class alone what index is supposed to mean. Also, there's
an implicit requirement that index be 1-based which would go away if the
relevance was set from the provider.

[Mathieu, 2014/12/05 20:14:32]

Done.

+
+  UpdateIcon();
+}
+
+SuggestionsResult::~SuggestionsResult() {}
+
+void SuggestionsResult::Open(int event_flags) {
+  RecordHistogram(SUGGESTIONS_SEARCH_RESULT);
+  list_controller_->OpenURL(profile_, GURL(suggestion_.url()),
+                            ui::PageTransition::PAGE_TRANSITION_LINK,
+                            ui::DispositionFromEventFlags(event_flags));

[calamity, 2014/12/05 04:01:58]

So this type of result is a URL suggestion? If so, I think it would be more
informative to name the class URLSuggestionResult or similar.

[Mathieu, 2014/12/05 20:14:32]

Done.

+}
+
+scoped_ptr<SearchResult> SuggestionsResult::Duplicate() {
+  return scoped_ptr<SearchResult>(new SuggestionsResult(
+      profile_, list_controller_, suggestions_service_, suggestion_, index_));

[calamity, 2014/12/05 04:01:58]

If you went through with #public_relevance, you'd need to set the relevance of
the new result here too.

[Mathieu, 2014/12/05 20:14:32]

Done.

+}
+
+void SuggestionsResult::UpdateIcon() {
+  if (suggestions_service_) {
+    suggestions_service_->GetPageThumbnail(
+        GURL(suggestion_.url()),
+        base::Bind(&SuggestionsResult::OnSuggestionsThumbnailAvailable,
+                   weak_ptr_factory_.GetWeakPtr()));
+  }

[calamity, 2014/12/05 04:01:58]

Hmm. Does this mean that a blank suggestion will flicker onto the suggestions
and then vanish once it figures out it has no icon?

If that's the case, it may be better to have the SuggestionsSearchProvider
fan-in all of these icon loads before creating any SuggestionsResults.

The apps provider kind of gets away with it because it's loading from the disk
and the result doesn't disappear completely if the icon fails to load.

Some other options exist here... If anyone thinks of something better, please
let me know.

[Mathieu, 2014/12/05 20:14:32]

We actually heavily cache images to disk, and so in this version they are
technically only fetched once for each of them, in which case there would be a
small delay the first time. We can perhaps modify the call to GetPageThumbnail
to never go to network in this path, which is what FetchSuggestionsData does. It
would rather only serve from cache, and update in the background.

I have a next change ready for this class that will use local favicons/large
touch icons if available before looking for a fallback with
|suggestions_service_|.

I propose we leave this here and optimize it later with the two things I just
said.

[calamity, 2014/12/10 01:47:42]

Sweet.
Since this is behind an experiment, I'm ok with this.

+}
+
+void SuggestionsResult::OnSuggestionsThumbnailAvailable(
+    const GURL& url, const SkBitmap* bitmap) {
+  if (bitmap) {
+    SetIcon(gfx::ImageSkia::CreateFrom1xBitmap(*bitmap));
+  } else {
+    // There is no image for this suggestion. Disable being shown on the start
+    // screen.
+    set_display_type(DISPLAY_LIST);

[calamity, 2014/12/05 04:01:58]

Consider adding a DISPLAY_NONE to SearchResult.

[Mathieu, 2014/12/05 20:14:32]

Done.

+  }
+}
+
+}  // namespace app_list