New issue
Advanced search Search tips

Issue 900506 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 14
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 900508



Sign in to add a comment

Make ThreadableLoaderClient GarbageCollected

Project Member Reported by yhirano@chromium.org, Oct 31

Issue description

It's currently not GarbageCollected, because it should be, as we are going to deprecate it and ResourceClient is GarbageCollected.

I suspect this is the cause of issue 570946.
 
Blockedon: 900508
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 14

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/89797ba017745f20895099117706278ae305566d

commit 89797ba017745f20895099117706278ae305566d
Author: Yutaka Hirano <yhirano@chromium.org>
Date: Wed Nov 14 14:01:24 2018

Make ThreadableLoaderClient GarbageCollectedMixin

ThreadableLoaderClient has not been GarbageCollected mainly for
WorkerThreadableLoader. Now WorkerThreadableLoader is gone, so we can
make it an on-heap class. Doing so brings several benefits:

 - This is a blocker of removing ThreadableLoader after OOR-CORS is
   enabled, as ResourceClient is a GarbageCollectedMixin.
 - As ThreadableLoader holds a reference to its client as a raw
   pointer, each user needs to guarantee to cancel the loader properly
   before the client gets invalid. As most of clietns are already
   on-heap, this requires pre-finalizer for each client. Also, failing
   to keep the contract leads to crashes which are very hard to
   investigate.

Bug:  900506 
Change-Id: I67b3e3dab7e0600f06da5222d458ff2d81c36c7b
Reviewed-on: https://chromium-review.googlesource.com/c/1335056
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607970}
[modify] https://crrev.com/89797ba017745f20895099117706278ae305566d/third_party/blink/renderer/core/exported/web_associated_url_loader_impl.cc
[modify] https://crrev.com/89797ba017745f20895099117706278ae305566d/third_party/blink/renderer/core/exported/web_associated_url_loader_impl.h
[modify] https://crrev.com/89797ba017745f20895099117706278ae305566d/third_party/blink/renderer/core/fetch/fetch_manager.cc
[modify] https://crrev.com/89797ba017745f20895099117706278ae305566d/third_party/blink/renderer/core/inspector/inspector_network_agent.cc
[modify] https://crrev.com/89797ba017745f20895099117706278ae305566d/third_party/blink/renderer/core/inspector/inspector_network_agent.h
[modify] https://crrev.com/89797ba017745f20895099117706278ae305566d/third_party/blink/renderer/core/loader/threadable_loader.cc
[modify] https://crrev.com/89797ba017745f20895099117706278ae305566d/third_party/blink/renderer/core/loader/threadable_loader.h
[modify] https://crrev.com/89797ba017745f20895099117706278ae305566d/third_party/blink/renderer/core/loader/threadable_loader_client.h
[modify] https://crrev.com/89797ba017745f20895099117706278ae305566d/third_party/blink/renderer/core/loader/threadable_loader_test.cc
[modify] https://crrev.com/89797ba017745f20895099117706278ae305566d/third_party/blink/renderer/core/workers/worker_classic_script_loader.cc
[modify] https://crrev.com/89797ba017745f20895099117706278ae305566d/third_party/blink/renderer/core/workers/worker_classic_script_loader.h
[modify] https://crrev.com/89797ba017745f20895099117706278ae305566d/third_party/blink/renderer/core/xmlhttprequest/xml_http_request.cc
[modify] https://crrev.com/89797ba017745f20895099117706278ae305566d/third_party/blink/renderer/core/xmlhttprequest/xml_http_request.h
[modify] https://crrev.com/89797ba017745f20895099117706278ae305566d/third_party/blink/renderer/modules/background_fetch/background_fetch_icon_loader.h
[modify] https://crrev.com/89797ba017745f20895099117706278ae305566d/third_party/blink/renderer/modules/eventsource/event_source.cc
[modify] https://crrev.com/89797ba017745f20895099117706278ae305566d/third_party/blink/renderer/modules/notifications/notification_image_loader.h

Status: Fixed (was: Assigned)

Sign in to add a comment