Group preview navigation log with decision logs.

Group navigation logs with the corresponding decisions message of the
navigation.

Push the group message to the top of the table when there's a new message in
that group. Since navigation logs could be called called much later compared to
decision logs, the message group need to float to the top of the table when new
messages are added to the group.

Bug: 791778
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Ic872fd155ca25a35286bafaaffde41c2b65a01d4
Reviewed-on: https://chromium-review.googlesource.com/809936
Commit-Queue: Thanh Le <thanhdle@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522230}

chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_io_data.cc

@@ -11,6 +11,7 @@
 #include "build/build_config.h"
 #include "chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings.h"
 #include "chrome/browser/previews/previews_infobar_delegate.h"
+#include "chrome/browser/previews/previews_infobar_tab_helper.h"
 #include "chrome/browser/previews/previews_service.h"
 #include "chrome/browser/previews/previews_service_factory.h"
 #include "chrome/browser/profiles/profile.h"
@@ -46,12 +47,13 @@ void AddPreviewNavigationToBlackListCallback(
     content::BrowserContext* browser_context,
     const GURL& url,
     previews::PreviewsType type,
+    uint64_t page_id,
     bool opt_out) {
   PreviewsService* previews_service = PreviewsServiceFactory::GetForProfile(
       Profile::FromBrowserContext(browser_context));
   if (previews_service && previews_service->previews_ui_service()) {
-    previews_service->previews_ui_service()->AddPreviewNavigation(url, type,
-                                                                  opt_out);
+    previews_service->previews_ui_service()->AddPreviewNavigation(
+        url, type, opt_out, page_id);
   }
 }
 
@@ -66,6 +68,14 @@ void OnLoFiResponseReceivedOnUI(content::WebContents* web_contents) {
   previews::PreviewsUIService* previews_ui_service =
       previews_service ? previews_service->previews_ui_service() : nullptr;
 
+  PreviewsInfoBarTabHelper* infobar_tab_helper =
+      PreviewsInfoBarTabHelper::FromWebContents(web_contents);
+
+  uint64_t page_id = 0;
+  if (infobar_tab_helper && infobar_tab_helper->previews_user_data()) {
+    page_id = infobar_tab_helper->previews_user_data()->page_id();
+  }
+
   PreviewsInfoBarDelegate::Create(
       web_contents, previews::PreviewsType::LOFI,
       base::Time() /* previews_freshness */, true /* is_data_saver_user */,
@@ -75,11 +85,11 @@ void OnLoFiResponseReceivedOnUI(content::WebContents* web_contents) {
                  web_contents->GetController()
                      .GetLastCommittedEntry()
                      ->GetRedirectChain()[0],
-                 previews::PreviewsType::LOFI),
+                 previews::PreviewsType::LOFI, page_id),
       previews_ui_service);
 }
 
