New issue
Advanced search Search tips

Issue 591921 link

Starred by 2 users

Issue metadata

Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocking:
issue 561879



Sign in to add a comment

IcoImageDecoderTest and BmpImageDecoderTest need to be moved into blink

Project Member Reported by esprehn@chromium.org, Mar 4 2016

Issue description

Blink is vastly simplifying the public API as per the Onion Soup plan and won't be exposing APIs to content for unit testing classes inside blink. These tests need to be moved into blink.

Note that these tests are also extremely unfortunate since they depend on src-internal meaning the code looks untested and the tests don't actually run on any of the try jobs so external contributors can't work in that code. We should really upstream the images or switch to public images. For example we have a bunch of BMP unit tests now, though we apparently don't have ico ones.
 
Actually it looks like we do have ico tests, there's stuff like BitmapImageTest::icoHasWrongFrameDimensions.

So we should either upstream these images or just delete the tests.
I would be amazed if the public images tested anywhere remotely close to what the private images do.

Upstreaming them as-is is dangerous as they were scraped without attribution.

Making a new set of images from scratch to test all the cases here would be possible but extremely laborious.
Well step #1 is the move the tests into blink. Would you be able to do that?
Someday?  I currently have about 400% more tasks than I have time :(

I haven't landed anything functional in Blink in a very long time, but I assume this wouldn't be too hard to do.

Comment 5 by tkent@chromium.org, Jun 23 2016

Components: -Blink>Architecture Blink>Internals
Renaming Blink>Architecture to Blink>Internals

Sign in to add a comment