New issue
Advanced search Search tips

Issue 644913 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Document SVGImageForContainer.h and SVGImage.h with class-level comments

Project Member Reported by pdr@chromium.org, Sep 7 2016

Issue description

SVGImageForContainer is not completely obvious and we should document why it's needed with a high-level class-level comment in SVGImageForContainer.h.

I wrote an old doc on this a long time ago (slightly out of date now) that could be repurposed:
https://docs.google.com/document/d/1J3MPhE2nSQz_HQ0Ext6ZIlnMzVCdZbQY9wieR1PA2Tg/view
 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 8 2016

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

commit 51023c152e2acb949eedd5e5cc500622f1bfcc87
Author: pdr <pdr@chromium.org>
Date: Thu Sep 08 21:16:27 2016

Document SVGImageForContainer with a class-level comment

SVGImageForContainer is confusing and needs better documentation. This
patch adds a class-level comment in SVGImageForContainer.h that
describes the dimension and fragment url quirks that require this class.

BUG= 644913 
NOTRY=true

Review-Url: https://codereview.chromium.org/2322803002
Cr-Commit-Position: refs/heads/master@{#417400}

[modify] https://crrev.com/51023c152e2acb949eedd5e5cc500622f1bfcc87/third_party/WebKit/Source/core/svg/graphics/SVGImageForContainer.h

Project Member

Comment 2 by bugdroid1@chromium.org, Sep 9 2016

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

commit 6bfa5aace01353b25e5e468e759f48df3e010f50
Author: pdr <pdr@chromium.org>
Date: Fri Sep 09 19:23:00 2016

Document SVGImage with a class-level comment

The way SVGImage works is surprising. For example, it is not obvious
that we don't use Skia but instead re-use the existing paint pipeline
in Blink, nor that SVGImage contains a detached Page. This patch
adds some high-level documentation to make these quirks more obvious.

Some minor line-wrapping has also been cleaned up.

BUG= 644913 
NOTRY=true

Review-Url: https://codereview.chromium.org/2325733003
Cr-Commit-Position: refs/heads/master@{#417665}

[modify] https://crrev.com/6bfa5aace01353b25e5e468e759f48df3e010f50/third_party/WebKit/Source/core/svg/graphics/SVGImage.h

Comment 3 by pdr@chromium.org, Sep 9 2016

Cc: rbyers@chromium.org
Status: Fixed (was: Assigned)
Summary: Document SVGImageForContainer.h and SVGImage.h with class-level comments (was: Document SVGImageForContainer.h with a class-level comment)
+rbyers just fyi, we finally got some light documentation for these classes.

Sign in to add a comment