Decouple CachedMetadataHandler from Resource |
||
Issue descriptionCurrently CachedMetadataHandler (in fact its subclass) has a reference to Resource, but this reference should be removed: - The existence of the reference from CMH to Resource requires that the Resource shouldn't be revalidated before the metadata are written to CachedMetadataHandler, and thus blocks Issue 752711 and V8 code cache processing after script execution. - CachedMetadataHandler is only for metadata cache in the browser side, and shouldn't refer to Resource/MemoryCache layer. Also, probably CachedMetadataHandler's lifetime should be fixed (CachedMetadataHandler should be created when response is received etc).
,
Dec 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7d1a458799d108aefda87d9939c675a07c994970 commit 7d1a458799d108aefda87d9939c675a07c994970 Author: Hiroshige Hayashizaki <hiroshige@chromium.org> Date: Fri Dec 08 00:35:25 2017 Do not use Resource in CacheOptions() in ScriptController To remove Resource from ScriptController, this CL - Introduces CachedMetadataHandler::IsServedFromCacheStorage(), and - Uses it in ScriptController's CacheOptions(). Bug: 788828 , 784875 Change-Id: Iaf34c40ceff223357361e8a189c3bf5d33251ffc Reviewed-on: https://chromium-review.googlesource.com/786322 Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org> Reviewed-by: Nate Chapin <japhet@chromium.org> Reviewed-by: Ross McIlroy <rmcilroy@chromium.org> Reviewed-by: Kouhei Ueno <kouhei@chromium.org> Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org> Reviewed-by: Tsuyoshi Horo <horo@chromium.org> Cr-Commit-Position: refs/heads/master@{#522645} [modify] https://crrev.com/7d1a458799d108aefda87d9939c675a07c994970/third_party/WebKit/Source/bindings/core/v8/ScriptController.cpp [modify] https://crrev.com/7d1a458799d108aefda87d9939c675a07c994970/third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerScriptCachedMetadataHandler.cpp [modify] https://crrev.com/7d1a458799d108aefda87d9939c675a07c994970/third_party/WebKit/Source/modules/serviceworkers/ServiceWorkerScriptCachedMetadataHandler.h [modify] https://crrev.com/7d1a458799d108aefda87d9939c675a07c994970/third_party/WebKit/Source/platform/loader/fetch/CachedMetadataHandler.h [modify] https://crrev.com/7d1a458799d108aefda87d9939c675a07c994970/third_party/WebKit/Source/platform/loader/fetch/Resource.cpp
,
Dec 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9b0a3407099993d8acaa29e01a90dcfa5c1de1c2 commit 9b0a3407099993d8acaa29e01a90dcfa5c1de1c2 Author: Hiroshige Hayashizaki <hiroshige@chromium.org> Date: Wed Dec 13 23:06:02 2017 Fix/add unit tests related to CachedMetadataHandler This CL updates unit tests as a preparation for CachedMetadataHandler changes in [1]. This CL - Calls Resource::SetResponse() in ScriptStreamerTest.cpp and V8ScriptRunnerTest.cpp, because [1] will depend on SetResponse(). - Sets ResourceResponse's URL, because CachedMetadataHandler depends on response URLs. - Uses http: URLs instead of data: URLs in ResourceTest, because CachedMetadataHandler is not set for data: URLs. These are also natural, as - SetResponse() is called and response URLs are set in non-test code. - revalidation never occurs on data: URLs. This CL also adds ResourceTest.RevalidationFailed, and adds unit test assertions related to CachedMetadataHandler. [1] https://chromium-review.googlesource.com/786410 Bug: 784875 Change-Id: Ia58d3c6c9b2ad20c502d8ed9c69d505aa73be03b Reviewed-on: https://chromium-review.googlesource.com/812569 Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org> Reviewed-by: Kouhei Ueno <kouhei@chromium.org> Reviewed-by: Nate Chapin <japhet@chromium.org> Cr-Commit-Position: refs/heads/master@{#523926} [modify] https://crrev.com/9b0a3407099993d8acaa29e01a90dcfa5c1de1c2/third_party/WebKit/Source/bindings/core/v8/ScriptStreamerTest.cpp [modify] https://crrev.com/9b0a3407099993d8acaa29e01a90dcfa5c1de1c2/third_party/WebKit/Source/bindings/core/v8/V8ScriptRunnerTest.cpp [modify] https://crrev.com/9b0a3407099993d8acaa29e01a90dcfa5c1de1c2/third_party/WebKit/Source/platform/loader/fetch/ResourceTest.cpp
,
Dec 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cecbff796c9b4a57a49eb98912c718959e54218c commit cecbff796c9b4a57a49eb98912c718959e54218c Author: Hiroshige Hayashizaki <hiroshige@chromium.org> Date: Sat Dec 16 00:57:44 2017 Create CachedMetadataHandler when a response is received This CL creates CachedMetadataHandler when a response is received, instead of creating when a Resource is constructed. This is to - Prepare for removing the reference from CachedMetadataHandler to Resource in [1]. The data needed for CachedMetadataHandler is available when a response is received, but not available when the Resource is created. Also, this CL makes the lifetime of CachedMetadataHandler match with the period where the corresponding ResourceResponse is set in Resource::response_. - Enable CachedMetadataHandler even after revalidation. Previously, CachedMetadataHandler was cleared on failed revalidation and was never re-created after that. This CL creates a new CachedMetadataHandler everytime a ResourceResponse is received, and thus re-creates a CachedMetadataHandler on failed revalidation. [1] https://chromium-review.googlesource.com/791494 Bug: 784875 Change-Id: Ia32f5772cbd5970734f67098f235c2a1428b46ea Reviewed-on: https://chromium-review.googlesource.com/786410 Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org> Reviewed-by: Tsuyoshi Horo <horo@chromium.org> Reviewed-by: Yutaka Hirano <yhirano@chromium.org> Reviewed-by: Kouhei Ueno <kouhei@chromium.org> Reviewed-by: Nate Chapin <japhet@chromium.org> Cr-Commit-Position: refs/heads/master@{#524540} [modify] https://crrev.com/cecbff796c9b4a57a49eb98912c718959e54218c/third_party/WebKit/Source/platform/loader/fetch/Resource.cpp [modify] https://crrev.com/cecbff796c9b4a57a49eb98912c718959e54218c/third_party/WebKit/Source/platform/loader/fetch/ResourceTest.cpp
,
Jan 30 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f35da1e67a72557eece061711241cc10e1572e22 commit f35da1e67a72557eece061711241cc10e1572e22 Author: Hiroshige Hayashizaki <hiroshige@chromium.org> Date: Tue Jan 30 05:58:32 2018 Decouple CachedMetadataHandler from Resource This CL removes a reference from CachedMetadataHandlerImpl to Resource, and instead captures necessary data from Resource when CachedMetadataHandlerImpl is created, i.e. when a response is received. This is to make CachedMetadataHandler and Resource independent, so that CachedMetadataHandler can be used even when the corresponding Resource is garbage-collected or revalidated, for Issue 783124. This CL causes a behavior change in CachedMetadataHandler::Encoding(): Previously, it returned the final encoding that reflected the encoding detection by the response body. After this CL, it returns the encoding at the time of ResponseReceived() that doesn't reflect the encoding detection (by UTF BOMs in the case of scripts). This shouldn't affect the correctness, because if the encoding at the time of ResponseReceived() is the same, then the final encoding should be also the same (provided the response content is the same). This might cause different Encoding()s are returned for the same final encoding and thus affect memory/performance, but this is expected to be quite rare because UTF BOMs are quite rare. Bug: 784875 , 783124 Change-Id: I0b72fafc54537fae93c90d1abf3bddf4e094add8 Reviewed-on: https://chromium-review.googlesource.com/791494 Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org> Reviewed-by: Kouhei Ueno <kouhei@chromium.org> Reviewed-by: Tsuyoshi Horo <horo@chromium.org> Reviewed-by: Yutaka Hirano <yhirano@chromium.org> Cr-Commit-Position: refs/heads/master@{#532784} [modify] https://crrev.com/f35da1e67a72557eece061711241cc10e1572e22/third_party/WebKit/Source/platform/loader/fetch/Resource.cpp
,
Feb 13 2018
|
||
►
Sign in to add a comment |
||
Comment 1 by hirosh...@chromium.org
, Nov 28 2017