New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 671502 link

Starred by 1 user

Issue metadata

Status: Assigned
Owner:
Last visit > 30 days ago
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocked on:
issue 682662



Sign in to add a comment

Increase test coverage for MemoryCoordinator/onMemoryStateChange()/memory pruning

Project Member Reported by hirosh...@chromium.org, Dec 6 2016

Issue description

Sometimes onMemoryStateChange() causes bugs, but currently it is hard to catch and investigate, because such bugs do not reproduce unless memory pressure is high, unless memory pressure notification is called in specific timing, or unless a certain code path is executed after onMemoryStateChange().
Examples are:
- Issue 668611: top crasher on canary.
-  Issue 618597 : reported on beta, reproduced only on ChromeOS, took 1 month before the correct suspected CL is found.

This also makes it hard to write tests, because it is hard to predict what kind of timing/code path causes the regression.

This issue tracks efforts to increase the coverage.
 
From offline discussion:
One candidate is
1. To create virtual layout tests that invokes onMemoryStateChange() more frequently, e.g. calling onMemoryStateChange() everytime before document onload.
2. If we find specific failure cases, write unit tests/layout tests for those specific cases, with exposing onMemoryStateChange() to blink::Internals.

I'll create a test CL to see how many tests would fail for Step 1.

Comment 2 by bashi@chromium.org, Dec 6 2016

Components: Blink>MemoryAllocator
Test CL: https://codereview.chromium.org/2553983002/
onMemoryStateChange() isn't called at this timing (i.e. onMemoryStateChange() should be always called as a top-level task), so the test CL might have more test failures than expected.

Many tests are failing due to one cause.

Comment 4 by bashi@chromium.org, Dec 6 2016

Owner: tasak@google.com
Status: Assigned (was: Available)
Sakamoto-san, would you mind looking at these failures? Please feel free to re-assign to me (or someone :)

Comment 5 by bashi@chromium.org, Dec 8 2016

Other than FontCache assertion failure, there seems other assertion failures. For example,

 [5444:5472:1206/001859.809:FATAL:document.cpp(2059)] Check failed: !needsStyleRecalc(). 
  Backtrace:
  	base::debug::StackTrace::StackTrace [0x000007FEE45B7201+33]
  	logging::LogMessage::~LogMessage [0x000007FEE45441BA+74]
  	blink::Document::updateStyle [0x000007FEE55FC787+1091]
  	blink::Document::updateStyleAndLayoutTree [0x000007FEE55FD02C+516]
  	blink::FrameView::updateStyleAndLayoutIfNeededRecursiveInternal [0x000007FEE57CD494+224]
  	blink::FrameView::updateStyleAndLayoutIfNeededRecursive [0x000007FEE57CD386+246]
  	blink::FrameView::updateLifecyclePhasesInternal [0x000007FEE57CBBE0+380]
  	blink::PageAnimator::updateAllLifecyclePhases [0x000007FEE5BBB6F4+32]
  	blink::WebViewImpl::updateAllLifecyclePhases [0x000007FEE525A98C+216]
  	cc::ProxyMain::BeginMainFrame [0x000007FEE66B486D+1373]
  	base::internal::Invoker<base::internal::BindState<void (__cdecl cc::ProxyMain::*)(std::unique_ptr<cc::BeginMainFrameAndCommitState,std::default_delete<cc::BeginMainFrameAndCommitState> >) __ptr64,base::WeakPtr<cc::ProxyMain>,base::internal::PassedWrapper< [0x000007FEE66C2FB2+162]
  	base::debug::TaskAnnotator::RunTask [0x000007FEE45D71F6+390]
  	blink::scheduler::TaskQueueManager::ProcessTaskFromWorkQueue [0x000007FEE51E8FF3+1247]
  	blink::scheduler::TaskQueueManager::DoWork [0x000007FEE51E8105+493]
  	base::internal::InvokeHelper<1,void>::MakeItSo<void (__cdecl blink::scheduler::TaskQueueManager::*const & __ptr64)(base::TimeTicks,bool) __ptr64,base::WeakPtr<blink::scheduler::TaskQueueManager> const & __ptr64,base::TimeTicks const & __ptr64,bool const & [0x000007FEE51E6DD5+105]
  	base::debug::TaskAnnotator::RunTask [0x000007FEE45D71F6+390]
  	base::MessageLoop::RunTask [0x000007FEE458FDEE+1294]
  	base::MessageLoop::DoWork [0x000007FEE458EA3A+394]
  	base::MessagePumpDefault::Run [0x000007FEE45D97A2+242]
  	base::RunLoop::Run [0x000007FEE45A31A3+67]
  	content::RendererMain [0x000007FEE61D77F0+676]
  	content::RunNamedProcessTypeMain [0x000007FEE452589F+423]
  	content::ContentMainRunnerImpl::Run [0x000007FEE452569E+358]
  	content::ContentMain [0x000007FEE45249F8+48]
  	ChromeMain [0x000007FEE382E6BC+236]
  	MainDllLoader::Launch [0x000000013FCE6204+868]
  	wWinMain [0x000000013FCE2F6F+443]
  	__scrt_common_main_seh [0x000000013FD49B83+279] (f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:253)
  	BaseThreadInitThunk [0x00000000770C5A4D+13]
  	RtlUserThreadStart [0x00000000772FB831+33]