-} // namespace
+}  // namespace
 
 std::unique_ptr<data_reduction_proxy::DataReductionProxyIOData>
 CreateDataReductionProxyChromeIOData(

chrome/browser/previews/previews_infobar_delegate.cc

@@ -137,6 +137,9 @@ void PreviewsInfoBarDelegate::Create(
 #endif
 
   infobar_service->AddInfoBar(std::move(infobar_ptr));
+  uint64_t page_id = (infobar_tab_helper->previews_user_data())
+                         ? infobar_tab_helper->previews_user_data()->page_id()
+                         : 0;
 
   if (previews_ui_service) {
     // Not in incognito mode or guest mode.
@@ -146,7 +149,7 @@ void PreviewsInfoBarDelegate::Create(
         web_contents->GetController()
             .GetLastCommittedEntry()
             ->GetRedirectChain()[0] /* GURL */,
-        base::Time::Now(), 0 /* page_id */);
+        base::Time::Now(), page_id);
   }
 
   RecordPreviewsInfoBarAction(previews_type, INFOBAR_SHOWN);

chrome/browser/previews/previews_infobar_tab_helper.cc

@@ -37,12 +37,13 @@ namespace {
 void AddPreviewNavigationCallback(content::BrowserContext* browser_context,
                                   const GURL& url,
                                   previews::PreviewsType type,
+                                  uint64_t page_id,
                                   bool opt_out) {
   PreviewsService* previews_service = PreviewsServiceFactory::GetForProfile(
       Profile::FromBrowserContext(browser_context));
   if (previews_service && previews_service->previews_ui_service()) {
-    previews_service->previews_ui_service()->AddPreviewNavigation(url, type,
-                                                                  opt_out);
+    previews_service->previews_ui_service()->AddPreviewNavigation(
+        url, type, opt_out, page_id);
   }
 }
 
@@ -75,6 +76,8 @@ void PreviewsInfoBarTabHelper::DidFinishNavigation(
     previews_user_data_ = nav_data->previews_user_data()->DeepCopy();
   }
 
+  uint64_t page_id = (previews_user_data_) ? previews_user_data_->page_id() : 0;
+
   // The infobar should only be told if the page was a reload if the previous
   // page displayed a timestamp.
   bool is_reload =
@@ -111,7 +114,7 @@ void PreviewsInfoBarTabHelper::DidFinishNavigation(
         base::Bind(&AddPreviewNavigationCallback,
                    web_contents()->GetBrowserContext(),
                    navigation_handle->GetRedirectChain()[0],
-                   previews::PreviewsType::OFFLINE),
+                   previews::PreviewsType::OFFLINE, page_id),
         previews_ui_service);
     // Don't try to show other infobars if this is an offline preview.
     return;
@@ -133,7 +136,7 @@ void PreviewsInfoBarTabHelper::DidFinishNavigation(
         base::Bind(&AddPreviewNavigationCallback,
                    web_contents()->GetBrowserContext(),
                    navigation_handle->GetRedirectChain()[0],
-                   previews::PreviewsType::LITE_PAGE),
+                   previews::PreviewsType::LITE_PAGE, page_id),
         previews_ui_service);
     return;
   }
@@ -148,7 +151,7 @@ void PreviewsInfoBarTabHelper::DidFinishNavigation(
           base::Bind(&AddPreviewNavigationCallback,
                      web_contents()->GetBrowserContext(),
                      navigation_handle->GetRedirectChain()[0],
-                     previews::PreviewsType::NOSCRIPT),
+                     previews::PreviewsType::NOSCRIPT, page_id),
           previews_ui_service);
     }
   }

chrome/browser/resources/interventions_internals/index.js

@@ -57,6 +57,43 @@ function addMoreDetailsButton(element, pageId) {
   });
 }
 
