Feedv2: Don't try to load more without a token

If the server doesn't provide a continuation token, we
shouldn't continue to request the next page.

Bug: 1044139
Change-Id: I8a9de4013fdedb269dda904cc3c81123631e7eea
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2261410
Commit-Queue: Dan H <harringtond@chromium.org>
Reviewed-by: Ian Wells <iwells@chromium.org>
Cr-Commit-Position: refs/heads/master@{#782777}

components/feed/core/v2/enums.cc

@@ -51,6 +51,8 @@ std::ostream& operator<<(std::ostream& out, LoadStreamStatus value) {
       return out << "kLoadNotAllowedDisabledByEnterprisePolicy";
     case LoadStreamStatus::kNetworkFetchFailed:
       return out << "kNetworkFetchFailed";
+    case LoadStreamStatus::kCannotLoadMoreNoNextPageToken:
+      return out << "kCannotLoadMoreNoNextPageToken";
   }
 #else
   return out << (static_cast<int>(value));

components/feed/core/v2/enums.h

@@ -40,7 +40,8 @@ enum class LoadStreamStatus {
   kLoadMoreModelIsNotLoaded = 16,
   kLoadNotAllowedDisabledByEnterprisePolicy = 17,
   kNetworkFetchFailed = 18,
-  kMaxValue = kNetworkFetchFailed,
+  kCannotLoadMoreNoNextPageToken = 19,
+  kMaxValue = kCannotLoadMoreNoNextPageToken,
 };
 
 std::ostream& operator<<(std::ostream& out, LoadStreamStatus value);

components/feed/core/v2/feed_stream.cc

@@ -210,7 +210,16 @@ void FeedStream::LoadMore(SurfaceId surface_id,
     DLOG(ERROR) << "Ignoring LoadMore() before the model is loaded";
     return std::move(callback).Run(false);
   }
+  // We want to abort early to avoid showing a loading spinner if it's not
+  // necessary.
+  if (ShouldMakeFeedQueryRequest(/*is_load_more=*/true,
+                                 /*consume_quota=*/false) !=
+      LoadStreamStatus::kNoStatus) {
+    return std::move(callback).Run(false);
+  }
+
   surface_updater_->SetLoadingMore(true);
+
   // Have at most one in-flight LoadMore() request. Send the result to all
   // requestors.
   load_more_complete_callbacks_.push_back(std::move(callback));
@@ -371,7 +380,8 @@ LoadStreamStatus FeedStream::ShouldAttemptLoad(bool model_loading) {
   return LoadStreamStatus::kNoStatus;
 }
 
-LoadStreamStatus FeedStream::ShouldMakeFeedQueryRequest(bool is_load_more) {
+LoadStreamStatus FeedStream::ShouldMakeFeedQueryRequest(bool is_load_more,
+                                                        bool consume_quota) {
   if (!is_load_more) {
     // Time has passed since calling |ShouldAttemptLoad()|, call it again to
     // confirm we should still attempt loading.
@@ -380,6 +390,11 @@ LoadStreamStatus FeedStream::ShouldMakeFeedQueryRequest(bool is_load_more) {
     if (should_not_attempt_reason != LoadStreamStatus::kNoStatus) {
       return should_not_attempt_reason;
     }
+  } else {
+    // LoadMore requires a next page token.
+    if (!model_ || model_->GetNextPageToken().empty()) {
+      return LoadStreamStatus::kCannotLoadMoreNoNextPageToken;
+    }
   }
 
   // TODO(harringtond): |suppress_refreshes_until_| was historically used
@@ -396,7 +411,8 @@ LoadStreamStatus FeedStream::ShouldMakeFeedQueryRequest(bool is_load_more) {
     return LoadStreamStatus::kCannotLoadFromNetworkOffline;
   }
 
-  if (!request_throttler_.RequestQuota(NetworkRequestType::kFeedQuery)) {
+  if (consume_quota &&
+      !request_throttler_.RequestQuota(NetworkRequestType::kFeedQuery)) {
     return LoadStreamStatus::kCannotLoadFromNetworkThrottled;
   }
 

components/feed/core/v2/feed_stream.h

@@ -198,8 +198,11 @@ class FeedStream : public FeedStreamApi,
 
   // Determines if a FeedQuery request can be made. If successful,
   // returns |LoadStreamStatus::kNoStatus| and acquires throttler quota.
-  // Otherwise returns the reason.
-  LoadStreamStatus ShouldMakeFeedQueryRequest(bool is_load_more = false);
+  // Otherwise returns the reason. If |consume_quota| is false, no quota is
+  // consumed. This can be used to predict the likely result on a subsequent
+  // call.
+  LoadStreamStatus ShouldMakeFeedQueryRequest(bool is_load_more = false,
+                                              bool consume_quota = true);
 
   // Unloads the model. Surfaces are not updated, and will remain frozen until a
   // model load is requested.

components/feed/core/v2/feed_stream_unittest.cc

@@ -1153,6 +1153,26 @@ TEST_F(FeedStreamTest, LoadMoreSendsTokens) {
                           .next_page_token());
 }
 
+TEST_F(FeedStreamTest, LoadMoreAbortsIfNoNextPageToken) {
+  {
+    std::unique_ptr<StreamModelUpdateRequest> initial_state =
+        MakeTypicalInitialModelState();
+    initial_state->stream_data.clear_next_page_token();
+    response_translator_.InjectResponse(std::move(initial_state));
+  }
+  TestSurface surface(stream_.get());
+  WaitForIdleTaskQueue();
+
+  CallbackReceiver<bool> callback;
+  stream_->LoadMore(surface.GetSurfaceId(), callback.Bind());
+  WaitForIdleTaskQueue();
+
+  // LoadMore fails, and does not make an additional request.
+  EXPECT_EQ(base::Optional<bool>(false), callback.GetResult());
+  ASSERT_EQ(1, network_.send_query_call_count);
+  EXPECT_EQ("loading -> 2 slices", surface.DescribeUpdates());
+}
+
 TEST_F(FeedStreamTest, LoadMoreFail) {
   response_translator_.InjectResponse(MakeTypicalInitialModelState());
   TestSurface surface(stream_.get());

components/feed/core/v2/surface_updater.cc

@@ -150,6 +150,7 @@ feedui::ZeroStateSlice::Type GetZeroStateType(LoadStreamStatus status) {
     case LoadStreamStatus::kCannotParseNetworkResponseBody:
     case LoadStreamStatus::kLoadMoreModelIsNotLoaded:
     case LoadStreamStatus::kLoadNotAllowedDisabledByEnterprisePolicy:
+    case LoadStreamStatus::kCannotLoadMoreNoNextPageToken:
       break;
   }
   return feedui::ZeroStateSlice::NO_CARDS_AVAILABLE;