[CrossThreadHandle] Convert third_party/blink/renderer/core/paint/paint_timing.cc [chromium/src : main]

0 visualização
Pular para a primeira mensagem não lida

Ari Chivukula (Gerrit)

não lida,
22 de nov. de 2022, 07:38:3722/11/2022
para blink-rev...@chromium.org, blink-...@chromium.org, Yoav Weiss, chromium...@chromium.org

Attention is currently required from: Yoav Weiss.

Patch set 1:Auto-Submit +1Commit-Queue +1

View Change

    To view, visit change 4047900. To unsubscribe, or for help writing mail filters, visit settings.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I18467b519da52ac14aa4adc4eb2bb976665fb7a0
    Gerrit-Change-Number: 4047900
    Gerrit-PatchSet: 1
    Gerrit-Owner: Ari Chivukula <ari...@chromium.org>
    Gerrit-Reviewer: Ari Chivukula <ari...@chromium.org>
    Gerrit-Reviewer: Yoav Weiss <yoav...@chromium.org>
    Gerrit-Attention: Yoav Weiss <yoav...@chromium.org>
    Gerrit-Comment-Date: Tue, 22 Nov 2022 12:37:00 +0000
    Gerrit-HasComments: No
    Gerrit-Has-Labels: Yes
    Gerrit-MessageType: comment

    Yoav Weiss (Gerrit)

    não lida,
    22 de nov. de 2022, 22:42:1922/11/2022
    para Ari Chivukula, blink-rev...@chromium.org, blink-...@chromium.org, Chromium LUCI CQ, chromium...@chromium.org

    Attention is currently required from: Ari Chivukula.

    View Change

    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.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I18467b519da52ac14aa4adc4eb2bb976665fb7a0
    Gerrit-Change-Number: 4047900
    Gerrit-PatchSet: 1
    Gerrit-Owner: Ari Chivukula <ari...@chromium.org>
    Gerrit-Reviewer: Ari Chivukula <ari...@chromium.org>
    Gerrit-Reviewer: Yoav Weiss <yoav...@chromium.org>
    Gerrit-Attention: Ari Chivukula <ari...@chromium.org>
    Gerrit-Comment-Date: Wed, 23 Nov 2022 03:39:00 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Gerrit-MessageType: comment

    Ari Chivukula (Gerrit)

    não lida,
    23 de nov. de 2022, 07:15:2823/11/2022
    para blink-rev...@chromium.org, blink-...@chromium.org, Chromium LUCI CQ, Yoav Weiss, chromium...@chromium.org

    Attention is currently required from: Yoav Weiss.

    View Change

    1 comment:

    • File third_party/blink/renderer/core/paint/timing/paint_timing.cc:

      • 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.

    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I18467b519da52ac14aa4adc4eb2bb976665fb7a0
    Gerrit-Change-Number: 4047900
    Gerrit-PatchSet: 1
    Gerrit-Owner: Ari Chivukula <ari...@chromium.org>
    Gerrit-Reviewer: Ari Chivukula <ari...@chromium.org>
    Gerrit-Reviewer: Yoav Weiss <yoav...@chromium.org>
    Gerrit-Attention: Yoav Weiss <yoav...@chromium.org>
    Gerrit-Comment-Date: Wed, 23 Nov 2022 12:12:42 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    Comment-In-Reply-To: Yoav Weiss <yoav...@chromium.org>
    Gerrit-MessageType: comment

    Yoav Weiss (Gerrit)

    não lida,
    25 de nov. de 2022, 04:35:3725/11/2022
    para Ari Chivukula, blink-rev...@chromium.org, blink-...@chromium.org, Chromium LUCI CQ, chromium...@chromium.org

    Attention is currently required from: Ari Chivukula.

    Patch set 1:Code-Review +1Commit-Queue +2

    View Change

      To view, visit change 4047900. To unsubscribe, or for help writing mail filters, visit settings.

      Gerrit-Project: chromium/src
      Gerrit-Branch: main
      Gerrit-Change-Id: I18467b519da52ac14aa4adc4eb2bb976665fb7a0
      Gerrit-Change-Number: 4047900
      Gerrit-PatchSet: 1
      Gerrit-Owner: Ari Chivukula <ari...@chromium.org>
      Gerrit-Reviewer: Ari Chivukula <ari...@chromium.org>
      Gerrit-Reviewer: Yoav Weiss <yoav...@chromium.org>
      Gerrit-Attention: Ari Chivukula <ari...@chromium.org>
      Gerrit-Comment-Date: Fri, 25 Nov 2022 09:32:18 +0000

      Ari Chivukula (Gerrit)

      não lida,
      25 de nov. de 2022, 14:12:2025/11/2022
      para blink-rev...@chromium.org, blink-...@chromium.org, Yoav Weiss, Chromium LUCI CQ, chromium...@chromium.org

      Patch set 2:Commit-Queue +2

      View Change

        To view, visit change 4047900. To unsubscribe, or for help writing mail filters, visit settings.

        Gerrit-Project: chromium/src
        Gerrit-Branch: main
        Gerrit-Change-Id: I18467b519da52ac14aa4adc4eb2bb976665fb7a0
        Gerrit-Change-Number: 4047900
        Gerrit-PatchSet: 2
        Gerrit-Owner: Ari Chivukula <ari...@chromium.org>
        Gerrit-Reviewer: Ari Chivukula <ari...@chromium.org>
        Gerrit-Reviewer: Yoav Weiss <yoav...@chromium.org>
        Gerrit-Comment-Date: Fri, 25 Nov 2022 19:10:19 +0000

        Ari Chivukula (Gerrit)

        não lida,
        25 de nov. de 2022, 16:41:2525/11/2022
        para blink-rev...@chromium.org, blink-...@chromium.org, Yoav Weiss, Chromium LUCI CQ, chromium...@chromium.org

        Attention is currently required from: Ari Chivukula.

        Patch set 2:Commit-Queue +2

        View Change

          To view, visit change 4047900. To unsubscribe, or for help writing mail filters, visit settings.

          Gerrit-Project: chromium/src
          Gerrit-Branch: main
          Gerrit-Change-Id: I18467b519da52ac14aa4adc4eb2bb976665fb7a0
          Gerrit-Change-Number: 4047900
          Gerrit-PatchSet: 2
          Gerrit-Owner: Ari Chivukula <ari...@chromium.org>
          Gerrit-Reviewer: Ari Chivukula <ari...@chromium.org>
          Gerrit-Reviewer: Yoav Weiss <yoav...@chromium.org>
          Gerrit-Attention: Ari Chivukula <ari...@chromium.org>
          Gerrit-Comment-Date: Fri, 25 Nov 2022 21:39:26 +0000

          Ari Chivukula (Gerrit)

          não lida,
          25 de nov. de 2022, 23:20:1025/11/2022
          para blink-rev...@chromium.org, blink-...@chromium.org, Yoav Weiss, Chromium LUCI CQ, chromium...@chromium.org

          Patch set 3:Auto-Submit +1Commit-Queue +2

          View Change

            To view, visit change 4047900. To unsubscribe, or for help writing mail filters, visit settings.

            Gerrit-Project: chromium/src
            Gerrit-Branch: main
            Gerrit-Change-Id: I18467b519da52ac14aa4adc4eb2bb976665fb7a0
            Gerrit-Change-Number: 4047900
            Gerrit-PatchSet: 3
            Gerrit-Owner: Ari Chivukula <ari...@chromium.org>
            Gerrit-Reviewer: Ari Chivukula <ari...@chromium.org>
            Gerrit-Reviewer: Yoav Weiss <yoav...@chromium.org>
            Gerrit-Comment-Date: Sat, 26 Nov 2022 04:17:37 +0000

            Ari Chivukula (Gerrit)

            não lida,
            26 de nov. de 2022, 08:02:0126/11/2022
            para blink-rev...@chromium.org, blink-...@chromium.org, Yoav Weiss, Chromium LUCI CQ, chromium...@chromium.org

            Attention is currently required from: Ari Chivukula.

            Patch set 3:Commit-Queue +2

            View Change

              To view, visit change 4047900. To unsubscribe, or for help writing mail filters, visit settings.

              Gerrit-Project: chromium/src
              Gerrit-Branch: main
              Gerrit-Change-Id: I18467b519da52ac14aa4adc4eb2bb976665fb7a0
              Gerrit-Change-Number: 4047900
              Gerrit-PatchSet: 3
              Gerrit-Owner: Ari Chivukula <ari...@chromium.org>
              Gerrit-Reviewer: Ari Chivukula <ari...@chromium.org>
              Gerrit-Reviewer: Yoav Weiss <yoav...@chromium.org>
              Gerrit-Attention: Ari Chivukula <ari...@chromium.org>
              Gerrit-Comment-Date: Sat, 26 Nov 2022 13:00:04 +0000

              Ari Chivukula (Gerrit)

              não lida,
              26 de nov. de 2022, 12:14:3326/11/2022
              para blink-rev...@chromium.org, blink-...@chromium.org, Yoav Weiss, Chromium LUCI CQ, chromium...@chromium.org

              Patch set 4:Auto-Submit +1Commit-Queue +2

              View Change

                To view, visit change 4047900. To unsubscribe, or for help writing mail filters, visit settings.

                Gerrit-Project: chromium/src
                Gerrit-Branch: main
                Gerrit-Change-Id: I18467b519da52ac14aa4adc4eb2bb976665fb7a0
                Gerrit-Change-Number: 4047900
                Gerrit-PatchSet: 4
                Gerrit-Owner: Ari Chivukula <ari...@chromium.org>
                Gerrit-Reviewer: Ari Chivukula <ari...@chromium.org>
                Gerrit-Reviewer: Yoav Weiss <yoav...@chromium.org>
                Gerrit-Comment-Date: Sat, 26 Nov 2022 17:12:53 +0000

                Chromium LUCI CQ (Gerrit)

                não lida,
                26 de nov. de 2022, 12:17:1526/11/2022
                para Ari Chivukula, blink-rev...@chromium.org, blink-...@chromium.org, Yoav Weiss, chromium...@chromium.org

                Chromium LUCI CQ submitted this change.

                View Change



                1 is the latest approved patch-set.
                No files were changed between the latest approved patch-set and the submitted one.

                Approvals: Ari Chivukula: Send CL to CQ automatically after approval; Commit Yoav Weiss: Looks good to me
                [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.

                Gerrit-Project: chromium/src
                Gerrit-Branch: main
                Gerrit-Change-Id: I18467b519da52ac14aa4adc4eb2bb976665fb7a0
                Gerrit-Change-Number: 4047900
                Gerrit-PatchSet: 5
                Gerrit-Owner: Ari Chivukula <ari...@chromium.org>
                Gerrit-Reviewer: Ari Chivukula <ari...@chromium.org>
                Gerrit-Reviewer: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>
                Gerrit-Reviewer: Yoav Weiss <yoav...@chromium.org>
                Gerrit-MessageType: merged
                Responder a todos
                Responder ao autor
                Encaminhar
                0 nova mensagem