+/**
+ * Helper method to move a row to the top of a html table, below the header
+ * row.
+ * @param {!HTMLElement} row The row to move.
+ * @param {!HTMLElement} table The table to move.
+ */
+function pushRowToTopOfLogsTable(row, table) {
+  let newRow = table.insertRow(1);
+  newRow.className = row.className;
+  newRow.id = row.id;
+  newRow.innerHTML = row.innerHTML;
+  row.remove();
+}
+
+/**
+ * Helper method to move a group of messages to the top of the Logs Table,
+ * including the expansion row corresponding to the |pageId|.
+ *
+ * @param {number} pageId The key of |logTableMap| of the moving row.
+ */
+function pushMessagesToTopOfLogsTable(pageId) {
+  let logsTable = $('message-logs-table');
+  let currentMessageRow = window.logTableMap[pageId];
+
+  // Moving empty row.
+  let emptyRow = logsTable.rows[currentMessageRow.rowIndex + 2];
+  pushRowToTopOfLogsTable(emptyRow, logsTable);
+
+  // Moving expansion row.
+  let expansionRow = logsTable.rows[currentMessageRow.rowIndex + 1];
+  pushRowToTopOfLogsTable(expansionRow, logsTable);
+
+  // Moving the original row.
+  pushRowToTopOfLogsTable(currentMessageRow, logsTable);
+  window.logTableMap[pageId] = logsTable.rows[1];
+}
+
 /**
  * Update the |pageId| log message group. Copy the main row that contains the
  * most updated log message of the group to the expansion row, and update the
@@ -70,6 +107,8 @@ function addMoreDetailsButton(element, pageId) {
 function updateTableRowByPageId(time, type, description, url, pageId) {
   assert(pageId > 0);
   assert(window.logTableMap[pageId]);
+  pushMessagesToTopOfLogsTable(pageId);
+
   let currentRow = window.logTableMap[pageId];
   let expansionRow = $('expansion-row-' + pageId);
   let newRow = expansionRow.querySelector('.expansion-logs-table').insertRow(0);
@@ -230,9 +269,10 @@ function setupTabControl() {
 function setupLogSearch() {
   $('log-search-bar').addEventListener('keyup', () => {
     let keyword = $('log-search-bar').value.toUpperCase();
-    let rows = document.querySelectorAll('.log-message');
+    let rows = $('message-logs-table').rows;
 
-    rows.forEach((row) => {
+    for (let i = 1; i < rows.length; i++) {
+      let row = rows[i];
       let found = KEY_COLUMNS.some((column) => {
         let cell = row.querySelector('.' + column);
         if (!cell) {
@@ -241,7 +281,7 @@ function setupLogSearch() {
         return cell.textContent.toUpperCase().includes(keyword);
       });
       row.style.display = found ? '' : 'none';
-    });
+    }
   });
 }
 

chrome/test/data/webui/interventions_internals_browsertest.js

@@ -455,6 +455,57 @@ TEST_F(
       mocha.run();
     });
 
+TEST_F(
+    'InterventionsInternalsUITest',
+    'LogNewMessageExistedPageIdGroupToTopOfTable', function() {
+      test('NewMessagePushedToTopOfTable', () => {
+        let pageImpl = new InterventionsInternalPageImpl(null);
+        let logs = [
+          {
+            type: 'Type_a',
+            description: 'Some description_a',
+            url: {url: 'Some gurl.spec()_a'},
+            time: 0,
+            pageId: 3,
+          },
+          {
+            type: 'Type_b',
+            description: 'Some description_b',
+            url: {url: 'Some gurl.spec()_b'},
+            time: 1,
+            pageId: 123,
+          },
+          {
+            type: 'Type_c',
+            description: 'Some description_c',
+            url: {url: 'Some gurl.spec()_c'},
+            time: 2,
+            pageId: 3,
+          },
+        ];
+
+        pageImpl.logNewMessage(logs[0]);
+        pageImpl.logNewMessage(logs[1]);
+        let rows = $('message-logs-table').querySelectorAll('.log-message');
+        expectEquals(2, rows.length);
+        expectEquals(
+            logs[1].type, rows[0].querySelector('.log-type').textContent);
+        expectEquals(
+            logs[0].type, rows[1].querySelector('.log-type').textContent);
+
+        // Existing group pushed to the top of the log table.
+        pageImpl.logNewMessage(logs[2]);
+        rows = $('message-logs-table').querySelectorAll('.log-message');
+        expectEquals(2, rows.length);
+        expectEquals(
+            logs[2].type, rows[0].querySelector('.log-type').textContent);
+        expectEquals(
+            logs[1].type, rows[1].querySelector('.log-type').textContent);
+      });
+
+      mocha.run();
+    });
+
 TEST_F('InterventionsInternalsUITest', 'AddNewBlacklistedHost', function() {
   test('AddNewBlacklistedHost', () => {
     let pageImpl = new InterventionsInternalPageImpl(null);

components/previews/content/previews_io_data.cc

@@ -148,10 +148,12 @@ void PreviewsIOData::SetPreviewsBlacklistForTesting(
 void PreviewsIOData::LogPreviewNavigation(const GURL& url,
                                           bool opt_out,
                                           PreviewsType type,
-                                          base::Time time) const {
+                                          base::Time time,
+                                          uint64_t page_id) const {
   ui_task_runner_->PostTask(
-      FROM_HERE, base::Bind(&PreviewsUIService::LogPreviewNavigation,
-                            previews_ui_service_, url, type, opt_out, time));
+      FROM_HERE,
+      base::BindOnce(&PreviewsUIService::LogPreviewNavigation,
+                     previews_ui_service_, url, type, opt_out, time, page_id));
 }
 
 void PreviewsIOData::LogPreviewDecisionMade(
@@ -170,11 +172,12 @@ void PreviewsIOData::LogPreviewDecisionMade(
 
 void PreviewsIOData::AddPreviewNavigation(const GURL& url,
                                           bool opt_out,
-                                          PreviewsType type) {
+                                          PreviewsType type,
+                                          uint64_t page_id) {
   DCHECK(io_task_runner_->BelongsToCurrentThread());
   base::Time time =
       previews_black_list_->AddPreviewNavigation(url, opt_out, type);
-  LogPreviewNavigation(url, opt_out, type, time);
+  LogPreviewNavigation(url, opt_out, type, time, page_id);
 }
 
 void PreviewsIOData::ClearBlackList(base::Time begin_time,

components/previews/content/previews_io_data.h

@@ -65,7 +65,8 @@ class PreviewsIOData : public PreviewsDecider,
   void LogPreviewNavigation(const GURL& url,
                             bool opt_out,
                             PreviewsType type,
-                            base::Time time) const;
+                            base::Time time,
+                            uint64_t page_id) const;
 
   // Adds log message of preview decision made asynchronously. |passed_reasons|
   // are PreviewsEligibilityReasons that got passed the decision before
@@ -81,7 +82,10 @@ class PreviewsIOData : public PreviewsDecider,
       uint64_t page_id) const;
 
   // Adds a navigation to |url| to the black list with result |opt_out|.
-  void AddPreviewNavigation(const GURL& url, bool opt_out, PreviewsType type);
+  void AddPreviewNavigation(const GURL& url,
+                            bool opt_out,
+                            PreviewsType type,
+                            uint64_t page_id);
 
   // Clears the history of the black list between |begin_time| and |end_time|,
   // both inclusive.

components/previews/content/previews_io_data_unittest.cc

@@ -193,6 +193,9 @@ class TestPreviewsUIService : public PreviewsUIService {
   const std::vector<PreviewsType>& navigation_types() const {
     return navigation_types_;
   }
+  const std::vector<uint64_t>& navigation_page_ids() const {
+    return navigation_page_ids_;
+  }
 
   // Expose passed in params for hosts and user blacklist event.
   std::string host_blacklisted() const { return host_blacklisted_; }
@@ -208,11 +211,13 @@ class TestPreviewsUIService : public PreviewsUIService {
   void LogPreviewNavigation(const GURL& url,
                             PreviewsType type,
                             bool opt_out,
-                            base::Time time) override {
+                            base::Time time,
+                            uint64_t page_id) override {
     navigation_urls_.push_back(url);
     navigation_opt_outs_.push_back(opt_out);
     navigation_types_.push_back(type);
     navigation_times_.push_back(time);
+    navigation_page_ids_.push_back(page_id);
   }
 
   void LogPreviewDecisionMade(
@@ -249,6 +254,7 @@ class TestPreviewsUIService : public PreviewsUIService {
   std::vector<bool> navigation_opt_outs_;
   std::vector<base::Time> navigation_times_;
   std::vector<PreviewsType> navigation_types_;
+  std::vector<uint64_t> navigation_page_ids_;
 
   // Whether the blacklist decisions are ignored or not.
   bool blacklist_ignored_;
@@ -758,12 +764,13 @@ TEST_F(PreviewsIODataTest, NoScriptAllowedByFeatureWithWhitelist) {
 
 TEST_F(PreviewsIODataTest, LogPreviewNavigationPassInCorrectParams) {
   InitializeUIService();
-  GURL url("http://www.url_a.com/url_a");
-  bool opt_out = true;
-  PreviewsType type = PreviewsType::OFFLINE;
-  base::Time time = base::Time::Now();
+  const GURL url("http://www.url_a.com/url_a");
+  const bool opt_out = true;
+  const PreviewsType type = PreviewsType::OFFLINE;
+  const base::Time time = base::Time::Now();
+  const uint64_t page_id = 1234;
 
-  io_data()->LogPreviewNavigation(url, opt_out, type, time);
+  io_data()->LogPreviewNavigation(url, opt_out, type, time, page_id);
   base::RunLoop().RunUntilIdle();
 
   EXPECT_THAT(ui_service()->navigation_urls(), ::testing::ElementsAre(url));
@@ -771,6 +778,8 @@ TEST_F(PreviewsIODataTest, LogPreviewNavigationPassInCorrectParams) {
               ::testing::ElementsAre(opt_out));
   EXPECT_THAT(ui_service()->navigation_types(), ::testing::ElementsAre(type));
   EXPECT_THAT(ui_service()->navigation_times(), ::testing::ElementsAre(time));
+  EXPECT_THAT(ui_service()->navigation_page_ids(),
+              ::testing::ElementsAre(page_id));
 }
 
 TEST_F(PreviewsIODataTest, LogPreviewDecisionMadePassInCorrectParams) {

components/previews/content/previews_ui_service.cc

@@ -37,19 +37,21 @@ void PreviewsUIService::SetIOData(base::WeakPtr<PreviewsIOData> io_data) {
 
 void PreviewsUIService::AddPreviewNavigation(const GURL& url,
                                              PreviewsType type,
-                                             bool opt_out) {
+                                             bool opt_out,
+                                             uint64_t page_id) {
   DCHECK(thread_checker_.CalledOnValidThread());
   io_task_runner_->PostTask(
       FROM_HERE, base::Bind(&PreviewsIOData::AddPreviewNavigation, io_data_,
-                            url, opt_out, type));
+                            url, opt_out, type, page_id));
 }
 
 void PreviewsUIService::LogPreviewNavigation(const GURL& url,
                                              PreviewsType type,
                                              bool opt_out,
-                                             base::Time time) {
+                                             base::Time time,
+                                             uint64_t page_id) {
   DCHECK(thread_checker_.CalledOnValidThread());
-  logger_->LogPreviewNavigation(url, type, opt_out, time);
+  logger_->LogPreviewNavigation(url, type, opt_out, time, page_id);
 }
 
 void PreviewsUIService::LogPreviewDecisionMade(

components/previews/content/previews_ui_service.h

@@ -46,7 +46,10 @@ class PreviewsUIService {
   virtual void SetIOData(base::WeakPtr<PreviewsIOData> io_data);
 
   // Adds a navigation to |url| to the black list with result |opt_out|.
-  void AddPreviewNavigation(const GURL& url, PreviewsType type, bool opt_out);
+  void AddPreviewNavigation(const GURL& url,
+                            PreviewsType type,
+                            bool opt_out,
+                            uint64_t page_id);
 
   // Clears the history of the black list between |begin_time| and |end_time|.
   void ClearBlackList(base::Time begin_time, base::Time end_time);
@@ -79,7 +82,8 @@ class PreviewsUIService {
   virtual void LogPreviewNavigation(const GURL& url,
                                     PreviewsType type,
                                     bool opt_out,
-                                    base::Time time);
+                                    base::Time time,
+                                    uint64_t page_id);
 
   // Log the made decision of previews to PreviewsLogger. |passed_reasons| is a
   // collection of PreviewsEligibilityReasons passed the checks before |reason|.

components/previews/content/previews_ui_service_unittest.cc

@@ -65,11 +65,13 @@ class TestPreviewsLogger : public PreviewsLogger {
   void LogPreviewNavigation(const GURL& url,
                             PreviewsType type,
                             bool opt_out,
-                            base::Time time) override {
+                            base::Time time,
+                            uint64_t page_id) override {
     navigation_url_ = url;
     navigation_opt_out_ = opt_out;
     navigation_type_ = type;
     navigation_time_ = base::Time(time);
+    navigation_page_id_ = page_id;
   }
 
   void LogPreviewDecisionMade(
@@ -120,6 +122,7 @@ class TestPreviewsLogger : public PreviewsLogger {
   bool navigation_opt_out() const { return navigation_opt_out_; }
   base::Time navigation_time() const { return navigation_time_; }
   PreviewsType navigation_type() const { return navigation_type_; }
+  uint64_t navigation_page_id() const { return navigation_page_id_; }
 
   // Return the passed in OnBlacklist events.
   std::string host_blacklisted() const { return host_blacklisted_; }
@@ -144,6 +147,7 @@ class TestPreviewsLogger : public PreviewsLogger {
   bool navigation_opt_out_;
   base::Time navigation_time_;
   PreviewsType navigation_type_;
+  uint64_t navigation_page_id_;
 
   // Passed in OnBlacklist events.
   std::string host_blacklisted_;
@@ -221,28 +225,34 @@ TEST_F(PreviewsUIServiceTest, TestInitialization) {
 
 TEST_F(PreviewsUIServiceTest, TestLogPreviewNavigationPassInCorrectParams) {
   const GURL url_a = GURL("http://www.url_a.com/url_a");
-  PreviewsType type_a = PreviewsType::LOFI;
-  bool opt_out_a = true;
-  base::Time time_a = base::Time::Now();
+  const PreviewsType type_a = PreviewsType::LOFI;
+  const bool opt_out_a = true;
+  const base::Time time_a = base::Time::Now();
+  const uint64_t page_id_a = 1234;
 
-  ui_service()->LogPreviewNavigation(url_a, type_a, opt_out_a, time_a);
+  ui_service()->LogPreviewNavigation(url_a, type_a, opt_out_a, time_a,
+                                     page_id_a);
 
   EXPECT_EQ(url_a, logger_ptr_->navigation_url());
   EXPECT_EQ(type_a, logger_ptr_->navigation_type());
   EXPECT_EQ(opt_out_a, logger_ptr_->navigation_opt_out());
   EXPECT_EQ(time_a, logger_ptr_->navigation_time());
+  EXPECT_EQ(page_id_a, logger_ptr_->navigation_page_id());
 
   const GURL url_b = GURL("http://www.url_b.com/url_b");
-  PreviewsType type_b = PreviewsType::OFFLINE;
-  bool opt_out_b = false;
-  base::Time time_b = base::Time::Now();
+  const PreviewsType type_b = PreviewsType::OFFLINE;
+  const bool opt_out_b = false;
+  const base::Time time_b = base::Time::Now();
+  const uint64_t page_id_b = 4321;
 
-  ui_service()->LogPreviewNavigation(url_b, type_b, opt_out_b, time_b);
+  ui_service()->LogPreviewNavigation(url_b, type_b, opt_out_b, time_b,
+                                     page_id_b);
 
   EXPECT_EQ(url_b, logger_ptr_->navigation_url());
   EXPECT_EQ(type_b, logger_ptr_->navigation_type());
   EXPECT_EQ(opt_out_b, logger_ptr_->navigation_opt_out());
   EXPECT_EQ(time_b, logger_ptr_->navigation_time());
+  EXPECT_EQ(page_id_b, logger_ptr_->navigation_page_id());
 }
 
 TEST_F(PreviewsUIServiceTest, TestLogPreviewDecisionMadePassesCorrectParams) {

components/previews/core/previews_logger.cc

@@ -169,12 +169,10 @@ void PreviewsLogger::LogMessage(const std::string& event_type,
 void PreviewsLogger::LogPreviewNavigation(const GURL& url,
                                           PreviewsType type,
                                           bool opt_out,
-                                          base::Time time) {
+                                          base::Time time,
+                                          uint64_t page_id) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK_GE(kMaximumNavigationLogs, navigations_logs_.size());
-  // TODO(thanhdle): Group navigation together with decisions logs.
-  // crbug.com/791778.
-  const uint64_t page_id = 0;  // Non grouped log message.
 
   std::string description = GetDescriptionForPreviewsNavigation(type, opt_out);
   LogMessage(kPreviewNavigationEventType, description, url, time, page_id);

components/previews/core/previews_logger.h

@@ -86,7 +86,8 @@ class PreviewsLogger {
   virtual void LogPreviewNavigation(const GURL& url,
                                     PreviewsType type,
                                     bool opt_out,
-                                    base::Time time);
+                                    base::Time time,
+                                    uint64_t page_id);
 
   // Add a MessageLog for the a decision that was made about the state of
   // previews and blacklist. |passed_reasons| is an ordered list of

components/previews/core/previews_logger_unittest.cc

@@ -219,13 +219,16 @@ TEST_F(PreviewsLoggerTest, LogPreviewNavigationLogMessage) {
   PreviewsType type_b = PreviewsType::LOFI;
   const GURL url_a("http://www.url_a.com/url_a");
   const GURL url_b("http://www.url_b.com/url_b");
-  const uint64_t expected_nav_id = 0;
+  const uint64_t page_id_a = 1234;
+  const uint64_t page_id_b = 1234;
 
   TestPreviewsLoggerObserver observer;
   logger_->AddAndNotifyObserver(&observer);
 
-  logger_->LogPreviewNavigation(url_a, type_a, true /* opt_out */, time);
-  logger_->LogPreviewNavigation(url_b, type_b, false /* opt_out */, time);
+  logger_->LogPreviewNavigation(url_a, type_a, true /* opt_out */, time,
+                                page_id_a);
+  logger_->LogPreviewNavigation(url_b, type_b, false /* opt_out */, time,
+                                page_id_b);
 
   auto actual = observer.messages();
 
