In Bug 53906, I add dummy functions to RenderThemeEFl.cpp. But, the added function's parameter is different from other's. Other function : bool RenderThemeEfl::paintSearchFieldResultsButton(RenderObject* o, const PaintInfo& i, const IntRect& rect) bool RenderThemeEfl::paintSearchFieldResultsDecoration(RenderObject* o, const PaintInfo& i, const IntRect& rect) bool RenderThemeEfl::paintSearchFieldCancelButton(RenderObject* o, const PaintInfo& i, const IntRect& rect) ... Added functions : bool RenderThemeEfl::paintMediaMuteButton(RenderObject* renderObject, const PaintInfo& paintInfo, const IntRect& re ct) bool RenderThemeEfl::paintMediaPlayButton(RenderObject* renderObject, const PaintInfo& paintInfo, const IntRect& rect) ... I'd like to change "paintInfo" with "i" in order to sync with other functions.
Created attachment 82316 [details] Patch
Comment on attachment 82316 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=82316&action=review > Source/WebCore/ChangeLog:9 > + Change pamater name of new functions. type: pamater
(In reply to comment #2) > (From update of attachment 82316 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=82316&action=review > > > Source/WebCore/ChangeLog:9 > > + Change pamater name of new functions. > > type: pamater typo even :)
Created attachment 82360 [details] Patch
Hello Antonio :-) Thank you for your review. I fix wrong spelling. And, I change parameter name one more thing. Other functions are using "o" for RenderObject* parameter. So, I change renderObject with o as well.
Comment on attachment 82360 [details] Patch After thinking a bit more about this, we try to avoid abreviations in the source code. Maybe we should renamed it the other way around? Also, most of the parameters are ununsed anyways. I know other ports also abreviate variable names in their respective RenderThemeXXX methods, but ...
I like the original code. Abbreviations are not good.
Comment on attachment 82360 [details] Patch Moving to abbreviations is not the right way to go here. :)
Created attachment 84649 [details] Patch Functions of RenderThemeEfl.cpp have efl coding style. But, WebCore's functions should adhere WebKit coding style. So, I modify the efl style parameter as below, - EvasObject* o -> EvasObject* object - ThemePartCacheEntry* ce -> ThemePartCacheEntry* entry - PaintInfo& i -> PaintInfo& info - RenderObject* o -> RenderObject* object
Comment on attachment 84649 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=84649&action=review Yay! LGTM. > Source/WebCore/platform/efl/RenderThemeEfl.cpp:1119 > +bool RenderThemeEfl::paintMediaCurrentTime(RenderObject* object, const PaintInfo& info, const IntRect& rect) Why info instead of paintInfo?
Comment on attachment 84649 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=84649&action=review > Source/WebCore/platform/efl/RenderThemeEfl.cpp:119 > + struct ThemePartCacheEntry *entry = new struct ThemePartCacheEntry; nit: We had better use OwnPtr<ThemePartCacheEntry> to avoid 'delete'.
(In reply to comment #10) > (From update of attachment 84649 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=84649&action=review > > Yay! LGTM. > > > Source/WebCore/platform/efl/RenderThemeEfl.cpp:1119 > > +bool RenderThemeEfl::paintMediaCurrentTime(RenderObject* object, const PaintInfo& info, const IntRect& rect) > > Why info instead of paintInfo? I'd like to keep sync with other parameters. Other parameters use last name of data type.
(In reply to comment #12) > (In reply to comment #10) > > (From update of attachment 84649 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=84649&action=review > > > > Yay! LGTM. > > > > > Source/WebCore/platform/efl/RenderThemeEfl.cpp:1119 > > > +bool RenderThemeEfl::paintMediaCurrentTime(RenderObject* object, const PaintInfo& info, const IntRect& rect) > > > > Why info instead of paintInfo? > > I'd like to keep sync with other parameters. Other parameters use last name of data type. Thank you for your comment. (In reply to comment #11) > (From update of attachment 84649 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=84649&action=review > > > Source/WebCore/platform/efl/RenderThemeEfl.cpp:119 > > + struct ThemePartCacheEntry *entry = new struct ThemePartCacheEntry; > > nit: We had better use OwnPtr<ThemePartCacheEntry> to avoid 'delete'. Thank you for your guidance. This seem to be processed by new bug. I will make a bug for it.
Comment on attachment 84649 [details] Patch Clearing flags on attachment: 84649 Committed r80324: <http://trac.webkit.org/changeset/80324>
All reviewed patches have been landed. Closing bug.
http://trac.webkit.org/changeset/80324 might have broken EFL Linux Release (Build)