summaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2017-09-27Remove FX_CPU=FX_WIN64 build flagDan Sinclair
The FX_WIN64 flag is a FX_OS flag, so comparing against FX_CPU doesn't make sense. Removed check. Bug: pdfium:906 Change-Id: I029de8cf89ade590f705556351416f4759bca87a Reviewed-on: https://pdfium-review.googlesource.com/14812 Commit-Queue: Nicolás Peña Moreno <npm@chromium.org> Reviewed-by: Nicolás Peña Moreno <npm@chromium.org>
2017-09-27Cleanup FX macrosDan Sinclair
This CL renames the FX_OS defines to have _OS_ in their names and drops the _DESKTOP suffix. The FXM defines have been changed to just FX. Change-Id: Iab172fba541713b5f6d14fb8098baf68e3364c74 Reviewed-on: https://pdfium-review.googlesource.com/14833 Reviewed-by: Tom Sepez <tsepez@chromium.org> Commit-Queue: dsinclair <dsinclair@chromium.org>
2017-09-26Remove _FX_IOS_chromium/3225Dan Sinclair
The _FX_IOS_ define is never defined, so it isn't useful to check _FX_OS_ against. Remove. Change-Id: I90b50a1a0dc165073ed223cbfe861b9a227818dd Reviewed-on: https://pdfium-review.googlesource.com/14831 Commit-Queue: dsinclair <dsinclair@chromium.org> Reviewed-by: Tom Sepez <tsepez@chromium.org>
2017-09-26Fix checks for FX_WIN64_DESKTOPDan Sinclair
Several of the OS checks were mis-typed as FX_WIN64 instead of FX_WIN64_DESKTOP. This CL updates the defines to check for the correct OS define. Bug: pdfium:906 Change-Id: Ib58a6472d1bc26c34d509edf32ac00b18d852a3b Reviewed-on: https://pdfium-review.googlesource.com/14813 Commit-Queue: dsinclair <dsinclair@chromium.org> Reviewed-by: Lei Zhang <thestig@chromium.org>
2017-09-26Confirm that a global palette has data before attempting to use itRyan Harrison
Previous implementation assumed that if the local colour palette was not specified and the global palette had its size specified, then use the global. If the global palette is disable, it will not have data, but it may have a size. Technically the size is giberish in this case, but the value is allowed to be non-zero, so isn't a sufficient check. BUG=chromium:768089 Change-Id: Iaec15fcd65f3983056df7d56d29118a516334cd9 Reviewed-on: https://pdfium-review.googlesource.com/14819 Reviewed-by: dsinclair <dsinclair@chromium.org> Commit-Queue: Ryan Harrison <rharrison@chromium.org>
2017-09-26Move LZW decoder out of fx_gifRyan Harrison
CGifLZWDecoder has been moved out into its own file, name changed to CFX_LZWDecoder, member variable names updated, creation pattern changed, and unit tests added. Wrt the creation pattern, there is no longer a constructor and 2 initialization methods that need to be called. Instead all of the initialization is done as part of the constructor. A wrapper has been added for generating a std::unique_ptr<CFX_LZWDecoder>, so that params can be validated. BUG=pdfium:900,pdfium:901,pdfium:903,pdfium:904 Change-Id: Idcbe773f7fb18b08e64d5a89bfd87d4801332c53 Reviewed-on: https://pdfium-review.googlesource.com/14814 Reviewed-by: Tom Sepez <tsepez@chromium.org> Commit-Queue: Ryan Harrison <rharrison@chromium.org>
2017-09-26Hide FX_HandleParentPathDan Sinclair
This CL moves FX_HandleParentPath to be a private inner class of CFX_FontSourceEnum_File. Change-Id: I5e2d16c4d78457a28e1833d0b937547019cdece6 Reviewed-on: https://pdfium-review.googlesource.com/14818 Reviewed-by: Henrique Nakashima <hnakashima@chromium.org> Commit-Queue: dsinclair <dsinclair@chromium.org>
2017-09-26Share public part of CFGAS_FontMgr declarationDan Sinclair
This CL moves the public part of the CFGAS_FontMgr declaration to be shared across all platforms. Change-Id: I92e587fe06611c92f72746477775802c2f3b8b57 Reviewed-on: https://pdfium-review.googlesource.com/14817 Reviewed-by: Henrique Nakashima <hnakashima@chromium.org> Commit-Queue: dsinclair <dsinclair@chromium.org>
2017-09-26Move font source into CFGAS_FontMgrDan Sinclair
This CL moves the font source into the font manager so it doesn't need to be stored by each caller (and removes the platform dependant code). The |EnumFonts| method is exposed on the CFGAS_FontMgr so the caller can clear the manager if the enumeration fails. Change-Id: Iecce3d2e09ff01152b7bb79a34fe2b728320da9c Reviewed-on: https://pdfium-review.googlesource.com/14816 Reviewed-by: Henrique Nakashima <hnakashima@chromium.org> Commit-Queue: dsinclair <dsinclair@chromium.org>
2017-09-26Remove fx_text_int.hDan Sinclair
The CFX_SizeGlyphCache is only used internally by the CFX_FaceCache. This CL moves the definition to that file and removes fx_text_int.h Change-Id: I38f26f437f0eaa72492995ae7442d5c38ed2f229 Reviewed-on: https://pdfium-review.googlesource.com/14771 Reviewed-by: Ryan Harrison <rharrison@chromium.org> Commit-Queue: dsinclair <dsinclair@chromium.org>
2017-09-26Remove font default paramsDan Sinclair
This CL removes some default parameters from the font code. Change-Id: If6f6e09c93474e1e8b7ffaae53eb37dd40c32fab Reviewed-on: https://pdfium-review.googlesource.com/14815 Reviewed-by: Ryan Harrison <rharrison@chromium.org> Commit-Queue: dsinclair <dsinclair@chromium.org>
2017-09-26Rename CFPDF_* classesDan Sinclair
This CL removes the CFPDF_* prefix. The classes have been renamed to their FPDF_* name with Context appended to the end. This makes it easier to differentiate things like CPDF_DataAvail vs CFPDF_DataAvail as the former is now FPDF_AvailContext. Change-Id: I4a97727b3418de268c8494eaca2ed413b5cc2954 Reviewed-on: https://pdfium-review.googlesource.com/14810 Reviewed-by: Art Snake <art-snake@yandex-team.ru> Commit-Queue: dsinclair <dsinclair@chromium.org>
2017-09-25Update comments regarding dictionary keys in public APIS.chromium/3224Lei Zhang
Change-Id: I6420fb5a92c4121d68e9db91c822ea53655ae605 Reviewed-on: https://pdfium-review.googlesource.com/14730 Reviewed-by: dsinclair <dsinclair@chromium.org> Commit-Queue: Lei Zhang <thestig@chromium.org>
2017-09-25Hide CFX_UniqueKeyGenDan Sinclair
The CFX_UniqueKeyGen struct is only used in cfx_facecache.cpp. This CL moves the struct to the anonymous namespace of that file. Change-Id: I1ec85126070e2e0446abc5639cea8465cb490f1a Reviewed-on: https://pdfium-review.googlesource.com/14770 Commit-Queue: dsinclair <dsinclair@chromium.org> Reviewed-by: Henrique Nakashima <hnakashima@chromium.org>
2017-09-25API: rename FPDFPage_CountObject() to FPDFPage_CountObjects()Miklos Vajna
Keep the old name, but mark it as deprecated and don't use it in own code. Change-Id: I17b88dccacf494a03562c0d003cfcb2bc03bf430 Reviewed-on: https://pdfium-review.googlesource.com/14750 Reviewed-by: dsinclair <dsinclair@chromium.org> Commit-Queue: dsinclair <dsinclair@chromium.org>
2017-09-22Fix UAF after destroying a widget during OnBeforeKeyStroke().chromium/3223Henrique Nakashima
Bug: chromium:766957 Change-Id: I61b282059fb4fc2c8ba6dafc502f030f31dd324d Reviewed-on: https://pdfium-review.googlesource.com/14710 Commit-Queue: Henrique Nakashima <hnakashima@chromium.org> Reviewed-by: Tom Sepez <tsepez@chromium.org>
2017-09-22Move C-style functions into CGifContextRyan Harrison
There are a number of functions in fx_gix.h/.cpp that take in a pointer to a CGifContext as their first parameter. Moving these to be methods in that class. BUG=pdfium:902 Change-Id: I8c583ab9e42dda4241aa8a6f865408a94fa6d273 Reviewed-on: https://pdfium-review.googlesource.com/14670 Commit-Queue: Ryan Harrison <rharrison@chromium.org> Reviewed-by: Tom Sepez <tsepez@chromium.org>
2017-09-22Fix crash when rendering invalid GIFRyan Harrison
The core fix in this CL is a change to how LWZ decompression works, so that when the min code table size and the color palette size are different, color codes after the end of the defined color palette are considered errors. This CL also introduces a bunch of tweaks to the call return path, since there were multiple locations where the GIF decode failing status was being dropped on the floor, so the end widget would have a bitmap with the default colour in it, instead of nothing. BUG=chromium:616671 Change-Id: Id6f40d552dc24650c91e9903f710ff2fa63bc774 Reviewed-on: https://pdfium-review.googlesource.com/14630 Commit-Queue: Ryan Harrison <rharrison@chromium.org> Reviewed-by: dsinclair <dsinclair@chromium.org>
2017-09-22API: rename FPDFPath_CountPoint() to FPDFPath_CountSegments()Miklos Vajna
This was the last place talking about "point objects". Change-Id: Ib5f5d5b8efdd674ca5902ff82c2bd4d9f2db8bb1 Reviewed-on: https://pdfium-review.googlesource.com/14651 Commit-Queue: dsinclair <dsinclair@chromium.org> Reviewed-by: dsinclair <dsinclair@chromium.org>
2017-09-21Rename CPDF_SyntaxParser::GetObject to GetObjectBodyArtem Strygin
Change-Id: Ia8fd1893f862391fbada396f505ac16985b59cd6 Reviewed-on: https://pdfium-review.googlesource.com/12850 Commit-Queue: Art Snake <art-snake@yandex-team.ru> Reviewed-by: dsinclair <dsinclair@chromium.org>
2017-09-21Move CFX_WeakPtr to WeakPtrDan Sinclair
This CL renames CFX_WeakPtr to WeakPtr and moves into the fxcrt namespace. Bug: pdfium:898 Change-Id: Ide50a8afeb1e987c48c8fbd103898745c9199d6a Reviewed-on: https://pdfium-review.googlesource.com/14621 Commit-Queue: dsinclair <dsinclair@chromium.org> Commit-Queue: Tom Sepez <tsepez@chromium.org> Reviewed-by: Tom Sepez <tsepez@chromium.org>
2017-09-21Move CFX_UnownedPtr to UnownedPtrDan Sinclair
This CL moves CFX_UnownedPtr to UnownedPtr and places in the fxcrt namespace. Bug: pdfium:898 Change-Id: I6d1fa463f365e5cb3aafa8c8a7a5f7eff62ed8e0 Reviewed-on: https://pdfium-review.googlesource.com/14620 Reviewed-by: Tom Sepez <tsepez@chromium.org> Commit-Queue: dsinclair <dsinclair@chromium.org>
2017-09-21Move CFX_StringPoolTemplate to StringPoolTemplateDan Sinclair
This CL moves CFX_StringPoolTemplate to StringPoolTemplate and places in the fxcrt namespace. Bug: pdfium:898 Change-Id: I41c8f23d6ad814d65cd154092143efaa630a96d1 Reviewed-on: https://pdfium-review.googlesource.com/14619 Reviewed-by: Tom Sepez <tsepez@chromium.org> Commit-Queue: dsinclair <dsinclair@chromium.org>
2017-09-21Move CFX_StringDataTemplate to StringDataTemplateDan Sinclair
This CL renames CFX_StringDataTemplate to StringDataTemplate and moves into the fxcrt namespace. Bug: pdfium:898 Change-Id: I1c1e5ae674c3cca34fd595272e9eebc9346ed6ac Reviewed-on: https://pdfium-review.googlesource.com/14618 Reviewed-by: Tom Sepez <tsepez@chromium.org> Commit-Queue: dsinclair <dsinclair@chromium.org>
2017-09-21Move CFX_SharedCopyOnWrite to SharedCopyOnWriteDan Sinclair
This CL renames CFX_SharedCopyOnWrite to SharedCopyOnWrite and moves to the fxcrt namespace. Bug: pdfium:898 Change-Id: Iced796b9f341407720e2a88f11d1916df56fe68c Reviewed-on: https://pdfium-review.googlesource.com/14617 Commit-Queue: dsinclair <dsinclair@chromium.org> Reviewed-by: Tom Sepez <tsepez@chromium.org>
2017-09-21Rename CFX_RetainPtr to RetainPtrDan Sinclair
This CL renames CFX_RetainPtr to RetainPtr and places in the fxcrt namespace. Bug: pdfium:898 Change-Id: I8798a9f79cb0840d3f037e8d04937cedd742914e Reviewed-on: https://pdfium-review.googlesource.com/14616 Reviewed-by: Tom Sepez <tsepez@chromium.org> Commit-Queue: dsinclair <dsinclair@chromium.org>
2017-09-21Cleanup WidgetMgr ownershipDan Sinclair
The CXFA_FFApp can get the FWLWidgetMgr from the CFWL_App and does not need to store a copy. This removes the need to pass the delegate into the GetFWLAdapterWidgetMgr() method and we can set the capabilities directy. Change-Id: If4794d07fa0fb5dd32b816a21f04e6d7710886ca Reviewed-on: https://pdfium-review.googlesource.com/14611 Commit-Queue: dsinclair <dsinclair@chromium.org> Reviewed-by: Henrique Nakashima <hnakashima@chromium.org>
2017-09-21Move CFX_Observable to ObservableDan Sinclair
This CL renames CFX_Observable to Observable and moves into the fxcrt namespace. The test suite names were updated to ObservedPtr from fxcrt. Bug: pdfium:898 Change-Id: Ia507482bc0a1d6d8bbf12c0c55c4a88f6c8910e2 Reviewed-on: https://pdfium-review.googlesource.com/14615 Reviewed-by: Tom Sepez <tsepez@chromium.org> Commit-Queue: dsinclair <dsinclair@chromium.org>
2017-09-21Rename CXFA_FFApp::GetWidgetMgr to make it clear the typechromium/3222Dan Sinclair
The CXFA_FFApp::GetWidgetMgr has two return types. This CL renames the methods to make it clear which one is being used. Change-Id: Id4fdaa33c1e6079afd2f0f7dcc77ae46f1c07ac2 Reviewed-on: https://pdfium-review.googlesource.com/14610 Reviewed-by: Henrique Nakashima <hnakashima@chromium.org> Commit-Queue: dsinclair <dsinclair@chromium.org>
2017-09-21Move CFGAS_PDFFontMgr to the CXFA_FFDocDan Sinclair
This CL removes the map of CXFA_FFDoc -> CFGAS_PDFFontMgr from the CXFA_FontMgr and moves the storage of the PDFFontMgr into the CXFA_FFDoc itself. Change-Id: Ia725ddd94b1ac2c9504bfd6370040ab6ac01e272 Reviewed-on: https://pdfium-review.googlesource.com/14590 Reviewed-by: Henrique Nakashima <hnakashima@chromium.org> Commit-Queue: dsinclair <dsinclair@chromium.org>
2017-09-21Rename CXFA_DefFontMgr to CFGAS_DefaultFontManagerDan Sinclair
This CL removes any XFA requirements on the default font manager and moves it to the fgas/ directory. Some helper methods from CXFA_FontMgr are moved into the fgas/fgas_fontutils.h files. Change-Id: I17ba8dc2bd60085c17d8a3328f6625675a82787f Reviewed-on: https://pdfium-review.googlesource.com/14570 Reviewed-by: Henrique Nakashima <hnakashima@chromium.org> Commit-Queue: dsinclair <dsinclair@chromium.org>
2017-09-21Move CFX_MaybeOwned to fxcrt::MaybeOwnedDan Sinclair
This CL moves CFX_MaybeOwned into the fxcrt namespace and removes the CFX_ prefix. The test names for maybe owned were updated to be in the MaybeOned test suite instead of the fxcrt suite. Bug: pdfium:898 Change-Id: I0c07057d66c8610e7b19133094b4507fff725e76 Reviewed-on: https://pdfium-review.googlesource.com/14470 Commit-Queue: dsinclair <dsinclair@chromium.org> Reviewed-by: Tom Sepez <tsepez@chromium.org>
2017-09-21Move CFX_AutoRestorer to fxcrt::AutoRestorerDan Sinclair
This CL renames CFX_AutoRestorer to just AutoRestorer and places in the fxcrt namespace. Bug: pdfium:898 Change-Id: Id9f36df94e95f3b2a55054bc198ca1bfd249ee3d Reviewed-on: https://pdfium-review.googlesource.com/14450 Commit-Queue: dsinclair <dsinclair@chromium.org> Reviewed-by: Tom Sepez <tsepez@chromium.org>
2017-09-21Move CXFA_PDFFontMgr to CFGAS_PDFFontMgrDan Sinclair
XFA bits passed into the PDFFontMgr are just used to get the CPDF_Document and the CFGAS_FontMgr. This CL changes the code to pass those two things into the constructor. Then, the fxfa/cxfa_pdffontmgr.* code has been moved to fgas/cfgas_pdffontmgr.* to show it doesn't depend on any of the XFA classes. Change-Id: I21d791869e2a32ffedfd7c87bb3bbd035232d991 Reviewed-on: https://pdfium-review.googlesource.com/14550 Commit-Queue: dsinclair <dsinclair@chromium.org> Reviewed-by: Henrique Nakashima <hnakashima@chromium.org> Reviewed-by: Tom Sepez <tsepez@chromium.org>
2017-09-21Remove old TODODan Sinclair
I don't think this is valid anymore, removing TODO. Change-Id: I69cfbf499fe9f259f664d30be850604a246ed6dd Reviewed-on: https://pdfium-review.googlesource.com/14531 Reviewed-by: Ryan Harrison <rharrison@chromium.org> Commit-Queue: dsinclair <dsinclair@chromium.org>
2017-09-21Add embeddertest for CFWL_EditDan Sinclair
This CL adds two mouse selection tests for CFWL_Edit. In order to do so the needed selection code was added to the XFA widget handler and plumbed down to the CFWL_Edit field as needed. Bug: pdfium:840 Change-Id: Ia3b5f5d191494a4579c01524df8fb35b24cc0085 Reviewed-on: https://pdfium-review.googlesource.com/14530 Reviewed-by: Ryan Harrison <rharrison@chromium.org> Commit-Queue: dsinclair <dsinclair@chromium.org>
2017-09-20Add FPDFPath_GetPoint() APIchromium/3221Miklos Vajna
Combined with the previously added FPDFPath_CountPoint(), this allows getting the coordinates of all points of a path. Change-Id: Ic969723d4b01ee427498d38ce323c74147b87a9c Reviewed-on: https://pdfium-review.googlesource.com/14111 Commit-Queue: dsinclair <dsinclair@chromium.org> Reviewed-by: dsinclair <dsinclair@chromium.org>
2017-09-20Add bounds checks in CAgg_PathData::BuildPathDan Sinclair
When working with LineTo and BezierTo commands, verify we are within the bounds of the path data before accessing elements. Bug: pdfium:899 Change-Id: Iae9f9f3d0e5dbaf8d5452b86961ab8c79a6210f1 Reviewed-on: https://pdfium-review.googlesource.com/14490 Reviewed-by: Lei Zhang <thestig@chromium.org> Commit-Queue: dsinclair <dsinclair@chromium.org>
2017-09-20Add in missed parse recursion depth checksRyan Harrison
Some of the calls in CXFA_FMParser on the prase recursion had been missed when adding in the parse depth limiting logic. The fuzzers found them. BUG=chromium:759295 Change-Id: Iad54beb356c4c555908797d4b58a42549c006e9e Reviewed-on: https://pdfium-review.googlesource.com/14510 Reviewed-by: dsinclair <dsinclair@chromium.org> Commit-Queue: Ryan Harrison <rharrison@chromium.org>
2017-09-20Implement CFDE_TextEditEngine::GetIndex* methods.Dan Sinclair
This CL adds unittests and implementations for the text edit engine methods to get various indexes based on cursor position. The |RebuildPieces| method was fixed to correctly keep track of character position when dealing with BIDI characters. Change-Id: Ie3c5ee5d63bfd00f6f0cdcb1c6fcfe6e05bba50e Reviewed-on: https://pdfium-review.googlesource.com/14430 Commit-Queue: dsinclair <dsinclair@chromium.org> Reviewed-by: Ryan Harrison <rharrison@chromium.org>
2017-09-20Convert FindNextBreakPos to return positionDan Sinclair
This CL changes FindNextBreakPos to return the found index instead of requiring a call to GetAt(). This also fixes the issue that we may return -1 from GetAt which would cause issues in the BoundsForWordAt method when it gets shoved into a size_t variable. Change-Id: I29e09de5d0837921a027208fc2471a9b1de287f1 Reviewed-on: https://pdfium-review.googlesource.com/14293 Reviewed-by: Ryan Harrison <rharrison@chromium.org> Commit-Queue: dsinclair <dsinclair@chromium.org>
2017-09-20Cleanup CPWL_Caret codeDan Sinclair
Add some early returns where possible. Change-Id: I9c53435e6e927057b085b1ccaa30a1637c185486 Reviewed-on: https://pdfium-review.googlesource.com/14350 Commit-Queue: dsinclair <dsinclair@chromium.org> Reviewed-by: Tom Sepez <tsepez@chromium.org>
2017-09-20Get system lcms2 config using pkg-config.Lei Zhang
BUG=chromium:765914 Change-Id: I187507cd87ff5d88be37ce07216ffa3d7ff7bb83 Reviewed-on: https://pdfium-review.googlesource.com/14411 Commit-Queue: dsinclair <dsinclair@chromium.org> Reviewed-by: dsinclair <dsinclair@chromium.org>
2017-09-20Isolate lcms2 usage to a few files.Lei Zhang
Then set up the right dependencies for them. BUG=chromium:765914 Change-Id: I036cd888c741927d5efe0d020c6676f169e7cbb1 Reviewed-on: https://pdfium-review.googlesource.com/14410 Commit-Queue: dsinclair <dsinclair@chromium.org> Reviewed-by: dsinclair <dsinclair@chromium.org>
2017-09-20Forward declare codec modules in fx_codec.h.Lei Zhang
Change-Id: I020b862619f3f93f71dbb027b9e799d78744f686 Reviewed-on: https://pdfium-review.googlesource.com/14391 Commit-Queue: dsinclair <dsinclair@chromium.org> Reviewed-by: dsinclair <dsinclair@chromium.org>
2017-09-20Move CCodec_ScanlineDecoder to its own file.Lei Zhang
Change-Id: Icacf877e2b66ca7d49637dcf9eaec0f99bcdd8bb Reviewed-on: https://pdfium-review.googlesource.com/14390 Commit-Queue: dsinclair <dsinclair@chromium.org> Reviewed-by: dsinclair <dsinclair@chromium.org>
2017-09-19Fix draw order of popup annotations.Henrique Nakashima
Bug: chromium:765375 Change-Id: Ifdbd33ca92ec7105778788f891095eaf280c18cf Reviewed-on: https://pdfium-review.googlesource.com/14370 Commit-Queue: Henrique Nakashima <hnakashima@chromium.org> Reviewed-by: dsinclair <dsinclair@chromium.org>
2017-09-19Remove unneeded core/fxcodec/fx_codec.h includes.chromium/3220Lei Zhang
Change-Id: I703321108712e8c4a11a8343ecf8b1a8804c1d1a Reviewed-on: https://pdfium-review.googlesource.com/14352 Reviewed-by: Ryan Harrison <rharrison@chromium.org> Reviewed-by: dsinclair <dsinclair@chromium.org> Commit-Queue: Lei Zhang <thestig@chromium.org>
2017-09-19Default to c++11 instead of c++14.Lei Zhang
PDFium has users that may not be ready for C++14 yet. Change-Id: I99de1c8126fd94ac728653262c05e5d26b1734f3 Reviewed-on: https://pdfium-review.googlesource.com/14351 Reviewed-by: Tom Sepez <tsepez@chromium.org> Commit-Queue: Lei Zhang <thestig@chromium.org>
2017-09-19Setting focus on a widget may destroy the widgetDan Sinclair
When a widget has focus set, this can trigger an Invalidation call which can trigger a page and annotation reload. This reload can destroy the current widget we're handling. This CL adds ObservedPtrs as needed so we can make sure the widgets are still alive after we've done the Invalidation. Bug: chromium:765921 Change-Id: I51cd24aa1ebd96abe9478efef5130a4e568dac1a Reviewed-on: https://pdfium-review.googlesource.com/14290 Commit-Queue: dsinclair <dsinclair@chromium.org> Reviewed-by: Tom Sepez <tsepez@chromium.org>