New issue
Advanced search Search tips

Issue 778799 link

Starred by 2 users
Status: Untriaged
Owner: ----
Cc:
kinuko@chromium.org
hirosh...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 759961



Sign in to add a comment

Should ScriptResource be garbage-collected/evicted from MemoryCache after script execution?

Project Member Reported by hirosh...@chromium.org, Oct 26 2017 
This issue is for tracking discussion on whether ScriptResource should be garbage-collected/evicted from MemoryCache after script execution, or not.

[A] ScriptResource is kept alive and kept on MemoryCache, as long as the corresponding <script> element (i.e. ScriptLoader etc.) is alive.
[B] ScriptResource can be garbage collected after the script is executed, even if the corresponding <script> element is alive.

Doc: https://docs.google.com/document/d/1aTPmpe-cCDW7VfExN-fjMTe7eU9EwBcOW4VjVx2zOSE/edit?ts=59b1cd79#heading=h.bvv2ejduz9j9

The current behavior is:
[B] for scripts controlled by ScriptRunner (e.g. <script async>,  Issue 759961 )
[A] for other scripts.

This is related to  Issue 766653  (and shares the doc above) but is different from the issue, because  Issue 766653  doesn't affect the lifetime of ScriptResource within the lifetime of a page before navigation starts.

Comment 1 by hirosh...@chromium.org, Oct 26 2017 

I'll keep the existing behavior, as the Blink loading team hasn't reached a strong consensus about this issue.
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 26 2017 

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

commit 9d0b896c09461e4c1fcee17856f768d3e66880bf
Author: Hiroshige Hayashizaki <hiroshige@chromium.org>
Date: Thu Oct 26 22:39:56 2017

Create PendingScript when script loading is started in PrepareScript()

Previously, PendingScript was created in
ScriptLoader::CreatePendingScript() which could be invoked by
non-ScriptLoader after ScriptLoader::PrepareScript().

This CL creates PendingScript during ScriptLoader::PrepareScript()
when the script fetch is initiated.

This is in order to:
- Couple ScriptResource::Fetch() and SetResource() (and thus
  AddClient()) to make it easier to assign TaskRunner to AddClient().
  (Context:
  https://chromium-review.googlesource.com/c/chromium/src/+/723879)
- Simplify the lifetime of PendingScript, and remove glue code for
  bridging PrepareScript() and CreatePendingScript().

This CL renames |ScriptLoader::resource_| into |resource_keep_alive_|,
because it is now used only to keep the ScriptResource alive even
after script execution.
This CL doesn't remove the reference from ScriptLoader to
ScriptResource, because removing it makes the ScriptResource's
lifetime shorter and thus evicted from MemoryCache (because it is
the only reference to the ScriptResource after script execution),
and causes devtools layout test failures.

Bug:  777612 , 686281, 778799
Change-Id: I5ea0a393ee567520c526ee69b82e08281caa7a7d
Reviewed-on: https://chromium-review.googlesource.com/730852
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511991}
[modify] https://crrev.com/9d0b896c09461e4c1fcee17856f768d3e66880bf/third_party/WebKit/Source/bindings/core/v8/ScriptStreamerTest.cpp
[modify] https://crrev.com/9d0b896c09461e4c1fcee17856f768d3e66880bf/third_party/WebKit/Source/core/dom/ClassicPendingScript.cpp
[modify] https://crrev.com/9d0b896c09461e4c1fcee17856f768d3e66880bf/third_party/WebKit/Source/core/dom/ClassicPendingScript.h
[modify] https://crrev.com/9d0b896c09461e4c1fcee17856f768d3e66880bf/third_party/WebKit/Source/core/dom/PendingScript.h
[modify] https://crrev.com/9d0b896c09461e4c1fcee17856f768d3e66880bf/third_party/WebKit/Source/core/dom/ScriptLoader.cpp
[modify] https://crrev.com/9d0b896c09461e4c1fcee17856f768d3e66880bf/third_party/WebKit/Source/core/dom/ScriptLoader.h
[modify] https://crrev.com/9d0b896c09461e4c1fcee17856f768d3e66880bf/third_party/WebKit/Source/core/html/parser/HTMLParserScriptRunner.cpp
[modify] https://crrev.com/9d0b896c09461e4c1fcee17856f768d3e66880bf/third_party/WebKit/Source/core/xml/parser/XMLParserScriptRunner.cpp

Comment 3 by hirosh...@chromium.org, Oct 26 2017 

The CL in Comment #2 doesn't change the behavior. The reference from ScriptLoader to ScriptResource is ScriptLoader::resource_keep_alive_ that is only used for the keep-alive purpose. We can adjust the lifetime of ScriptResource by clearing/not-clearing resource_keep_alive_ without affecting other part of the code.
Project Member

Comment 4 by sheriffbot@chromium.org, Oct 29

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

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

Sign in to add a comment