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

Issue 770886 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug
Team-Accessibility



Sign in to add a comment

AXTree::Unserialize() CHECK failure in PdfAccessibilityTree::Finish().

Project Member Reported by thestig@chromium.org, Oct 2 2017

Issue description

Chrome Version: 61.x.
OS: Windows

Crash report id: ec26879126faecf2

0x00007ff96e125ff7	(chrome_child.dll -pdf_accessibility_tree.cc:195 )	pdf::PdfAccessibilityTree::Finish()
0x00007ff96e1267d7	(chrome_child.dll -pdf_accessibility_tree.cc:184 )	pdf::PdfAccessibilityTree::SetAccessibilityPageInfo(PP_PrivateAccessibilityPageInfo const &,std::vector<PP_PrivateAccessibilityTextRunInfo,std::allocator<PP_PrivateAccessibilityTextRunInfo> > const &,std::vector<PP_PrivateAccessibilityCharInfo,std::allocator<PP_PrivateAccessibilityCharInfo> > const &)
0x00007ff96e1252f8	(chrome_child.dll -pepper_pdf_host.cc:224 )	pdf::PepperPDFHost::OnHostMsgSetAccessibilityPageInfo(ppapi::host::HostMessageContext *,PP_PrivateAccessibilityPageInfo const &,std::vector<PP_PrivateAccessibilityTextRunInfo,std::allocator<PP_PrivateAccessibilityTextRunInfo> > const &,std::vector<PP_PrivateAccessibilityCharInfo,std::allocator<PP_PrivateAccessibilityCharInfo> > const &)
0x00007ff96e12591e	(chrome_child.dll -pepper_pdf_host.cc:91 )	pdf::PepperPDFHost::OnResourceMessageReceived(IPC::Message const &,ppapi::host::HostMessageContext *)
...

 
Labels: Stability-Crash
Looks like the vast majority of these are in Print Preview. Based on the stack traces I wonder if this is happening when the window is closed while we're in the middle of loading PDF accessibility data?

I'd love for a way to repro this, otherwise digging into the Win minidump might be the best bet.

Could also try a speculative fix, but there are very few crashes on beta/dev - only stable.

Worst case, we could disable the CHECK and just abort if we end up in a bad state. If this is only happening during shutdown the impact should be minimal, but if it's happening during other times that wouldn't really be a fix.

This is probably not print preview, though I'm not 100% certain since I don't have a repro. The sample crash has a data:application/pdf;base64,$DATA URL. I don't think that's print preview, which uses chrome://print/id/page_index/print.pdf.

Looking further down the stack trace, it is during destruction though. So maybe changing the CHECK() to an if statement and just returning would avoid the crash and have no functionality impact.
Cc: brajkumar@chromium.org
Just to update the latest behavior, crash instances are observed on latest stable, beta and dev channels for Windows platform. Currently this crash is ranked as number #20 under extension process. Below information provides the comparison between previous and latest channels including total number of instances.

+---------------------------------------------------+      
| Latest Channel         |    Previous Channel      |
|---------------------------------------------------+
|62.0.3202.62   - 121    |   61.0.3163.100 - 14068  |--> Stable
|62.0.3202.62   - 121    |   62.0.3202.52  - 14     |--> Beta
|63.0.3239.9    - 1      |   63.0.3236.0   - 6      |--> Dev
+---------------------------------------------------+
	
Link to the list of the builds getting crash:
---------------------------------------------
https://crash.corp.google.com/browse?q=product.name%3D%27Chrome%27%20AND%20custom_data.ChromeCrashProto.magic_signature_1.name%3D%27pdf%3A%3APdfAccessibilityTree%3A%3AFinish%27&sql_dialect=dremelsql&ignore_case=false&enable_rewrite=true&omit_field_name=&omit_field_value=&omit_field_opt=%3D#-samplereports,productversion:1000


Thanks!

	
Cc: ranjitkan@chromium.org
Labels: M-64
Just to update so far 8 instances of this crash is observed on M63 Beta# 63.0.3239.30 for Windows OS and is currently ranking 29 when updated. Below link gives in detail about the same:

https://crash.corp.google.com/browse?q=product.name%3D%27Chrome%27%20AND%20custom_data.ChromeCrashProto.ptype%3D%27extension%27%20AND%20custom_data.ChromeCrashProto.magic_signature_1.name%3D%27pdf%3A%3APdfAccessibilityTree%3A%3AFinish%27&sql_dialect=googlesql&ignore_case=false&enable_rewrite=true&omit_field_name=&omit_field_value=&omit_field_opt=%3D#productversion:1000

Instances are also reported on M64 builds as well. Tagging with M64 Milestone.

