DCHECK failure in ImageResource::addObserver |
||||
Issue descriptionStep: (1) Run a build with DCHECKs enabled. (2) Load an HTML file which loads a large (~70K) script followed a single-part multipart image. What is the expected output? Browser does not crash with a DCHECK failure. What do you see instead? Browser crashes with a DCHECK and the following stack trace: [1:1:0725/185319:1053201297114:FATAL:ImageResource.cpp(133)] Check failed: !m_data || m_image. #0 0x7f36dfc7d86e base::debug::StackTrace::StackTrace() #1 0x7f36dfc9e9fb logging::LogMessage::~LogMessage() #2 0x7f36dbf077fc blink::ImageResource::addObserver() #3 0x7f36dc077ad8 blink::ImageLoader::doUpdateFromElement() #4 0x7f36dc077eb3 blink::ImageLoader::updateFromElement() #5 0x7f36dbb52f9b blink::HTMLImageElement::selectSourceURL() #6 0x7f36dba0c6cf blink::Element::attributeChanged() #7 0x7f36dba0db80 blink::Element::parserSetAttributes() #8 0x7f36dbc06b3d blink::HTMLConstructionSite::createHTMLElement() #9 0x7f36dbc06e6a blink::HTMLConstructionSite::insertSelfClosingHTMLElementDestroyingToken() #10 0x7f36dbc3f384 blink::HTMLTreeBuilder::processStartTagForInBody() #11 0x7f36dbc39645 blink::HTMLTreeBuilder::constructTree() #12 0x7f36dbc0ddf8 blink::HTMLDocumentParser::processParsedChunkFromBackgroundParser() #13 0x7f36dbc0b9b9 blink::HTMLDocumentParser::pumpPendingSpeculations() #14 0x7f36dba64f38 blink::PendingScript::streamingFinished() #15 0x7f36da5ec909 _ZN4base8internal7InvokerINS0_9BindStateIPFvSt10unique_ptrIN5blink13WebTaskRunner4TaskESt14default_deleteIS6_EEEJNS0_13PassedWrapperIS9_EEEEEFvvEE3RunEPNS0_13BindStateBaseE #16 0x7f36dfc7ea59 base::debug::TaskAnnotator::RunTask() #17 0x7f36da5df797 scheduler::TaskQueueManager::ProcessTaskFromWorkQueue() #18 0x7f36da5de429 scheduler::TaskQueueManager::DoWork() #19 0x7f36dfc7ea59 base::debug::TaskAnnotator::RunTask() #20 0x7f36dfca9375 base::MessageLoop::RunTask() #21 0x7f36dfca96a8 base::MessageLoop::DeferOrRunPendingTask() #22 0x7f36dfca9a5b base::MessageLoop::DoWork() #23 0x7f36dfcab21e base::MessagePumpDefault::Run() #24 0x7f36dfca8e71 base::MessageLoop::RunHandler() #25 0x7f36dfcd7680 base::RunLoop::Run() #26 0x7f36e0d3f92d content::RendererMain() #27 0x7f36e0e5418b content::RunZygote() #28 0x7f36e0e54a32 content::RunNamedProcessTypeMain() #29 0x7f36e0e55483 content::ContentMainRunnerImpl::Run() #30 0x7f36e0e53d50 content::ContentMain() #31 0x000000464c8b main #32 0x7f36d82c0f45 __libc_start_main #33 0x000000464b81 <unknown> My repro is: <!DOCTYPE html> <script src="/resources/testharness.js"></script> <img src="/multipart/resources/multipart.php?interval=1&loop=1&img1=abe.png"> I encountered this while working on a layout test. I have not experimented with changing the img src URL, however the crash started happening when I reduced the number of images in the URL from 2 to 1. I have reproduced the crash with JS files containing only spaces as small as 7K, however it works more reliably with larger JS files, and testharness.js is the most reliable one I have found.
,
Jul 25 2016
There's a repro/regression test at https://codereview.chromium.org/2179753002/
,
Jul 25 2016
Thanks. Reproduced.
,
Jul 27 2016
,
Jul 27 2016
,
Jul 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d1e1dd413401a3434537e12f1f12637e7c8bf2fb commit d1e1dd413401a3434537e12f1f12637e7c8bf2fb Author: hajimehoshi <hajimehoshi@chromium.org> Date: Wed Jul 27 10:59:47 2016 Bug fix: Fix wrong assumption in ImageResource when adding observer When the response is not multipart, if |m_data| exists, |m_image| must be created. This is assured that |updateImage()| is called when |appendData()| is called. On the other hand, when the response is multipart, |updateImage()| is not called in |appendData()|, which means |m_image| might not be created even when |m_data| exists. This is intentional since creating a |m_image| on receiving data might destroy an existing image in a previous part. This CL fixes the wrong DCHECK in addObserver and didAddClient in ImageResource. BUG= 630983 TEST=webkit_unit_tests --gtest_filter=ImageResourceTest.MultipartImage Review-Url: https://codereview.chromium.org/2182023002 Cr-Commit-Position: refs/heads/master@{#408092} [modify] https://crrev.com/d1e1dd413401a3434537e12f1f12637e7c8bf2fb/third_party/WebKit/Source/core/fetch/ImageResource.cpp [modify] https://crrev.com/d1e1dd413401a3434537e12f1f12637e7c8bf2fb/third_party/WebKit/Source/core/fetch/ImageResourceTest.cpp
,
Jul 27 2016
|
||||
►
Sign in to add a comment |
||||
Comment 1 by hajimehoshi@chromium.org
, Jul 25 2016Status: Assigned (was: Untriaged)