Sakamoto-san, do you have any idea of the cause?

Comment 6 by tasak@google.com, Dec 8 2016

Looking.

Comment 7 by tasak@google.com, Dec 8 2016

virtual/gpu-rasterization/images/image-load-event-crash.html:

#0  setNeedsStyleRecalc ()
    at ../../third_party/WebKit/Source/core/dom/Node.cpp:714
#1  0x0000000015227e7c in blink::StyleEngine::fontsNeedUpdate(blink::CSSFontSelector*) () at ../../third_party/WebKit/Source/core/dom/StyleEngine.cpp:659
#2  0x000000001494f557 in blink::CSSFontSelector::dispatchInvalidationCallbacks() () at ../../third_party/WebKit/Source/core/css/CSSFontSelector.cpp:77
#3  0x000000001494f615 in blink::CSSFontSelector::fontCacheInvalidated() ()
    at ../../third_party/WebKit/Source/core/css/CSSFontSelector.cpp:85
#4  0x00000000128f1de9 in blink::FontCache::invalidate() ()
    at ../../third_party/WebKit/Source/platform/fonts/FontCache.cpp:456
#5  0x0000000012832c56 in blink::MemoryCoordinator::clearMemory() ()
    at ../../third_party/WebKit/Source/platform/MemoryCoordinator.cpp:78
#6  0x0000000012832783 in blink::MemoryCoordinator::onMemoryPressure(blink::WebMemoryPressureLevel) ()
    at ../../third_party/WebKit/Source/platform/MemoryCoordinator.cpp:61
#7  0x0000000014eb49fb in blink::Document::implicitClose() ()
    at ../../third_party/WebKit/Source/core/dom/Document.cpp:2876
#8  0x0000000016d72811 in blink::FrameLoader::checkCompleted() ()
    at ../../third_party/WebKit/Source/core/loader/FrameLoader.cpp:720
#9  0x0000000016d72590 in blink::FrameLoader::finishedParsing() ()
    at ../../third_party/WebKit/Source/core/loader/FrameLoader.cpp:632
#10 0x0000000014ed12db in finishedParsing ()
    at ../../third_party/WebKit/Source/core/dom/Document.cpp:5320
#11 0x00000000176ab24b in blink::XMLDocumentParser::end() ()
    at ../../third_party/WebKit/Source/core/xml/parser/XMLDocumentParser.cpp:415
#12 0x00000000176abf89 in blink::XMLDocumentParser::finish() ()
    at ../../third_party/WebKit/Source/core/xml/parser/XMLDocumentParser.cpp:430
#13 0x0000000016d5345b in end ()
    at ../../third_party/WebKit/Source/core/loader/DocumentWriter.cpp:109
#14 0x0000000016d1f26b in blink::DocumentLoader::endWriting() ()
    at ../../third_party/WebKit/Source/core/loader/DocumentLoader.cpp:748
#15 0x0000000016d1e8df in blink::DocumentLoader::finishedLoading(double) ()
    at ../../third_party/WebKit/Source/core/loader/DocumentLoader.cpp:316
#16 0x0000000016d1e3e9 in blink::DocumentLoader::notifyFinished(blink::Resource*) () at ../../third_party/WebKit/Source/core/loader/DocumentLoader.cpp:277
#17 0x000000001576c29a in blink::Resource::didAddClient(blink::ResourceClient*)
    () at ../../third_party/WebKit/Source/core/fetch/Resource.cpp:664
#18 0x000000001f7b3df8 in didAddClient ()
    at ../../third_party/WebKit/Source/core/fetch/RawResource.cpp:157
