New issue
Advanced search Search tips

Issue 913280 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Dec 11
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Merging query time logging fix to M72

Project Member Reported by charleszhao@chromium.org, Dec 9

Issue description

The query time logging of TabDiscarder is a critical feature for us to train and evaluate ML model on tab discards on ChromeOS and Chrome. 
The initial code is landed in this change to 72.0.3626.0
https://chromium.googlesource.com/chromium/src/+/3e8927839919e847328389775c5df58b798c7fd3

But we immediately found a bug that will prevent us from getting any logging or evaluation for ChromeOS platform, which is essentially we have been focusing on for months.

A fix has been made and landed to 73.0.3634.0
https://chromium.googlesource.com/chromium/src/+/6378752b810e6122744cefac544c8d84278dcc71

We hope we can merge this change to M72, so that we can work on this feature from M72.

From safety point of view:
(1) the change is all about logging tab features and labels in the background; it won't cause much noticeable change for users.

(2) the added feature is behind a flag (TabRanker) and disabled as default.


From impact point of view:
Without this change, we basically can do nothing but wait for 6 more weeks for M73 to release.









 
Cc: cmtm@google.com
More details about this fix 6378752b810e6122744cefac544c8d84278dcc71.

These tree files are basically changed back to its state before 
3e8927839919e847328389775c5df58b798c7fd3 except one line change:
    resource_coordinator::TabActivityWatcher::GetInstance()
        ->LogOldestNTabFeatures();
To enable the logging on ChromeOS and Chrome.
chrome/browser/resource_coordinator/tab_lifecycle_unit.cc
chrome/browser/resource_coordinator/tab_manager.h
chrome/browser/resource_coordinator/tab_manager.cc


These two files are also changed back to its state before 
3e8927839919e847328389775c5df58b798c7fd3 except 
void LogOldestNTabFeatures() is added for logging and GetSortedWebContentsData() is added due to a small refactor.
chrome/browser/resource_coordinator/tab_activity_watcher.h
chrome/browser/resource_coordinator/tab_activity_watcher.cc
Project Member

Comment 3 by sheriffbot@chromium.org, Dec 10

Labels: -Merge-Request-72 Hotlist-Merge-Approved Merge-Approved-72
Your change meets the bar and is auto-approved for M72. Please go ahead and merge the CL to branch 3626 manually. Please contact milestone owner if you have questions.
Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: merge-merged-3626
Status: Verified (was: Untriaged)
Project Member

Comment 6 by sheriffbot@chromium.org, Dec 14

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 7 by sheriffbot@chromium.org, Dec 17

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Sign in to add a comment