@@ -237,14 +240,14 @@ TEST_F(PreviewsLoggerTest, LogPreviewNavigationLogMessage) {
   EXPECT_EQ(expected_description_a, actual[0].event_description);
   EXPECT_EQ(url_a, actual[0].url);
   EXPECT_EQ(time, actual[0].time);
-  EXPECT_EQ(expected_nav_id, actual[0].page_id);
+  EXPECT_EQ(page_id_a, actual[0].page_id);
 
   std::string expected_description_b = "LoFi preview - user opt-out: False";
   EXPECT_EQ(kPreviewsNavigationEventType, actual[1].event_type);
   EXPECT_EQ(expected_description_b, actual[1].event_description);
   EXPECT_EQ(url_b, actual[1].url);
   EXPECT_EQ(time, actual[1].time);
-  EXPECT_EQ(expected_nav_id, actual[1].page_id);
+  EXPECT_EQ(page_id_b, actual[1].page_id);
 }
 
 TEST_F(PreviewsLoggerTest, PreviewsLoggerOnlyKeepsCertainNumberOfDecisionLogs) {
@@ -271,9 +274,10 @@ TEST_F(PreviewsLoggerTest,
   PreviewsType type = PreviewsType::OFFLINE;
   const GURL url("http://www.url_.com/url_");
   const base::Time time = base::Time::Now();
+  const uint64_t page_id = 1234;
 
   for (size_t i = 0; i < 2 * kMaximumNavigationLogs; ++i) {
-    logger_->LogPreviewNavigation(url, type, true /* opt_out */, time);
+    logger_->LogPreviewNavigation(url, type, true /* opt_out */, time, page_id);
   }
 
   TestPreviewsLoggerObserver observer;
@@ -303,12 +307,13 @@ TEST_F(PreviewsLoggerTest,
       base::Time::FromJsTime(-413696806000),  // Same as above.
       base::Time::FromJsTime(758620800000),   // Jan 15 1994 08:00:00 UTC
   };
-  const uint64_t page_ids[] = {1233, 1233, 0 /* Navigation page_id */};
+  const uint64_t page_ids[] = {1233, 1233, 5678 /* Navigation page_id */};
 
   // Logging decisions and navigations events.
   logger_->LogPreviewDecisionMade(final_reason, urls[0], times[0], type,
                                   std::move(passed_reasons), page_ids[0]);
-  logger_->LogPreviewNavigation(urls[2], type, true /* opt_out */, times[2]);
+  logger_->LogPreviewNavigation(urls[2], type, true /* opt_out */, times[2],
+                                page_ids[2]);
 
   TestPreviewsLoggerObserver observer;
   logger_->AddAndNotifyObserver(&observer);