Can this be addressed.

Thanks.!
Labels: a11y-secondary
Adding crash keys to debug this: https://chromium-review.googlesource.com/c/chromium/src/+/827525

Project Member

Comment 8 by bugdroid1@chromium.org, Dec 14 2017

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

commit 2eee15f39fd4bfaefd0092b830b9385115249696
Author: Dominic Mazzoni <dmazzoni@chromium.org>
Date: Thu Dec 14 19:59:53 2017

Add some crash keys to debug PDF accessibility crashes

Bug:  770886 
Change-Id: I2f15ece913da0d5d7d21369329005a58fd06aa0e
Reviewed-on: https://chromium-review.googlesource.com/827525
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524153}
[modify] https://crrev.com/2eee15f39fd4bfaefd0092b830b9385115249696/components/pdf/renderer/pdf_accessibility_tree.cc

Labels: -Pri-3 Pri-1
Status: Started (was: Assigned)
The crash log worked, I figured it out!

PdfAccessibilityTree::CreateNode calls content::RenderAccessibility::GenerateAXID, which ends up calling BlinkAXTreeSource::ComputeRoot(), which accesses the WebFrame and a WebDocument. If that fails, it returns an empty/uninitialized WebAXObject as the root, which returns -1 from GenerateAXID.

This is probably happening on shutdown. The Document or Frame have been destroyed, but the PepperPDFHost still exists and still gets a callback from the PDF plugin process before it's been shut down.

Project Member

Comment 11 by bugdroid1@chromium.org, Dec 22 2017

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

commit c6f4bc7f662518577c6ecbc1ef2b2c6d7acc748a
Author: Dominic Mazzoni <dmazzoni@chromium.org>
Date: Fri Dec 22 23:44:26 2017

Fix crash in PdfAccessibilityTree.

PdfAccessibilityTree::CreateNode calls
content::RenderAccessibility::GenerateAXID, which ends up calling
BlinkAXTreeSource::ComputeRoot(), which accesses the WebFrame and a
WebDocument. If that fails, it returns an empty/uninitialized
WebAXObject as the root, which returns -1 from GenerateAXID.
A -1 in the accessibility tree isn't necessarily bad, but reusing the
same ID for multiple nodes will quickly lead to a CHECK failure.

Fix it by checking that GenerateAXID is returning valid values
in each plugin callback.

Bug:  770886 
Change-Id: Ibe641d06302253a2e242d59fd723a626b6b6c0cd
Reviewed-on: https://chromium-review.googlesource.com/840883
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526092}
[modify] https://crrev.com/c6f4bc7f662518577c6ecbc1ef2b2c6d7acc748a/components/BUILD.gn
[modify] https://crrev.com/c6f4bc7f662518577c6ecbc1ef2b2c6d7acc748a/components/pdf/renderer/pdf_accessibility_tree.cc
[add] https://crrev.com/c6f4bc7f662518577c6ecbc1ef2b2c6d7acc748a/components/pdf/renderer/pdf_accessibility_tree_browsertest.cc
[modify] https://crrev.com/c6f4bc7f662518577c6ecbc1ef2b2c6d7acc748a/content/public/renderer/render_frame.h
[modify] https://crrev.com/c6f4bc7f662518577c6ecbc1ef2b2c6d7acc748a/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/c6f4bc7f662518577c6ecbc1ef2b2c6d7acc748a/content/renderer/render_frame_impl.h

Status: Fixed (was: Started)
Sorry for the bug resurrection; but I noticed that this bug is referenced in the codebase alongside some crash strings : https://cs.chromium.org/chromium/src/components/pdf/renderer/pdf_accessibility_tree.cc?l=218&rcl=86f8273e2fd5cd87e70f3711250d6edc12da2823

Should those be removed if this is now fixed?
smcgruer: Yes. Thank you for noticing. Care to send me a CL?
Project Member

Comment 15 by bugdroid1@chromium.org, Aug 15

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

commit 06aa0e18ed50d679fa7ac2578bc26ee5ecf4c05e
Author: Stephen McGruer <smcgruer@chromium.org>
Date: Wed Aug 15 18:50:06 2018

Remove crash keys in PDF accessibility tree

The crash that these were meant to help debug has been long fixed.
They are no longer needed.

Bug:  770886 
Change-Id: I7abf99be6cd00b9ae9a99934edaca95fe48e638d
Reviewed-on: https://chromium-review.googlesource.com/1174992
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583341}
[modify] https://crrev.com/06aa0e18ed50d679fa7ac2578bc26ee5ecf4c05e/components/pdf/renderer/pdf_accessibility_tree.cc

Sign in to add a comment