[ios] TabUsageRecorder should use virtual URL to decided when to record.

TabUsageRecorder::ShouldIgnoreWebState() is meant to filter out tabs
that are currently loading a chrome:// URL and not record page load
stats for them. However, it treats pending item and last committed item
inconsistently and looks at the URL for one and virtual URL for the
other. This causes a problem when switching to WKBasedNavigationManager
because switching to an evicted tab will load restore_session.html in
the web state. This results in a pending item with a file:/// URL and
a virtual URL that is whatever the page that is being restored, which
may be a chrome:// URL.

Not using virtual URL for pending item seems to be a bug introduced in
https://codereview.chromium.org/2820763002. It replaces [tab url] with
pending_item->GetURL. At the time, [tab url] was implemented using
virtual URL (see https://codereview.chromium.org/2919983002).

Added a unit test to cover this case.

Bug: 789993
Cq-Include-Trybots: master.tryserver.chromium.mac:ios-simulator-cronet;master.tryserver.chromium.mac:ios-simulator-full-configs
Change-Id: I0861ee94ca0d3c5612d3e1ad7680f18bfd7368f0
Reviewed-on: https://chromium-review.googlesource.com/817356Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Reviewed-by: Peter Lee <pkl@chromium.org>
Reviewed-by: Eugene But <eugenebut@chromium.org>
Commit-Queue: Danyao Wang <danyao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523813}

ios/chrome/browser/metrics/tab_usage_recorder.mm

@@ -370,7 +370,7 @@ bool TabUsageRecorder::ShouldIgnoreWebState(web::WebState* web_state) {
   web::NavigationItem* pending_item =
       web_state->GetNavigationManager()->GetPendingItem();
   if (pending_item)
-    return pending_item->GetURL().SchemeIs(kChromeUIScheme);
+    return pending_item->GetVirtualURL().SchemeIs(kChromeUIScheme);
 
   web::NavigationItem* last_committed_item =
       web_state->GetNavigationManager()->GetLastCommittedItem();

ios/chrome/browser/metrics/tab_usage_recorder_unittest.mm

@@ -59,10 +59,9 @@ class TabUsageRecorderTest : public PlatformTest {
                                         WebStateInMemoryOption in_memory) {
     auto test_navigation_manager =
         std::make_unique<web::TestNavigationManager>();
-    test_navigation_manager->AddItem(GURL(url), ui::PAGE_TRANSITION_LINK);
-    test_navigation_manager->SetLastCommittedItem(
-        test_navigation_manager->GetItemAtIndex(
-            test_navigation_manager->GetLastCommittedItemIndex()));
+    web::NavigationItem* item =
+        InsertItemToTestNavigationManager(test_navigation_manager.get(), url);
+    test_navigation_manager->SetLastCommittedItem(item);
 
     auto test_web_state = std::make_unique<web::TestWebState>();
     test_web_state->SetNavigationManager(std::move(test_navigation_manager));
@@ -76,6 +75,16 @@ class TabUsageRecorderTest : public PlatformTest {
         web_state_list_.GetWebStateAt(insertion_index));
   }
 
+  web::NavigationItem* InsertItemToTestNavigationManager(
+      web::TestNavigationManager* test_navigation_manager,
+      const char* url) {
+    test_navigation_manager->AddItem(GURL(), ui::PAGE_TRANSITION_LINK);
+    web::NavigationItem* item = test_navigation_manager->GetItemAtIndex(
+        test_navigation_manager->GetLastCommittedItemIndex());
+    item->SetVirtualURL(GURL(url));
+    return item;
+  }
+
   void AddTimeToDequeInTabUsageRecorder(base::TimeTicks time) {
     tab_usage_recorder_.termination_timestamps_.push_back(time);
   }
@@ -138,6 +147,7 @@ TEST_F(TabUsageRecorderTest, CountPageLoadsBeforeEvictedTab) {
                                        kNumReloads, 1);
 }
 
+// Tests that chrome:// URLs are not counted in page load stats.
 TEST_F(TabUsageRecorderTest, CountNativePageLoadsBeforeEvictedTab) {
   web::TestWebState* mock_tab_a = InsertTestWebState(kNativeURL, IN_MEMORY);
   web::TestWebState* mock_tab_b = InsertTestWebState(kNativeURL, NOT_IN_MEMORY);
@@ -147,10 +157,29 @@ TEST_F(TabUsageRecorderTest, CountNativePageLoadsBeforeEvictedTab) {
   for (int i = 0; i < kNumReloads; i++) {
     tab_usage_recorder_.RecordPageLoadStart(mock_tab_a);
   }
+
   tab_usage_recorder_.RecordTabSwitched(mock_tab_a, mock_tab_b);
   histogram_tester_.ExpectTotalCount(kPageLoadsBeforeEvictedTabSelected, 0);
 }
 
+// Tests that page load stats is not updated for an evicted tab that has a
+// pending chrome:// URL.
+TEST_F(TabUsageRecorderTest, CountPendingNativePageLoadBeforeEvictedTab) {
+  web::TestWebState* old_tab = InsertTestWebState(kURL, IN_MEMORY);
+  web::TestWebState* new_evicted_tab = InsertTestWebState(kURL, NOT_IN_MEMORY);
+
+  tab_usage_recorder_.RecordPageLoadStart(old_tab);
+
+  auto* test_navigation_manager = static_cast<web::TestNavigationManager*>(
+      new_evicted_tab->GetNavigationManager());
+  web::NavigationItem* item =
+      InsertItemToTestNavigationManager(test_navigation_manager, kNativeURL);
+  test_navigation_manager->SetPendingItem(item);
+
+  tab_usage_recorder_.RecordTabSwitched(old_tab, new_evicted_tab);
+  histogram_tester_.ExpectTotalCount(kPageLoadsBeforeEvictedTabSelected, 0);
+}
+
 TEST_F(TabUsageRecorderTest, TestColdStartTabs) {
   web::TestWebState* mock_tab_a = InsertTestWebState(kURL, NOT_IN_MEMORY);
   web::TestWebState* mock_tab_b = InsertTestWebState(kURL, NOT_IN_MEMORY);