[Webkit-unassigned] [Bug 31423] [Android] The Android-specific files in platform/android are out of date

[bugzilla-daemon at webkit.org]

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





--- Comment #5 from Andrei Popescu <andreip at google.com>  2009-11-13 11:03:35 PST ---
Thanks Dimitry!

(In reply to comment #2)
> (From update of attachment 43083 [details])
> Looks good. Couple of style-related comments:
> 
> 
> > +PassRefPtr<RenderTheme> RenderTheme::themeForPage(Page* page)
> > +{
> > +    RefPtr<RenderThemeAndroid> androidTheme = new RenderThemeAndroid();
> > +    return androidTheme.release();
> 
> return adoptRef(new RenderThemeAndroid()) ?
> The regular pattern is to have a RenderThemeAndroid::create() - see
> RenderThemeChromiumWin for example.
>

Fixed.

> > +    return RenderSkinCombo::Draw(getCanvasFromInfo(info), obj->node(), rect.x(),
> > +            rect.y(), rect.width(), rect.height());
> 
> Usually the lines longer then 80 characters are not wrapped in WebKit sources.
>

Fixed.


> > Index: WebCore/platform/android/ScreenAndroid.cpp
> >  
> > +#if PLATFORM(ANDROID)
> 
> Is this needed? It seems this file is Android-only.
> 

Good catch, removed.

> > Index: WebCore/platform/android/ScrollViewAndroid.cpp
> > -void ScrollView::platformOffscreenContentRectangle(const IntRect& rect)
> > +/*
> > +    Compute the offscreen parts of the drawn rectangle by subtracting
> > +    vis from rect. This can compute up to four rectangular slices.
> > +*/
> 
> Usually the comments like this use '//' style.

Fixed.

Thanks,
Andrei

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