Summary: | [chromium] Add OpaqueRegionSkia::currentTrackingOpaqueRect | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alok Priyadarshi <alokp> | ||||
Component: | Layout and Rendering | Assignee: | Alok Priyadarshi <alokp> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | danakj, enne, junov, senorblanco, webkit.review.bot | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Alok Priyadarshi
2013-01-07 15:38:49 PST
Created attachment 181590 [details]
Patch
Comment on attachment 181590 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=181590&action=review > Source/WebCore/platform/graphics/skia/OpaqueRegionSkia.cpp:431 > +SkRect& OpaqueRegionSkia::currentTrackingOpaqueRect() This is fine.. it's adding an extra function call to the stack though, instead of using the inline method. Is this worth it for some reason? I'm not sure of the motivation here. What's wrong with the current method? This patch comes from https://bugs.webkit.org/show_bug.cgi?id=105051, which I decided to abandon. I am trying to extract all the minor cleanup changes and land them separately. The motivation is to reduce duplicated code and potential for bugs. The function arguments (m_opaqueRect, m_canvasLayerStack) are duplicated in all call sites. (In reply to comment #3) > This patch comes from https://bugs.webkit.org/show_bug.cgi?id=105051, which I decided to abandon. I am trying to extract all the minor cleanup changes and land them separately. > > The motivation is to reduce duplicated code and potential for bugs. The function arguments (m_opaqueRect, m_canvasLayerStack) are duplicated in all call sites. I don't see this as a particularly dangerous pattern, but I don't feel strongly either way. I'll defer to senorblanco. Comment on attachment 181590 [details]
Patch
Seems ok. r=me
Comment on attachment 181590 [details] Patch Clearing flags on attachment: 181590 Committed r139083: <http://trac.webkit.org/changeset/139083> All reviewed patches have been landed. Closing bug. |