[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


Dmitry Titov <dimich at chromium.org> changed:

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




--- Comment #2 from Dmitry Titov <dimich at chromium.org>  2009-11-12 13:00:54 PST ---
(From update of attachment 43083)
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.

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