Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1782473002:
Check if stylesheet resource was cached before marking it as such. (Closed)
|
Created:
4 years, 9 months ago by sof Modified:
4 years, 9 months ago CC:
chromium-reviews, tyoshino+watch_chromium.org, blink-reviews-style_chromium.org, blink-reviews-css, dglazkov+blink, apavlov+blink_chromium.org, gavinp+loader_chromium.org, darktears, blink-reviews, loading-reviews+fetch_chromium.org, rwlbuis Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionCheck if stylesheet resource was cached before marking it as such.
Should the memory cache already have an entry for a resource other
than the stylesheet resource we're trying to add, do not mark
the underlying StyleSheetContents as being "cached".
Given the possibility that the StyleSheetContents may not be
memory cached, retire the sanity-checking assert that a stylesheet
resource must have been evicted from that cache when finalized.
R=japhet
BUG=
NOTRY=true
Committed: https://crrev.com/34cb0c85cdf289e217a497a5bff147966ddaad0a
Cr-Commit-Position: refs/heads/master@{#380559}
Patch Set 1 #Patch Set 2 : add unit test #Patch Set 3 : fix non-oilpan compilation #
Messages
Total messages: 34 (18 generated)
Description was changed from ========== Check if stylesheet resource was cached before marking it as such. Should the memory cache already have an entry for a resource other than the stylesheet resource we're trying to add, do not mark the underlying StyleSheetContents as being "cached". Given the possibility that the StyleSheetContents may not be memory cached, retire the sanity-checking assert that stylesheet resource must have been evicted from that cache when finalized. R= BUG= ========== to ========== Check if stylesheet resource was cached before marking it as such. Should the memory cache already have an entry for a resource other than the stylesheet resource we're trying to add, do not mark the underlying StyleSheetContents as being "cached". Given the possibility that the StyleSheetContents may not be memory cached, retire the sanity-checking assert that a stylesheet resource must have been evicted from that cache when finalized. R= BUG= ==========
sigbjornf@opera.com changed reviewers: + japhet@chromium.org, tkent@chromium.org
please take a look. The ~CSSStyleSheetResource assert triggered while the twitter widget was loading on bild.de; not been able to come up with a repro'ing layout test (if anyone has suggestions..).
On 2016/03/09 18:40:28, sof wrote: > please take a look. > > The ~CSSStyleSheetResource assert triggered while the twitter widget was loading > on bild.de; not been able to come up with a repro'ing layout test (if anyone has > suggestions..). Code change LGTM. Agreed that this sounds difficult to reproduce in a layout test. Any chance a unit test would be able to make the timing reliable?
On 2016/03/09 20:05:29, Nate Chapin wrote: > On 2016/03/09 18:40:28, sof wrote: > > please take a look. > > > > The ~CSSStyleSheetResource assert triggered while the twitter widget was > loading > > on bild.de; not been able to come up with a repro'ing layout test (if anyone > has > > suggestions..). > > Code change LGTM. Agreed that this sounds difficult to reproduce in a layout > test. Any chance a unit test would be able to make the timing reliable? Thanks, that appears possible - added a unit test.
Thanks! LGTM
Description was changed from ========== Check if stylesheet resource was cached before marking it as such. Should the memory cache already have an entry for a resource other than the stylesheet resource we're trying to add, do not mark the underlying StyleSheetContents as being "cached". Given the possibility that the StyleSheetContents may not be memory cached, retire the sanity-checking assert that a stylesheet resource must have been evicted from that cache when finalized. R= BUG= ========== to ========== Check if stylesheet resource was cached before marking it as such. Should the memory cache already have an entry for a resource other than the stylesheet resource we're trying to add, do not mark the underlying StyleSheetContents as being "cached". Given the possibility that the StyleSheetContents may not be memory cached, retire the sanity-checking assert that a stylesheet resource must have been evicted from that cache when finalized. R=japhet BUG= ==========
The CQ bit was checked by sigbjornf@opera.com
The patchset sent to the CQ was uploaded after l-g-t-m from japhet@chromium.org Link to the patchset: https://codereview.chromium.org/1782473002/#ps40001 (title: "fix non-oilpan compilation")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1782473002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1782473002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by sigbjornf@opera.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1782473002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1782473002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by sigbjornf@opera.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1782473002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1782473002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_asan_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by sigbjornf@opera.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1782473002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1782473002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_chromium_chromeos_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by sigbjornf@opera.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1782473002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1782473002/40001
The CQ bit was unchecked by sigbjornf@opera.com
Description was changed from ========== Check if stylesheet resource was cached before marking it as such. Should the memory cache already have an entry for a resource other than the stylesheet resource we're trying to add, do not mark the underlying StyleSheetContents as being "cached". Given the possibility that the StyleSheetContents may not be memory cached, retire the sanity-checking assert that a stylesheet resource must have been evicted from that cache when finalized. R=japhet BUG= ========== to ========== Check if stylesheet resource was cached before marking it as such. Should the memory cache already have an entry for a resource other than the stylesheet resource we're trying to add, do not mark the underlying StyleSheetContents as being "cached". Given the possibility that the StyleSheetContents may not be memory cached, retire the sanity-checking assert that a stylesheet resource must have been evicted from that cache when finalized. R=japhet BUG= NOTRY=true ==========
The CQ bit was checked by sigbjornf@opera.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1782473002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1782473002/40001
Message was sent while issue was closed.
Description was changed from ========== Check if stylesheet resource was cached before marking it as such. Should the memory cache already have an entry for a resource other than the stylesheet resource we're trying to add, do not mark the underlying StyleSheetContents as being "cached". Given the possibility that the StyleSheetContents may not be memory cached, retire the sanity-checking assert that a stylesheet resource must have been evicted from that cache when finalized. R=japhet BUG= NOTRY=true ========== to ========== Check if stylesheet resource was cached before marking it as such. Should the memory cache already have an entry for a resource other than the stylesheet resource we're trying to add, do not mark the underlying StyleSheetContents as being "cached". Given the possibility that the StyleSheetContents may not be memory cached, retire the sanity-checking assert that a stylesheet resource must have been evicted from that cache when finalized. R=japhet BUG= NOTRY=true ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Check if stylesheet resource was cached before marking it as such. Should the memory cache already have an entry for a resource other than the stylesheet resource we're trying to add, do not mark the underlying StyleSheetContents as being "cached". Given the possibility that the StyleSheetContents may not be memory cached, retire the sanity-checking assert that a stylesheet resource must have been evicted from that cache when finalized. R=japhet BUG= NOTRY=true ========== to ========== Check if stylesheet resource was cached before marking it as such. Should the memory cache already have an entry for a resource other than the stylesheet resource we're trying to add, do not mark the underlying StyleSheetContents as being "cached". Given the possibility that the StyleSheetContents may not be memory cached, retire the sanity-checking assert that a stylesheet resource must have been evicted from that cache when finalized. R=japhet BUG= NOTRY=true Committed: https://crrev.com/34cb0c85cdf289e217a497a5bff147966ddaad0a Cr-Commit-Position: refs/heads/master@{#380559} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/34cb0c85cdf289e217a497a5bff147966ddaad0a Cr-Commit-Position: refs/heads/master@{#380559} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
