[Webkit-unassigned] [Bug 22579] Avoid high-level WebCore types under platform

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=22579


darin at apple.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #25643|review?                     |review-
               Flag|                            |




------- Comment #2 from darin at apple.com  2008-12-04 09:29 PDT -------
(From update of attachment 25643)
Patch generally looks good. Definitely a step in the right direction.

The name of this function needs to talk about tick marks, not text-match
markers. Not only do we want to avoid types from the rest of WebCore, we want
to avoid hard-coding in the concepts too. So the function should be one that
gives the tick mark implementation what it needs in a form that's about "tick
marks" not about "text match markers", since the latter concept is one that
doesn't make any sense in the context of a scroll view.

I'm thinking that instead of Vector<IntRect> you might want this to be a
Vector<int> of tick mark positions.

> +void FrameView::renderedRectsForTextMatchMarkers(Vector<IntRect>* rects) {
> +    *rects = frame()->document()->renderedRectsForMarkers(DocumentMarker::TextMatch);
> +}

The brace that starts a new function needs to be on a separate line as
described in the WebKit coding style document.

> +    virtual void renderedRectsForTextMatchMarkers(Vector<IntRect>* rects);

We'd normally use a reference, not a pointer, for an "out" parameter like
rects. Also, in declarations we normally omit the name of the argument if the
type speaks for itself. For functions that use an "out" parameter for a return
value rather than the function result we use the word get, so this would be
getRendereredRectsForTextMatchMarkers.

> +    virtual void renderedRectsForTextMatchMarkers(Vector<IntRect>* rects) { }

Important to omit the argument here to avoid unused argument warnings, which we
probably want to turn on at some point.


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.