Local NTP: Expose "MV available" to the page, don't load tiles until it is

This works around a race condition between loading the page and the IPC
message that contains the MV items.
Before this CL, if the page load won the race, then it would assume there
are no tiles and load incorrect metrics. This CL exposes "did we ever
receive MV tiles" to the page, so it can delay loading them while they're
not available anyway.

This is very much working around the symptoms rather than fixing the root
cause. Unfortunately I have no idea how to really fix the issue.

Bug: 794942, 793818
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I091b71d17d36e28b3f53ceebef6829ef23d82b3b
Reviewed-on: https://chromium-review.googlesource.com/829254Reviewed-by: Chris Pickel <sfiera@chromium.org>
Commit-Queue: Marc Treib <treib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524415}

chrome/browser/resources/local_ntp/local_ntp.js

@@ -398,6 +398,15 @@ function onMostVisitedChange() {
  * them to the iframe.
  */
 function reloadTiles() {
+  // Don't attempt to load tiles if the MV data isn't available yet - this can
+  // happen occasionally, see https://crbug.com/794942. In that case, we should
+  // get an onMostVisitedChange call once they are available.
+  // Note that MV data being available is different from having > 0 tiles. There
+  // can legitimately be 0 tiles, e.g. if the user blacklisted them all.
+  if (!ntpApiHandle.mostVisitedAvailable) {
+    return;
+  }
+
   var pages = ntpApiHandle.mostVisited;
   var cmds = [];
   for (var i = 0; i < Math.min(MAX_NUM_TILES_TO_SHOW, pages.length); ++i) {

chrome/browser/resources/local_ntp/most_visited_single.js

@@ -199,6 +199,8 @@ var handleCommand = function(data) {
   if (cmd == 'tile') {
     addTile(data);
   } else if (cmd == 'show') {
+    // TODO(treib): If this happens before we have finished loading the previous
+    // tiles, we probably get into a bad state.
     showTiles(data);
   } else if (cmd == 'updateTheme') {
     updateTheme(data);

chrome/renderer/searchbox/searchbox.cc

@@ -219,6 +219,7 @@ SearchBox::SearchBox(content::RenderFrame* render_frame)
       is_input_in_progress_(false),
       is_key_capture_enabled_(false),
       most_visited_items_cache_(kMaxInstantMostVisitedItemCacheSize),
+      has_received_most_visited_(false),
       weak_ptr_factory_(this) {
   // Connect to the embedded search interface in the browser.
   chrome::mojom::EmbeddedSearchConnectorAssociatedPtr connector;
@@ -282,6 +283,10 @@ void SearchBox::GetMostVisitedItems(
   most_visited_items_cache_.GetCurrentItems(items);
 }
 
+bool SearchBox::AreMostVisitedItemsAvailable() const {
+  return has_received_most_visited_;
+}
+
 bool SearchBox::GetMostVisitedItemWithID(
     InstantRestrictedID most_visited_item_id,
     InstantMostVisitedItem* item) const {
@@ -368,6 +373,8 @@ void SearchBox::HistorySyncCheckResult(bool sync_history) {
 
 void SearchBox::MostVisitedChanged(
     const std::vector<InstantMostVisitedItem>& items) {
+  has_received_most_visited_ = true;
+
   std::vector<InstantMostVisitedItemIDPair> last_known_items;
   GetMostVisitedItems(&last_known_items);
 

chrome/renderer/searchbox/searchbox.h

@@ -95,6 +95,8 @@ class SearchBox : public content::RenderFrameObserver,
   void GetMostVisitedItems(
       std::vector<InstantMostVisitedItemIDPair>* items) const;
 
+  bool AreMostVisitedItemsAvailable() const;
+
   // If the |most_visited_item_id| is found in the cache, sets |item| to it
   // and returns true.
   bool GetMostVisitedItemWithID(InstantRestrictedID most_visited_item_id,
@@ -165,6 +167,7 @@ class SearchBox : public content::RenderFrameObserver,
   bool is_input_in_progress_;
   bool is_key_capture_enabled_;
   InstantRestrictedIDCache<InstantMostVisitedItem> most_visited_items_cache_;
+  bool has_received_most_visited_;
   ThemeBackgroundInfo theme_info_;
 
   base::WeakPtrFactory<SearchBox> weak_ptr_factory_;

chrome/renderer/searchbox/searchbox_extension.cc

@@ -549,6 +549,7 @@ class NewTabPageBindings : public gin::Wrappable<NewTabPageBindings> {
   // Handlers for JS properties.
   static bool IsInputInProgress();
   static v8::Local<v8::Value> GetMostVisited(v8::Isolate* isolate);
+  static bool GetMostVisitedAvailable(v8::Isolate* isolate);
   static v8::Local<v8::Value> GetThemeBackgroundInfo(v8::Isolate* isolate);
 
   // Handlers for JS functions visible to all NTPs.
@@ -592,6 +593,8 @@ gin::ObjectTemplateBuilder NewTabPageBindings::GetObjectTemplateBuilder(
   return gin::Wrappable<NewTabPageBindings>::GetObjectTemplateBuilder(isolate)
       .SetProperty("isInputInProgress", &NewTabPageBindings::IsInputInProgress)
       .SetProperty("mostVisited", &NewTabPageBindings::GetMostVisited)
+      .SetProperty("mostVisitedAvailable",
+                   &NewTabPageBindings::GetMostVisitedAvailable)
       .SetProperty("themeBackgroundInfo",
                    &NewTabPageBindings::GetThemeBackgroundInfo)
       .SetMethod("checkIsUserSignedIntoChromeAs",
@@ -658,6 +661,15 @@ v8::Local<v8::Value> NewTabPageBindings::GetMostVisited(v8::Isolate* isolate) {
   return v8_mv_items;
 }
 
+// static
+bool NewTabPageBindings::GetMostVisitedAvailable(v8::Isolate* isolate) {
+  const SearchBox* search_box = GetSearchBoxForCurrentContext();
+  if (!search_box)
+    return false;
+
+  return search_box->AreMostVisitedItemsAvailable();
+}
+
 // static
 v8::Local<v8::Value> NewTabPageBindings::GetThemeBackgroundInfo(
     v8::Isolate* isolate) {