New issue
Advanced search Search tips

Issue 717697 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 4
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug

Blocked on:
issue 717700
issue 717698

Blocking:
issue 717699



Sign in to add a comment

Image decoder code should be entirely inside Blink namespace

Project Member Reported by cblume@chromium.org, May 2 2017

Issue description

With the recent Blink renaming, GIFImageReader.h / .cpp did not get updates. When investigating, we found it is because GIFImageReader is not inside the blink namespace. The tool only updated contents inside the blink namespace.

In addition to this, free functions that are in an anonymous namespace also did not get updated. The anonymous namespace should be nested inside the blink namespace.
 
Blocking: 717698
Blocking: 717699
Blocking: 717700
In light of in-progress https://codereview.chromium.org/2565323003/, which deletes GIFImageReader, is it worth modifying this file?
(Leaving the style alone will also help make sure that we apply any real changes that have been made to that file since I forked it into Skia.)
Blockedon: 717698 717700
Blocking: -717698 -717700
For GIFImageReader, it is probably a good idea to leave it alone.
Style changes aren't fixing immediate problems. It is only a problem if things diverge and the code becomes inconsistent and difficult to read, which should no longer be a concern for gifs.

However, other free functions that should be moved into the blink namespace.
An example is: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/image-decoders/png/PNGImageReader.cpp?q=PNGImageReader+package:%5Echromium$&dr=CSs&l=54

There is a bug specifically for GIFImageReader.h / .cpp:  https://crbug.com/717698 
And there is a bug specifically for the free functions: https://crbug.com/717700
Status: Fixed (was: Assigned)

Sign in to add a comment