RESOLVED FIXED 106267
[chromium] Add OpaqueRegionSkia::currentTrackingOpaqueRect 
https://bugs.webkit.org/show_bug.cgi?id=106267
Summary [chromium] Add OpaqueRegionSkia::currentTrackingOpaqueRect
Alok Priyadarshi
Reported 2013-01-07 15:38:49 PST
Minor implementation cleanup. Make currentTrackingOpaqueRect() a member function of OpaqueRegionSkia so that we do not need to pass member variables everywhere.
Attachments
Patch (4.66 KB, patch)
2013-01-07 16:03 PST, Alok Priyadarshi 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Alok Priyadarshi
Comment 1 2013-01-07 16:03:31 PST
Created attachment 181590 [details] Patch
Dana Jansens
Comment 2 2013-01-08 10:06:39 PST
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?
Alok Priyadarshi
Comment 3 2013-01-08 10:38:49 PST
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.
Dana Jansens
Comment 4 2013-01-08 10:40:39 PST
(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.
Stephen White
Comment 5 2013-01-08 11:02:05 PST
Comment on attachment 181590 [details] Patch Seems ok. r=me
WebKit Review Bot
Comment 6 2013-01-08 11:17:18 PST
Comment on attachment 181590 [details] Patch Clearing flags on attachment: 181590 Committed r139083: <http://trac.webkit.org/changeset/139083>
WebKit Review Bot
Comment 7 2013-01-08 11:17:21 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.