[soft navs] Assume that tab w/ softnavs was in the foreground once [chromium/src : main]
0 views
Skip to first unread message
Johannes Henkel (Gerrit)
12:51 PM (3 hours ago) 12:51 PM
to Scott Haseley, Annie Sullivan, Chromium LUCI CQ, chromium...@chromium.org, bmcquad...@chromium.org, core-web-vita...@chromium.org, csharris...@chromium.org, loading-rev...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org
Attention needed from Annie Sullivan and Scott Haseley
New activity on the change![]( "Open in Gerrit")
Related details
Attention is currently required from:
- Annie Sullivan
- Scott Haseley
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Annie Sullivan (Gerrit)
12:58 PM (3 hours ago) 12:58 PM
to Johannes Henkel, Scott Haseley, Chromium LUCI CQ, chromium...@chromium.org, bmcquad...@chromium.org, core-web-vita...@chromium.org, csharris...@chromium.org, loading-rev...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org
Attention needed from Johannes Henkel and Scott Haseley
Annie Sullivan voted and added 1 comment![]( "Open in Gerrit")
Votes added by Annie Sullivan
| Code-Review | +1 |
1 comment
Patchset-level comments
File-level comment, Patchset 1 (Latest):
Annie Sullivan . resolved
This seems reasonable to me, but I wouldn't be surprised if we do hit the DCHECK for some
Related details
Attention is currently required from:
- Johannes Henkel
- Scott Haseley
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Scott Haseley (Gerrit)
1:10 PM (3 hours ago) 1:10 PM
to Johannes Henkel, Annie Sullivan, Chromium LUCI CQ, chromium...@chromium.org, bmcquad...@chromium.org, core-web-vita...@chromium.org, csharris...@chromium.org, loading-rev...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org
Attention needed from Johannes Henkel
Scott Haseley added 1 comment![]( "Open in Gerrit")
File chrome/browser/page_load_metrics/observers/core/ukm_page_load_metrics_observer.cc
Line 789, Patchset 1 (Latest):
DCHECK(!last_time_shown_.is_null());Scott Haseley . unresolved
You could make this a CHECK(, NotFatalUntil) to see if this happens without crashing the browser. Problem with DCHECK is it's only enabled in debug builds and a small number of Canary builds. The recommendation is typically to use CHECKs, and making it NotFatal makes sense here bc it _shouldn't_ break anything, IIUC
Related details
Attention is currently required from:
- Johannes Henkel
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Johannes Henkel (Gerrit)
1:25 PM (3 hours ago) 1:25 PM
to Annie Sullivan, Scott Haseley, Chromium LUCI CQ, chromium...@chromium.org, bmcquad...@chromium.org, core-web-vita...@chromium.org, csharris...@chromium.org, loading-rev...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org
Attention needed from Scott Haseley
Johannes Henkel added 2 comments![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 1:
Johannes Henkel . resolved
Thanks a lot, I switched to the non-fatal thingy.
I'm thinking if it triggers very infrequently, it should be ok-ish to remove the check in a little while.
File chrome/browser/page_load_metrics/observers/core/ukm_page_load_metrics_observer.cc
You could make this a CHECK(, NotFatalUntil) to see if this happens without crashing the browser. Problem with DCHECK is it's only enabled in debug builds and a small number of Canary builds. The recommendation is typically to use CHECKs, and making it NotFatal makes sense here bc it _shouldn't_ break anything, IIUC
Attention is currently required from:
- Scott Haseley
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Johannes Henkel (Gerrit)
1:28 PM (3 hours ago) 1:28 PM
to Annie Sullivan, Scott Haseley, Chromium LUCI CQ, chromium...@chromium.org, bmcquad...@chromium.org, core-web-vita...@chromium.org, csharris...@chromium.org, loading-rev...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org
Attention needed from Scott Haseley
Johannes Henkel voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
Attention is currently required from:
- Scott Haseley
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Scott Haseley (Gerrit)
1:28 PM (3 hours ago) 1:28 PM
to Johannes Henkel, Annie Sullivan, Chromium LUCI CQ, chromium...@chromium.org, bmcquad...@chromium.org, core-web-vita...@chromium.org, csharris...@chromium.org, loading-rev...@chromium.org, speed-metrics...@chromium.org, speed-metr...@chromium.org
Attention needed from Johannes Henkel
Scott Haseley voted and added 1 comment![]( "Open in Gerrit")
Votes added by Scott Haseley
| Code-Review | +1 |
1 comment
Patchset-level comments
Related details
Attention is currently required from:
- Johannes Henkel
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages