feedv2: UserJourney.GetMore reported failure too often

Don't report this journey at all if the GetMore request
was not allowed. This can happen if there's no next page
token for example, which is expected at the end of stream.

Bug: 1044139
Change-Id: I3330918653b012e4d85d595beef0db61d6c96f9f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2346794
Auto-Submit: Dan H <harringtond@chromium.org>
Reviewed-by: Carlos Knippschild <carlosk@chromium.org>
Commit-Queue: Dan H <harringtond@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797278}

components/feed/core/v2/feed_stream.cc

@@ -279,7 +279,6 @@ bool FeedStream::IsFeedEnabledByEnterprisePolicy() {
 
 void FeedStream::LoadMore(SurfaceId surface_id,
                           base::OnceCallback<void(bool)> callback) {
-  metrics_reporter_->OnLoadMoreBegin(surface_id);
   if (!model_) {
     DLOG(ERROR) << "Ignoring LoadMore() before the model is loaded";
     return std::move(callback).Run(false);
@@ -292,6 +291,7 @@ void FeedStream::LoadMore(SurfaceId surface_id,
     return std::move(callback).Run(false);
   }
 
+  metrics_reporter_->OnLoadMoreBegin(surface_id);
   surface_updater_->SetLoadingMore(true);
 
   // Have at most one in-flight LoadMore() request. Send the result to all

components/feed/core/v2/feed_stream_unittest.cc

@@ -408,6 +408,10 @@ class TestMetricsReporter : public MetricsReporter {
     MetricsReporter::OnLoadStream(load_from_store_status, final_status,
                                   std::move(latencies));
   }
+  void OnLoadMoreBegin(SurfaceId surface_id) override {
+    load_more_surface_id = surface_id;
+    MetricsReporter::OnLoadMoreBegin(surface_id);
+  }
   void OnLoadMore(LoadStreamStatus final_status) override {
     load_more_status = final_status;
     MetricsReporter::OnLoadMore(final_status);
@@ -425,6 +429,7 @@ class TestMetricsReporter : public MetricsReporter {
 
   base::Optional<int> slice_viewed_index;
   base::Optional<LoadStreamStatus> load_stream_status;
+  base::Optional<SurfaceId> load_more_surface_id;
   base::Optional<LoadStreamStatus> load_more_status;
   base::Optional<LoadStreamStatus> background_refresh_status;
   base::Optional<base::TimeDelta> time_since_last_clear;
@@ -1168,6 +1173,8 @@ TEST_F(FeedStreamTest, LoadMoreAppendsContent) {
   response_translator_.InjectResponse(MakeTypicalNextPageState(2));
   CallbackReceiver<bool> callback;
   stream_->LoadMore(surface.GetSurfaceId(), callback.Bind());
+  // Ensure metrics reporter was informed at the start of the operation.
+  EXPECT_EQ(surface.GetSurfaceId(), metrics_reporter_->load_more_surface_id);
   WaitForIdleTaskQueue();
   ASSERT_EQ(base::Optional<bool>(true), callback.GetResult());
   EXPECT_EQ("2 slices +spinner -> 4 slices", surface.DescribeUpdates());
@@ -1294,6 +1301,9 @@ TEST_F(FeedStreamTest, LoadMoreAbortsIfNoNextPageToken) {
   EXPECT_EQ(base::Optional<bool>(false), callback.GetResult());
   ASSERT_EQ(1, network_.send_query_call_count);
   EXPECT_EQ("loading -> 2 slices", surface.DescribeUpdates());
+  EXPECT_EQ(base::nullopt, metrics_reporter_->load_more_surface_id)
+      << "metrics reporter was informed about a load more operation which "
+         "didn't begin";
 }
 
 TEST_F(FeedStreamTest, LoadMoreFail) {

components/feed/core/v2/metrics_reporter.h

@@ -104,7 +104,7 @@ class MetricsReporter {
                             LoadStreamStatus final_status,
                             std::unique_ptr<LoadLatencyTimes> load_latencies);
   virtual void OnBackgroundRefresh(LoadStreamStatus final_status);
-  void OnLoadMoreBegin(SurfaceId surface_id);
+  virtual void OnLoadMoreBegin(SurfaceId surface_id);
   virtual void OnLoadMore(LoadStreamStatus final_status);
   virtual void OnClearAll(base::TimeDelta time_since_last_clear);
   // Called each time the surface receives new content.