Attention is currently required from: Yoav Weiss.
Patch set 1:Auto-Submit +1Commit-Queue +1
To view, visit change 4047900. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Ari Chivukula.
1 comment:
File third_party/blink/renderer/core/paint/timing/paint_timing.cc:
Patch Set #1, Line 272: MakeUnwrappingCrossThreadWeakHandle(this), event));
Thanks for cleaning up! It's not 100% clear to me why you're using the unwrapping variant here. Can you explain?
To view, visit change 4047900. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Yoav Weiss.
1 comment:
File third_party/blink/renderer/core/paint/timing/paint_timing.cc:
Patch Set #1, Line 272: MakeUnwrappingCrossThreadWeakHandle(this), event));
Thanks for cleaning up! It's not 100% clear to me why you're using the unwrapping variant here. […]
Sure! So if this was being passed to another thread directly, it would have to be a MakeCrossThreadHandle, so that the other thread wouldn't be able to use it except to pass to a callback on the originating thread.
In this instance, it's being passed directly to a callback that RegisterNotifyPresentationTime queues to be invoked on the originating thread, so we can safely package it as MakeUnwrappingCrossThreadHandle (weak in this instance) as this ensures the correct retention logic.
My understanding is that the cross thread binding library itself handles the unwrapping, so it's not literally being unwrapped here and as though it was passed in raw.
To view, visit change 4047900. To unsubscribe, or for help writing mail filters, visit settings.
Attention is currently required from: Ari Chivukula.
Patch set 1:Code-Review +1Commit-Queue +2
Patch set 2:Commit-Queue +2
Attention is currently required from: Ari Chivukula.
Patch set 2:Commit-Queue +2
Patch set 3:Auto-Submit +1Commit-Queue +2
Attention is currently required from: Ari Chivukula.
Patch set 3:Commit-Queue +2
Patch set 4:Auto-Submit +1Commit-Queue +2
Chromium LUCI CQ submitted this change.
1 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
[CrossThreadHandle] Convert third_party/blink/renderer/core/paint/paint_timing.cc
This is part of a cleanup task to make memory safer when data isn't used
cross thread, but is instead merely passed to be used back again on the
originating thread. Design doc:
https://docs.google.com/document/d/1GIT0ysdQ84sGhIo1r9EscF_fFt93lmNVM_q4vvHj2FQ/edit#
Bug: 1377345
Change-Id: I18467b519da52ac14aa4adc4eb2bb976665fb7a0
fuzzer infra failure
No-Try: true
Change-Id: I18467b519da52ac14aa4adc4eb2bb976665fb7a0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4047900
Commit-Queue: Ari Chivukula <ari...@chromium.org>
Reviewed-by: Yoav Weiss <yoav...@chromium.org>
Auto-Submit: Ari Chivukula <ari...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1075899}
---
M third_party/blink/renderer/core/paint/timing/paint_timing.cc
1 file changed, 28 insertions(+), 2 deletions(-)
diff --git a/third_party/blink/renderer/core/paint/timing/paint_timing.cc b/third_party/blink/renderer/core/paint/timing/paint_timing.cc
index b0e2200..7479f61 100644
--- a/third_party/blink/renderer/core/paint/timing/paint_timing.cc
+++ b/third_party/blink/renderer/core/paint/timing/paint_timing.cc
@@ -26,6 +26,7 @@
#include "third_party/blink/renderer/core/timing/performance_timing_for_reporting.h"
#include "third_party/blink/renderer/core/timing/window_performance.h"
#include "third_party/blink/renderer/platform/graphics/paint/ignore_paint_timing_scope.h"
+#include "third_party/blink/renderer/platform/heap/cross_thread_handle.h"
#include "third_party/blink/renderer/platform/instrumentation/resource_coordinator/document_resource_coordinator.h"
#include "third_party/blink/renderer/platform/instrumentation/tracing/trace_event.h"
#include "third_party/blink/renderer/platform/loader/fetch/resource_fetcher.h"
@@ -268,7 +269,7 @@
void PaintTiming::RegisterNotifyPresentationTime(PaintEvent event) {
RegisterNotifyPresentationTime(
CrossThreadBindOnce(&PaintTiming::ReportPresentationTime,
- WrapCrossThreadWeakPersistent(this), event));
+ MakeUnwrappingCrossThreadWeakHandle(this), event));
}
void PaintTiming::
@@ -277,7 +278,7 @@
RegisterNotifyPresentationTime(CrossThreadBindOnce(
&PaintTiming::
ReportFirstPaintAfterBackForwardCacheRestorePresentationTime,
- WrapCrossThreadWeakPersistent(this), index));
+ MakeUnwrappingCrossThreadWeakHandle(this), index));
}
void PaintTiming::RegisterNotifyPresentationTime(ReportTimeCallback callback) {
To view, visit change 4047900. To unsubscribe, or for help writing mail filters, visit settings.