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

Issue 604716 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Last visit > 30 days ago
Closed: Jul 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

Clean up any unnecessary dependencies on Skia

Project Member Reported by tomhud...@chromium.org, Apr 19 2016

Issue description

If we change anything in skia/config/SkUserConfig.h, more than 8000 files recompile. Do we really need so many direct couplings?

I *don't* want to introduce any unnecessary abstraction layers, but this suggests to me we may have more #includes of Skia than strictly necessary.
 
Status: Assigned (was: Untriaged)
This doesn't seem too unbelievable to me. Skia includes appear in platform and gfx code, which is then included in almost all of Blink and a bunch of chromium. Just SkColor alone hits a huge amount of code.
One thing to look at is whether we can trim the header includes some (use fwdecl instead).
Yep, that's one of my hopes. Unfortunately sk_sp<> seems to need #includes where skia::RefPtr could make do with forward declarations.
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 28 2016

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

commit 1a794d387568cadc30e8e585dec2f6514ddef821
Author: tomhudson <tomhudson@google.com>
Date: Thu Apr 28 18:19:18 2016

Trim Skia dependencies in Blink

Reduce coupling by removing Skia #includes from headers, switching to the *correct* Skia #includes, or changing to
forward declarations.

R=fmalita@chromium.org,junov@chromium.org
BUG= 604716 

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

[modify] https://crrev.com/1a794d387568cadc30e8e585dec2f6514ddef821/third_party/WebKit/Source/core/frame/ImageBitmap.cpp
[modify] https://crrev.com/1a794d387568cadc30e8e585dec2f6514ddef821/third_party/WebKit/Source/core/frame/ImageBitmapTest.cpp
[modify] https://crrev.com/1a794d387568cadc30e8e585dec2f6514ddef821/third_party/WebKit/Source/core/html/canvas/CanvasDrawListener.h
[modify] https://crrev.com/1a794d387568cadc30e8e585dec2f6514ddef821/third_party/WebKit/Source/core/imagebitmap/ImageBitmapFactories.h
[modify] https://crrev.com/1a794d387568cadc30e8e585dec2f6514ddef821/third_party/WebKit/Source/modules/canvas2d/CanvasStyle.cpp
[modify] https://crrev.com/1a794d387568cadc30e8e585dec2f6514ddef821/third_party/WebKit/Source/modules/csspaint/PaintRenderingContext2D.h
[modify] https://crrev.com/1a794d387568cadc30e8e585dec2f6514ddef821/third_party/WebKit/Source/platform/DragImageTest.cpp
[modify] https://crrev.com/1a794d387568cadc30e8e585dec2f6514ddef821/third_party/WebKit/Source/platform/graphics/BitmapImageTest.cpp
[modify] https://crrev.com/1a794d387568cadc30e8e585dec2f6514ddef821/third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.h
[modify] https://crrev.com/1a794d387568cadc30e8e585dec2f6514ddef821/third_party/WebKit/Source/platform/graphics/DeferredImageDecoder.h
[modify] https://crrev.com/1a794d387568cadc30e8e585dec2f6514ddef821/third_party/WebKit/Source/platform/graphics/FrameData.cpp
[modify] https://crrev.com/1a794d387568cadc30e8e585dec2f6514ddef821/third_party/WebKit/Source/platform/graphics/FrameData.h
[modify] https://crrev.com/1a794d387568cadc30e8e585dec2f6514ddef821/third_party/WebKit/Source/platform/graphics/GraphicsLayer.h
[modify] https://crrev.com/1a794d387568cadc30e8e585dec2f6514ddef821/third_party/WebKit/Source/platform/graphics/GraphicsTypes.h
[modify] https://crrev.com/1a794d387568cadc30e8e585dec2f6514ddef821/third_party/WebKit/Source/platform/graphics/Image.h
[modify] https://crrev.com/1a794d387568cadc30e8e585dec2f6514ddef821/third_party/WebKit/Source/platform/graphics/ImageFrameGenerator.cpp
[modify] https://crrev.com/1a794d387568cadc30e8e585dec2f6514ddef821/third_party/WebKit/Source/platform/graphics/ImageFrameGenerator.h
[modify] https://crrev.com/1a794d387568cadc30e8e585dec2f6514ddef821/third_party/WebKit/Source/platform/graphics/PaintGeneratedImage.h
[modify] https://crrev.com/1a794d387568cadc30e8e585dec2f6514ddef821/third_party/WebKit/Source/platform/graphics/RecordingImageBufferSurface.h
[modify] https://crrev.com/1a794d387568cadc30e8e585dec2f6514ddef821/third_party/WebKit/Source/platform/graphics/ReplayingCanvas.h
[modify] https://crrev.com/1a794d387568cadc30e8e585dec2f6514ddef821/third_party/WebKit/Source/platform/graphics/StrokeData.cpp
[modify] https://crrev.com/1a794d387568cadc30e8e585dec2f6514ddef821/third_party/WebKit/Source/platform/graphics/StrokeData.h
[modify] https://crrev.com/1a794d387568cadc30e8e585dec2f6514ddef821/third_party/WebKit/Source/platform/graphics/skia/ImagePixelLocker.h
[modify] https://crrev.com/1a794d387568cadc30e8e585dec2f6514ddef821/third_party/WebKit/Source/platform/graphics/skia/SkiaUtils.h
[modify] https://crrev.com/1a794d387568cadc30e8e585dec2f6514ddef821/third_party/WebKit/Source/platform/image-decoders/ImageDecoderTestHelpers.h
[modify] https://crrev.com/1a794d387568cadc30e8e585dec2f6514ddef821/third_party/WebKit/public/platform/WebDisplayItemList.h

Project Member

Comment 5 by bugdroid1@chromium.org, May 27 2016

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

commit 7e34e79b1d047678d872ae73c70d3870fef27693
Author: tomhudson <tomhudson@google.com>
Date: Fri May 27 02:02:48 2016

Remove unused Skia headers from Blink

skia/ext/refptr.h is being deprecated, and was no longer used anyway
in the files in question.

TBR=fmalita@chromium.org
BUG= 604716 ,skia:5077

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

[modify] https://crrev.com/7e34e79b1d047678d872ae73c70d3870fef27693/third_party/WebKit/Source/platform/graphics/CompositorFilterOperations.cpp
[modify] https://crrev.com/7e34e79b1d047678d872ae73c70d3870fef27693/third_party/WebKit/Source/platform/graphics/compositing/PaintArtifactCompositor.cpp

Status: WontFix (was: Assigned)
There doesn't seem to be a lot of easy win here, and I don't expect to spend time working on it; closing.

Sign in to add a comment