#19 0x000000001576cce4 in blink::Resource::addClient(blink::ResourceClient*, blink::Resource::PreloadReferencePolicy) ()
    at ../../third_party/WebKit/Source/core/fetch/Resource.cpp:733
#20 0x0000000016d23b84 in blink::DocumentLoader::startLoadingMainResource() ()
    at ../../third_party/WebKit/Source/core/loader/DocumentLoader.cpp:744
#21 0x0000000016d779a1 in blink::FrameLoader::startLoad(blink::FrameLoadRequest&, blink::FrameLoadType, blink::NavigationPolicy) ()
    at ../../third_party/WebKit/Source/core/loader/FrameLoader.cpp:1716
#22 0x0000000016d6cc74 in blink::FrameLoader::load(blink::FrameLoadRequest const&, blink::FrameLoadType, blink::HistoryItem*, blink::HistoryLoadType) ()
    at ../../third_party/WebKit/Source/core/loader/FrameLoader.cpp:1186
#23 0x00000000174d94a5 in blink::SVGImage::dataChanged(bool) ()
    at ../../third_party/WebKit/Source/core/svg/graphics/SVGImage.cpp:611
#24 0x0000000012a31d65 in blink::Image::setData(WTF::PassRefPtr<blink::SharedBuffer>, bool) ()
    at ../../third_party/WebKit/Source/platform/graphics/Image.cpp:88
#25 0x00000000157248ed in blink::ImageResource::updateImage(bool) ()
    at ../../third_party/WebKit/Source/core/fetch/ImageResource.cpp:477
#26 0x0000000015727733 in blink::ImageResource::finish(double) ()
    at ../../third_party/WebKit/Source/core/fetch/ImageResource.cpp:529
#27 0x00000000157aa124 in blink::Resource::finish() ()
    at ../../third_party/WebKit/Source/core/fetch/Resource.h:204
#28 0x0000000015792ec7 in resourceForStaticData ()
    at ../../third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp:432
#29 0x0000000015796714 in requestResource ()
    at ../../third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp:558
#30 0x000000001571f0de in blink::ImageResource::fetch(blink::FetchRequest&, blink::ResourceFetcher*) ()
    at ../../third_party/WebKit/Source/core/fetch/ImageResource.cpp:98
#31 0x000000001497ef28 in blink::CSSImageValue::cacheImage(blink::Document const&, blink::CrossOriginAttributeValue) ()
    at ../../third_party/WebKit/Source/core/css/CSSImageValue.cpp:71
#32 0x0000000014cdedfd in blink::ElementStyleResources::loadPendingImage(blink::ComputedStyle*, blink::StylePendingImage*, blink::CrossOriginAttributeValue) ()
    at ../../third_party/WebKit/Source/core/css/resolver/ElementStyleResources.cpp:136
#33 0x0000000014cdf40c in blink::ElementStyleResources::loadPendingImages(blink::ComputedStyle*) ()
    at ../../third_party/WebKit/Source/core/css/resolver/ElementStyleResources.cpp:188
#34 0x0000000014cdfcbf in blink::ElementStyleResources::loadPendingResources(blink::ComputedStyle*) ()
    at ../../third_party/WebKit/Source/core/css/resolver/ElementStyleResources.cpp:278
#35 0x0000000014d2dc92 in blink::StyleResolver::loadPendingResources(blink::StyleResolverState&) ()
    at ../../third_party/WebKit/Source/core/css/resolver/StyleResolver.cpp:637
#36 0x0000000014d3657e in blink::StyleResolver::applyMatchedStandardProperties(blink::StyleResolverState&, blink::MatchResult const&, blink::StyleResolver::CacheSuccess const&, blink::StyleResolver::NeedsApplyPass&) ()
    at ../../third_party/WebKit/Source/core/css/resolver/StyleResolver.cpp:1877
#37 0x0000000014d2f654 in blink::StyleResolver::applyMatchedPropertiesAndCustomPropertyAnimations(blink::StyleResolverState&, blink::MatchResult const&, blink::Element const*) ()
    at ../../third_party/WebKit/Source/core/css/resolver/StyleResolver.cpp:1648
#38 0x0000000014d2ec6c in blink::StyleResolver::styleForElement(blink::Element*, blink::ComputedStyle const*, blink::StyleSharingBehavior, blink::RuleMatchingBehavior) ()
    at ../../third_party/WebKit/Source/core/css/resolver/StyleResolver.cpp:797
#39 0x0000000014fd5b76 in blink::Element::originalStyleForLayoutObject() ()
    at ../../third_party/WebKit/Source/core/dom/Element.cpp:1835
