[webkit-reviews] review denied: [Bug 31423] [Android] The Android-specific files in platform/android are out of date : [Attachment 43083] Bring existing files in platform/android in line with Android 2.0

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Nov 12 13:00:53 PST 2009


Dmitry Titov <dimich at chromium.org> has denied Andrei Popescu
<andreip at google.com>'s request for review:
Bug 31423: [Android] The Android-specific files in platform/android are out of
date
https://bugs.webkit.org/show_bug.cgi?id=31423

Attachment 43083: Bring existing files in platform/android in line with Android
2.0
https://bugs.webkit.org/attachment.cgi?id=43083&action=review

------- Additional Comments from Dmitry Titov <dimich at chromium.org>
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.

> +    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.

> Index: WebCore/platform/android/ScreenAndroid.cpp
>  
> +#if PLATFORM(ANDROID)

Is this needed? It seems this file is Android-only.

> 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.


More information about the webkit-reviews mailing list