Bug 135326

Summary:

Cleanup RenderSelectionInfoBase/RenderSelectionInfo/RenderBlockSelectionInfo.

Product:

WebKit

Reporter:

alan <zalan>

Component:

Layout and Rendering

Assignee:

alan <zalan>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bunhere, commit-queue, esprehn+autocc, glenn, gyuyoung.kim, kondapallykalyan, rakuco, rniwa, sergio, simon.fraser

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none

alan

Reported 2014-07-26 20:59:54 PDT

instead of pointer to the renderer.

Attachments
Patch (9.84 KB, patch) 2014-07-26 22:28 PDT, alan	no flags	Details Formatted Diff Diff
Patch (17.00 KB, patch) 2014-07-28 21:00 PDT, alan	no flags	Details Formatted Diff Diff
Patch (21.02 KB, patch) 2014-07-29 07:02 PDT, alan	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

alan

Comment 1 2014-07-26 22:28:41 PDT

Created attachment 235581 [details] Patch

Darin Adler

Comment 2 2014-07-27 23:10:55 PDT

Comment on attachment 235581 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=235581&action=review Good refactoring. > Source/WebCore/ChangeLog:3 > + RenderSelectionInfoBase/RenderSelectionInfo/RenderBlockSelectionInfo should take renderer reference. Bug title is a little more specific than the slightly broader refactoring this patch does. > Source/WebCore/rendering/RenderSelectionInfo.h:37 > + explicit RenderSelectionInfoBase(RenderObject& renderer) Too bad we can’t make the type tighter than RenderObject, but I don’t think we can. > Source/WebCore/rendering/RenderSelectionInfo.h:51 > + void repaintRectangle(const LayoutRect& repaintRect) > + { > + m_renderer.repaintUsingContainer(m_repaintContainer, enclosingIntRect(repaintRect)); > + } I’m not sure this should be in the class definition. I think classes are easier to read if we move multiple-line function definitions out of the class definitions. I’ve been doing that when working on various header files. > Source/WebCore/rendering/RenderSelectionInfo.h:84 > - Vector<LayoutRect> m_rects; > + Vector<LayoutRect> m_rects; // relative to repaint container > LayoutRect m_rect; // relative to repaint container Really unclear how these data members relate. Same is true of the two public functions that expose them; not clear why we have both.

alan

Comment 3 2014-07-28 21:00:32 PDT

Created attachment 235661 [details] Patch

alan

Comment 4 2014-07-28 21:00:51 PDT

Comment on attachment 235661 [details] Patch EWS testing.

alan

Comment 5 2014-07-29 07:02:17 PDT

Created attachment 235686 [details] Patch

alan

Comment 6 2014-07-29 07:02:39 PDT

Comment on attachment 235686 [details] Patch EWS