#40 0x0000000015ba3055 in blink::HTMLImageElement::customStyleForLayoutObject()
    () at ../../third_party/WebKit/Source/core/html/HTMLImageElement.cpp:872
#41 0x0000000014fd5645 in blink::Element::styleForLayoutObject() ()
    at ../../third_party/WebKit/Source/core/dom/Element.cpp:1810
#42 0x000000001509bd0c in blink::LayoutTreeBuilderForElement::style() const ()
    at ../../third_party/WebKit/Source/core/dom/LayoutTreeBuilder.cpp:115
#43 0x000000001509bbb9 in blink::LayoutTreeBuilderForElement::shouldCreateLayoutObject() const ()
    at ../../third_party/WebKit/Source/core/dom/LayoutTreeBuilder.cpp:110
#44 0x0000000014ff19a9 in blink::LayoutTreeBuilderForElement::createLayoutObjectIfNeeded() ()
    at ../../third_party/WebKit/Source/core/dom/LayoutTreeBuilder.h:76
#45 0x0000000014fd3f58 in blink::Element::attachLayoutTree(blink::Node::AttachContext const&) () at ../../third_party/WebKit/Source/core/dom/Element.cpp:1674
#46 0x0000000015b9cc77 in blink::HTMLImageElement::attachLayoutTree(blink::Node::AttachContext const&) ()
    at ../../third_party/WebKit/Source/core/html/HTMLImageElement.cpp:369
#47 0x0000000014dfa502 in blink::ContainerNode::attachLayoutTree(blink::Node::AttachContext const&) ()
    at ../../third_party/WebKit/Source/core/dom/ContainerNode.cpp:766
#48 0x0000000014fd403a in blink::Element::attachLayoutTree(blink::Node::AttachContext const&) () at ../../third_party/WebKit/Source/core/dom/Element.cpp:1695
#49 0x0000000014dfa502 in blink::ContainerNode::attachLayoutTree(blink::Node::AttachContext const&) ()
    at ../../third_party/WebKit/Source/core/dom/ContainerNode.cpp:766
#50 0x0000000014fd403a in blink::Element::attachLayoutTree(blink::Node::AttachContext const&) () at ../../third_party/WebKit/Source/core/dom/Element.cpp:1695
#51 0x000000001510119a in blink::Node::reattachLayoutTree(blink::Node::AttachContext const&) () at ../../third_party/WebKit/Source/core/dom/Node.cpp:890
#52 0x0000000014fd88b0 in blink::Element::rebuildLayoutTree() ()
    at ../../third_party/WebKit/Source/core/dom/Element.cpp:2007
#53 0x0000000014fd71ee in recalcOwnStyle ()
    at ../../third_party/WebKit/Source/core/dom/Element.cpp:1953
#54 0x0000000014fd6276 in blink::Element::recalcStyle(blink::StyleRecalcChange, blink::Text*) () at ../../third_party/WebKit/Source/core/dom/Element.cpp:1859
#55 0x0000000014eaa79b in updateStyle ()
    at ../../third_party/WebKit/Source/core/dom/Document.cpp:2048
#56 0x0000000014ea288e in updateStyleAndLayoutTree ()
    at ../../third_party/WebKit/Source/core/dom/Document.cpp:1967
#57 0x0000000014ed12bb in finishedParsing ()

The layout test has a svg image. The image is required while style recalc.
The image is parsed and document.implicitClose is invoked. This means, onMemoryPressure is invoked while style recalc.

Since FontCache::invalidate (invoked by memory coordinator) invokes setNeedsStyleRecalc, the assertion fails.




Project Member

Comment 8 by bugdroid1@chromium.org, Dec 8 2016

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

commit e17e2ebb5d24ba236e8fb3b0d2ecf5f542be03ad
Author: bashi <bashi@chromium.org>
Date: Thu Dec 08 22:57:03 2016

Allow calling FontCache::purge() at any timing

MemoryCoordinator calls purge() when it receives memory pressure
signals. Allow calling purge() at any timing because memory pressure
signals can be triggered at any timing.

BUG=671502

Review-Url: https://codereview.chromium.org/2558453005
Cr-Commit-Position: refs/heads/master@{#437387}

[modify] https://crrev.com/e17e2ebb5d24ba236e8fb3b0d2ecf5f542be03ad/third_party/WebKit/Source/platform/fonts/FontCache.cpp

Blockedon: 682662
Owner: tasak@chromium.org

Sign in to add a comment