New issue
Advanced search Search tips

Issue 922908 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 20
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

blink::Cache's mojo associated pointer could be disconnected

Project Member Reported by bashi@chromium.org, Jan 17 (5 days ago)

Issue description

See https://bugs.chromium.org/p/chromium/issues/detail?id=922449#c8 for the original issue.

blink::Cache has an implicit assumption that |cache_ptr_| is alive during its lifetime. However, this assumption may not hold in some situations because |cache_ptr_| is an associated pointer of blink::CacheStorage's |cache_storage_ptr_|. When the wrapper of CacheStorage is destroyed, |cache_ptr_| will be disconnected too. We probably need to make Cache have a reference to CacheStorage so that CacheStorage wouldn't be destroyed until the wrapper of Cache is GCed.
 

Comment 1 by wanderview@chromium.org, Jan 17 (5 days ago)

> We probably need to make Cache have a reference to CacheStorage so that CacheStorage wouldn't be destroyed until the wrapper of Cache is GCed.

Yea, adding a Member<blink::CacheStorage> to blink::Cache sounds good.  Thanks for finding this!
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 18 (5 days ago)

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

commit 7bc86423ba2bdd56b15a5cf44089043142326dda
Author: Kenichi Ishibashi <bashi@chromium.org>
Date: Fri Jan 18 03:14:01 2019

CacheStorage: Hold a reference to CacheStorage from Cache

So that CacheStorage will be alive as long as Cache instances are
alive. This makes sure that Cache's mojo interface pointer wouldn't
be disconnected during Cache's lifetime.

Bug:  922908 , 922449
Change-Id: I8e83c1b20af906b34fc28cde54e4ec4ce502d458
Reviewed-on: https://chromium-review.googlesource.com/c/1419757
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Ben Kelly <wanderview@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623984}
[modify] https://crrev.com/7bc86423ba2bdd56b15a5cf44089043142326dda/third_party/blink/renderer/modules/cache_storage/cache.cc
[modify] https://crrev.com/7bc86423ba2bdd56b15a5cf44089043142326dda/third_party/blink/renderer/modules/cache_storage/cache.h
[modify] https://crrev.com/7bc86423ba2bdd56b15a5cf44089043142326dda/third_party/blink/renderer/modules/cache_storage/cache_storage.cc
[modify] https://crrev.com/7bc86423ba2bdd56b15a5cf44089043142326dda/third_party/blink/renderer/modules/cache_storage/cache_test.cc

Comment 3 by bashi@chromium.org, Jan 20 (2 days ago)

Status: Fixed (was: Assigned)

Sign in to add a comment