Upstream BlackBerry API backing store files

[Jacky Jiang]

Upstream BlackBerry API backing store implementation files.

[Jacky Jiang]

Created attachment 117648 [details]
The patch

[Antonio Gomes]

Comment on attachment 117648 [details]
The patch

View in context: https://bugs.webkit.org/attachment.cgi?id=117648&action=review

Patch looks good, as I know the code, but we have too many minor issues, that another round is needed.

Jacky, please fix all comment styles (including the ones I did not highlight).

> Source/WebKit/ChangeLog:156
> +        (BlackBerry::WebKit::divisors):
> +        (BlackBerry::WebKit::bestDivisor):
> +        (BlackBerry::WebKit::BackingStoreMutexLocker::BackingStoreMutexLocker):
> +        (BlackBerry::WebKit::BackingStoreMutexLocker::~BackingStoreMutexLocker):
> +        (BlackBerry::WebKit::BackingStoreGeometry::backingStoreRect):
> +        (BlackBerry::WebKit::BackingStoreGeometry::backingStoreSize):
> +        (BlackBerry::WebKit::BackingStorePrivate::BackingStorePrivate):
> +        (BlackBerry::WebKit::BackingStorePrivate::~BackingStorePrivate):
> +        (BlackBerry::WebKit::BackingStorePrivate::shouldDirectRenderingToWindow):
> +        (BlackBerry::WebKit::BackingStorePrivate::suspendScreenAndBackingStoreUpdates):
> +        (BlackBerry::WebKit::BackingStorePrivate::resumeScreenAndBackingStoreUpdates):
> +        (BlackBerry::WebKit::BackingStorePrivate::repaint):
> +        (BlackBerry::WebKit::BackingStorePrivate::slowScroll):
> +        (BlackBerry::WebKit::BackingStorePrivate::scroll):
> +        (BlackBerry::WebKit::BackingStorePrivate::scrollingStartedHelper):
> +        (BlackBerry::WebKit::BackingStorePrivate::shouldSuppressNonVisibleRegularRenderJobs):
> +        (BlackBerry::WebKit::BackingStorePrivate::shouldPerformRenderJobs):
> +        (BlackBerry::WebKit::BackingStorePrivate::shouldPerformRegularRenderJobs):
> +        (BlackBerry::WebKit::BackingStorePrivate::startRenderTimer):
> +        (BlackBerry::WebKit::BackingStorePrivate::stopRenderTimer):
> +        (BlackBerry::WebKit::BackingStorePrivate::renderOnTimer):
> +        (BlackBerry::WebKit::BackingStorePrivate::renderOnIdle):
> +        (BlackBerry::WebKit::BackingStorePrivate::willFireTimer):
> +        (BlackBerry::WebKit::BackingStorePrivate::expandedContentsRect):
> +        (BlackBerry::WebKit::BackingStorePrivate::visibleContentsRect):
> +        (BlackBerry::WebKit::BackingStorePrivate::unclippedVisibleContentsRect):
> +        (BlackBerry::WebKit::BackingStorePrivate::shouldMoveLeft):
> +        (BlackBerry::WebKit::BackingStorePrivate::shouldMoveRight):
> +        (BlackBerry::WebKit::BackingStorePrivate::shouldMoveUp):
> +        (BlackBerry::WebKit::BackingStorePrivate::shouldMoveDown):
> +        (BlackBerry::WebKit::BackingStorePrivate::canMoveX):
> +        (BlackBerry::WebKit::BackingStorePrivate::canMoveY):
> +        (BlackBerry::WebKit::BackingStorePrivate::canMoveLeft):
> +        (BlackBerry::WebKit::BackingStorePrivate::canMoveRight):
> +        (BlackBerry::WebKit::BackingStorePrivate::canMoveUp):
> +        (BlackBerry::WebKit::BackingStorePrivate::canMoveDown):
> +        (BlackBerry::WebKit::BackingStorePrivate::backingStoreRectForScroll):
> +        (BlackBerry::WebKit::BackingStorePrivate::setBackingStoreRect):
> +        (BlackBerry::WebKit::BackingStorePrivate::indexesForBackingStoreRect):
> +        (BlackBerry::WebKit::BackingStorePrivate::originOfLastRenderForTile):
> +        (BlackBerry::WebKit::BackingStorePrivate::indexOfLastRenderForTile):
> +        (BlackBerry::WebKit::BackingStorePrivate::indexOfTile):
> +        (BlackBerry::WebKit::BackingStorePrivate::clearAndUpdateTileOfNotRenderedRegion):
> +        (BlackBerry::WebKit::BackingStorePrivate::isCurrentVisibleJob):
> +        (BlackBerry::WebKit::BackingStorePrivate::scrollBackingStore):
> +        (BlackBerry::WebKit::BackingStorePrivate::render):
> +        (BlackBerry::WebKit::BackingStorePrivate::renderDirectToWindow):
> +        (BlackBerry::WebKit::BackingStorePrivate::requestLayoutIfNeeded):
> +        (BlackBerry::WebKit::BackingStorePrivate::renderVisibleContents):
> +        (BlackBerry::WebKit::BackingStorePrivate::renderBackingStore):
> +        (BlackBerry::WebKit::BackingStorePrivate::blitVisibleContents):
> +        (BlackBerry::WebKit::BackingStorePrivate::copyPreviousContentsToBackSurfaceOfWindow):
> +        (BlackBerry::WebKit::BackingStorePrivate::copyPreviousContentsToBackSurfaceOfTile):
> +        (BlackBerry::WebKit::BackingStorePrivate::paintDefaultBackground):
> +        (BlackBerry::WebKit::BackingStorePrivate::blitContents):
> +        (BlackBerry::WebKit::BackingStorePrivate::blitTileRect):
> +        (BlackBerry::WebKit::BackingStorePrivate::blendCompositingSurface):
> +        (BlackBerry::WebKit::BackingStorePrivate::clearCompositingSurface):
> +        (BlackBerry::WebKit::BackingStorePrivate::blitHorizontalScrollbar):
> +        (BlackBerry::WebKit::BackingStorePrivate::blitVerticalScrollbar):
> +        (BlackBerry::WebKit::BackingStorePrivate::isTileVisible):
> +        (BlackBerry::WebKit::BackingStorePrivate::visibleTilesRect):
> +        (BlackBerry::WebKit::BackingStorePrivate::tileVisibleContentsRect):
> +        (BlackBerry::WebKit::BackingStorePrivate::tileUnclippedVisibleContentsRect):
> +        (BlackBerry::WebKit::BackingStorePrivate::tileContentsRect):
> +        (BlackBerry::WebKit::BackingStorePrivate::resetRenderQueue):
> +        (BlackBerry::WebKit::BackingStorePrivate::clearVisibleZoom):
> +        (BlackBerry::WebKit::BackingStorePrivate::resetTiles):
> +        (BlackBerry::WebKit::BackingStorePrivate::updateTiles):
> +        (BlackBerry::WebKit::BackingStorePrivate::updateTilesForScrollOrNotRenderedRegion):
> +        (BlackBerry::WebKit::BackingStorePrivate::resetTile):
> +        (BlackBerry::WebKit::BackingStorePrivate::updateTile):
> +        (BlackBerry::WebKit::BackingStorePrivate::mapFromTilesToTransformedContents):
> +        (BlackBerry::WebKit::BackingStorePrivate::mapFromTransformedContentsToAbsoluteTileBoundaries):
> +        (BlackBerry::WebKit::BackingStorePrivate::mapFromTransformedContentsToTiles):
> +        (BlackBerry::WebKit::BackingStorePrivate::updateTileMatrixIfNeeded):
> +        (BlackBerry::WebKit::BackingStorePrivate::contentsSizeChanged):
> +        (BlackBerry::WebKit::BackingStorePrivate::scrollChanged):
> +        (BlackBerry::WebKit::BackingStorePrivate::transformChanged):
> +        (BlackBerry::WebKit::BackingStorePrivate::orientationChanged):
> +        (BlackBerry::WebKit::BackingStorePrivate::actualVisibleSizeChanged):
> +        (BlackBerry::WebKit::createVisibleTileBufferForWebPage):
> +        (BlackBerry::WebKit::BackingStorePrivate::createSurfaces):
> +        (BlackBerry::WebKit::BackingStorePrivate::createVisibleTileBuffer):
> +        (BlackBerry::WebKit::BackingStorePrivate::originOfTile):
> +        (BlackBerry::WebKit::BackingStorePrivate::minimumNumberOfTilesWide):
> +        (BlackBerry::WebKit::BackingStorePrivate::minimumNumberOfTilesHigh):
> +        (BlackBerry::WebKit::BackingStorePrivate::expandedContentsSize):
> +        (BlackBerry::WebKit::BackingStorePrivate::tileWidth):
> +        (BlackBerry::WebKit::BackingStorePrivate::tileHeight):
> +        (BlackBerry::WebKit::BackingStorePrivate::tileSize):
> +        (BlackBerry::WebKit::BackingStorePrivate::tileRect):
> +        (BlackBerry::WebKit::BackingStorePrivate::renderContents):
> +        (BlackBerry::WebKit::drawDebugRect):
> +        (BlackBerry::WebKit::BackingStorePrivate::blitToWindow):
> +        (BlackBerry::WebKit::BackingStorePrivate::checkerWindow):
> +        (BlackBerry::WebKit::BackingStorePrivate::invalidateWindow):
> +        (BlackBerry::WebKit::BackingStorePrivate::clearWindow):
> +        (BlackBerry::WebKit::BackingStorePrivate::isScrollingOrZooming):
> +        (BlackBerry::WebKit::BackingStorePrivate::setScrollingOrZooming):
> +        (BlackBerry::WebKit::BackingStorePrivate::lockBackingStore):
> +        (BlackBerry::WebKit::BackingStorePrivate::unlockBackingStore):
> +        (BlackBerry::WebKit::BackingStorePrivate::frontState):
> +        (BlackBerry::WebKit::BackingStorePrivate::backState):
> +        (BlackBerry::WebKit::BackingStorePrivate::swapState):
> +        (BlackBerry::WebKit::BackingStorePrivate::windowFrontBufferState):
> +        (BlackBerry::WebKit::BackingStorePrivate::windowBackBufferState):
> +        (BlackBerry::WebKit::BackingStorePrivate::drawSubLayers):
> +        (BlackBerry::WebKit::BackingStorePrivate::isActive):
> +        (BlackBerry::WebKit::BackingStore::BackingStore):
> +        (BlackBerry::WebKit::BackingStore::~BackingStore):
> +        (BlackBerry::WebKit::BackingStore::createSurface):
> +        (BlackBerry::WebKit::BackingStore::suspendScreenAndBackingStoreUpdates):
> +        (BlackBerry::WebKit::BackingStore::resumeScreenAndBackingStoreUpdates):
> +        (BlackBerry::WebKit::BackingStore::isScrollingOrZooming):
> +        (BlackBerry::WebKit::BackingStore::setScrollingOrZooming):
> +        (BlackBerry::WebKit::BackingStore::blitContents):
> +        (BlackBerry::WebKit::BackingStore::repaint):
> +        (BlackBerry::WebKit::BackingStore::hasRenderJobs):
> +        (BlackBerry::WebKit::BackingStore::renderOnIdle):
> +        (BlackBerry::WebKit::BackingStore::isDirectRenderingToWindow):
> +        (BlackBerry::WebKit::BackingStore::defersBlit):
> +        (BlackBerry::WebKit::BackingStore::setDefersBlit):
> +        (BlackBerry::WebKit::BackingStore::hasBlitJobs):
> +        (BlackBerry::WebKit::BackingStore::blitOnIdle):
> +        * blackberry/Api/BackingStore.h: Added.
> +        * blackberry/Api/BackingStore_p.h: Added.
> +        (BlackBerry::WebKit::BackingStoreGeometry::BackingStoreGeometry):
> +        (BlackBerry::WebKit::BackingStoreGeometry::numberOfTilesWide):
> +        (BlackBerry::WebKit::BackingStoreGeometry::setNumberOfTilesWide):
> +        (BlackBerry::WebKit::BackingStoreGeometry::numberOfTilesHigh):
> +        (BlackBerry::WebKit::BackingStoreGeometry::setNumberOfTilesHigh):
> +        (BlackBerry::WebKit::BackingStoreGeometry::backingStoreOffset):
> +        (BlackBerry::WebKit::BackingStoreGeometry::setBackingStoreOffset):
> +        (BlackBerry::WebKit::BackingStoreGeometry::tileAt):
> +        (BlackBerry::WebKit::BackingStoreGeometry::tileMap):
> +        (BlackBerry::WebKit::BackingStoreGeometry::setTileMap):
> +        (BlackBerry::WebKit::BackingStoreWindowBufferState::blittedRegion):
> +        (BlackBerry::WebKit::BackingStoreWindowBufferState::addBlittedRegion):
> +        (BlackBerry::WebKit::BackingStoreWindowBufferState::clearBlittedRegion):
> +        (BlackBerry::WebKit::BackingStoreWindowBufferState::isRendered):
> +        (BlackBerry::WebKit::BackingStorePrivate::setCurrentBackingStoreOwner):
> +        (BlackBerry::WebKit::BackingStorePrivate::currentBackingStoreOwner):

This whole block can be omitted since it is a new file.

> Source/WebKit/blackberry/Api/BackingStore.cpp:302
> +    /*
> +     * If immediate is true, then we're being asked to perform synchronously
> +     *
> +     * NOTE: WebCore::ScrollView will call this method with immediate:true and contentChanged:false.
> +     * This is a special case introduced specifically for the Apple's windows port and can be safely ignored I believe.
> +     *
> +     * Now this method will be called from WebPagePrivate::repaint()
> +     */

Lets use "//" comments.

> Source/WebKit/blackberry/Api/BackingStore.cpp:308
> +        rect.inflate(1 /*dx*/, 1 /*dy*/); // account for anti-aliasing of previous rendering runs

comments start with capital letter, and dot at the end.

> Source/WebKit/blackberry/Api/BackingStore.cpp:483
> +    // so it can determine if it is under pressure

missing "." at the end.

> Source/WebKit/blackberry/Api/BackingStore.cpp:660
> +    // Record the current backingstore rect

ditto

> Source/WebKit/blackberry/Api/BackingStore.cpp:686
> +    TileMap tiles; // our new tile map

instead of a comment like this, we should have a better variable name

> Source/WebKit/blackberry/Api/BackingStore.cpp:687
> +    TileMap leftOverTiles; // tiles that are left over

unneeded comment, given the variable name

> Source/WebKit/blackberry/Api/BackingStore.cpp:690
> +    // Iterate through our current tile map and add tiles that are rendered with
> +    // our new backingstore rect

missing "."  in the end.

> Source/WebKit/blackberry/Api/BackingStore.cpp:696
> +        // Reset the old index

ditto

> Source/WebKit/blackberry/Api/BackingStore.cpp:699
> +        // Origin of last committed render for tile in transformed content coordinates

ditto

> Source/WebKit/blackberry/Api/BackingStore.cpp:703
> +        // If the new backing store rect contains this origin, then insert the tile there
> +        // and mark it as no longer shifted. Note: Platform::IntRect::contains checks for a 1x1 rect

ditto

> Source/WebKit/blackberry/Api/BackingStore.cpp:717
> +                // Intersect the tile with the not rendered region to get the areas
> +                // of the tile that we need to clear

ditto

> Source/WebKit/blackberry/Api/BackingStore.cpp:746
> +    ASSERT((int)indexesToFill.size() == leftOverTiles.size());

c-cast is to be avoided.

> Source/WebKit/blackberry/Api/BackingStore.cpp:759
> +        // Origin of last committed render for tile in transformed content coordinates

period

> Source/WebKit/blackberry/Api/BackingStore.cpp:778
> +    // Checks to make sure we haven't lost any tiles

ditto

> Source/WebKit/blackberry/Api/BackingStore.cpp:781
> +    // Actually set state

ditto

> Source/WebKit/blackberry/Api/BackingStore.cpp:787
> +    // Swap the front/back state

ditto

> Source/WebKit/blackberry/Api/BackingStore.cpp:835
> +    // Intersect the tile with the not rendered region to get the areas
> +    // of the tile that we need to clear

ditto

> Source/WebKit/blackberry/Api/BackingStore.cpp:847
> +        // Find the origin of this tile

ditto

> Source/WebKit/blackberry/Api/BackingStore.cpp:850
> +        // Map to tile coordinates

ditto

> Source/WebKit/blackberry/Api/BackingStore.cpp:1073
> +        // FIXME: modify render to take a Vector<IntRect> parameter so we're not recreating
> +        // GraphicsContext on the stack each time

ditto

> Source/WebKit/blackberry/Api/BackingStore.cpp:1076
> +        // Add the newly rendered region to the tile so it can keep track for blits

ditto

> Source/WebKit/blackberry/Api/BackingStore.cpp:1088
> +        // We will need a swap here because of the shared back buffer

ditto.

> Source/WebKit/blackberry/Api/BackingStore.cpp:1098
> +    // Clip to the visible content including overscroll

ditto

> Source/WebKit/blackberry/Api/BackingStore.cpp:2431
> +#if ENABLE_SCROLLBARS

Is ENABLE_SCROLLBARS really needed?

> Source/WebKit/blackberry/Api/BackingStore.cpp:2626
> +    d->blitVisibleContents(true /* force */);

/*force*/

[Jacky Jiang]

(In reply to comment #2)
> (From update of attachment 117648 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=117648&action=review
> 
> Patch looks good, as I know the code, but we have too many minor issues, that another round is needed.
> 
> Jacky, please fix all comment styles (including the ones I did not highlight).
> 
Will do, thanks for your review Antonio!

[Jacky Jiang]

Created attachment 117904 [details]
Updated patch

Updated the patch according to Antonio's comments.

[Jacky Jiang]

Created attachment 117912 [details]
Updated patch

Updated more style fixes.

[Daniel Bates]

Comment on attachment 117912 [details]
Updated patch

View in context: https://bugs.webkit.org/attachment.cgi?id=117912&action=review

> Source/WebKit/blackberry/Api/BackingStore.cpp:27
> +#include "FloatRect.h"

This include is unnecessary as GraphicsContext.h (below) includes this file.

> Source/WebKit/blackberry/Api/BackingStore.cpp:33
> +#include "PlatformString.h"

This file is unnecessary as Page.h (above) includes this file.

> Source/WebKit/blackberry/Api/BackingStore.cpp:77
> +// Compute divisor pairs.

This comment is inane. I suggest we remove it.

> Source/WebKit/blackberry/Api/BackingStore.cpp:80
> +// TODO: cache this and/or use a smarter algorithm.

Nit: TODO => FIXME

cache => Cache

> Source/WebKit/blackberry/Api/BackingStore.cpp:84
> +    for (unsigned int i = 1; i <= n; ++i)

unsigned int => unsigned

> Source/WebKit/blackberry/Api/BackingStore.cpp:153
> +struct BackingStoreMutexLocker {

Nit: I would the keyword "struct" to "class" since we have private instance data here and then explicitly add a "public:" keyword.

Also, this class should be non copyable since it doesn't make sense to copy this RAII object. Using WTF_MAKE_NONCOPYABLE() defined in <http://trac.webkit.org/browser/trunk/Source/JavaScriptCore/wtf/Noncopyable.h?rev=76248#L29>, I would write this class to have the form:

class BackingStoreMutexLocker {
    WTF_MAKE_NONCOPYABLE(BackingStoreMutexLocker);
public:
    ...
private:
    ...
};

> Source/WebKit/blackberry/Api/BackingStore.cpp:218
> +    delete m_renderQueue;
> +    m_renderQueue = 0;
> +    delete m_renderTimer;
> +    m_renderTimer = 0;

We should make both m_renderQueue and m_renderTimer OwnPtrs. Then we don't need to explicitly delete them here.

> Source/WebKit/blackberry/Api/BackingStore.cpp:335
> +        render(rect, false /*renderContentOnly*/);

We should make render() take an enum value for its second argument instead of a boolean so that we can remove the inline comment. We can do this in a separate patch.

> Source/WebKit/blackberry/Api/BackingStore.cpp:656
> +    // Record the current backingstore rect.

This comment is inane. Please remove it.

> Source/WebKit/blackberry/Api/BackingStore.cpp:677
> +    // The list of indexes we need to fill.

Ditto.

> Source/WebKit/blackberry/Api/BackingStore.cpp:686
> +    // our new backingstore rect.

Nit: backingstore => backing store

> Source/WebKit/blackberry/Api/BackingStore.cpp:748
> +        if (!(i < indexesToFill.size())) {

Nit: Negating an expression or sub-expression tends to be more difficult to reason about than reading the same expression with the negation pushed through. I suggest pushing the negation through and hence write the if-statement expression as: "i >= indexesToFill.size()" .

> Source/WebKit/blackberry/Api/BackingStore.cpp:757
> +        // Origin of the new index for the new backingstore rect.

Nit: backingstore => backing store

> Source/WebKit/blackberry/Api/BackingStore.cpp:761
> +        updateTile(originOfNew, false /*immediate*/);

Nit: We should make the second argument of updateTile() be an enum so that we don't need an inline comment. We can do this in a follow-up patch.

> Source/WebKit/blackberry/Api/BackingStore.cpp:763
> +        // Do some bookkeeping with shifting tiles...

This comment is inane. Please remove it.

> Source/WebKit/blackberry/Api/BackingStore.cpp:777
> +    // Actually set state.

This comment is inane. Please remove it.

> Source/WebKit/blackberry/Api/BackingStore.cpp:783
> +    // Swap the front/back state.

Ditto.

> Source/WebKit/blackberry/Api/BackingStore.cpp:868
> +        // Find the origin of this tile.

This comment is inane. Please remove it.

> Source/WebKit/blackberry/Api/BackingStore.cpp:874
> +        // Check if it is current.

Ditto.

> Source/WebKit/blackberry/Api/BackingStore.cpp:915
> +    // Set the state.

Ditto.

> Source/WebKit/blackberry/Api/BackingStore.cpp:940
> +    // Request a layout now which might change the contents rect.

Ditto.

> Source/WebKit/blackberry/Api/BackingStore.cpp:957
> +    renderContents(0 /*not needed since we're direct rendering to window*/,
> +                   origin, dirtyRect);

I would write this on one line. Also, I would move the inline comment so that it's above this line and update it so that it reads:

"We don't need a buffer since we're direct rendering to window."

> Source/WebKit/blackberry/Api/BackingStore.cpp:980
> +    // Request a layout now which might change the contents rect.

This comment is inane. Please remove it.

> Source/WebKit/blackberry/Api/BackingStore.cpp:1206
> +        clearWindow(overScrollRect,
> +                    color.red(), color.green(), color.blue(), color.alpha());

Nit: I would write this on one line.

[Jacky Jiang]

Created attachment 118047 [details]
Updated patch

Updated the patch according to Dan's comments.

[Daniel Bates]

Comment on attachment 118047 [details]
Updated patch

View in context: https://bugs.webkit.org/attachment.cgi?id=118047&action=review

> Source/WebKit/blackberry/Api/BackingStore.cpp:2437
> +#if OS(QNX)

Remove this if-define as it's unnecessary.

> Source/WebKit/blackberry/Api/BackingStore.cpp:2438
> +    // Store temps.

Remove this comment as it's inane.

> Source/WebKit/blackberry/Api/BackingStore.h:23
> +

Remove this empty line as it doesn't improve the readability of this file.

> Source/WebKit/blackberry/Api/BackingStore.h:25
> +#include <BlackBerryPlatformGraphics.h>
> +#include <BlackBerryPlatformPrimitives.h>

As far as I can tell we neither of these includes are necessary. Instead, we should just forward declare BlackBerry::Platform::IntRect.

> Source/WebKit/blackberry/Api/BackingStore_p.h:27
> +

Remove this empty line as it doesn't improve the readability of this file.

> Source/WebKit/blackberry/Api/BackingStore_p.h:354
> +}

Please add an inline comment on this line: // namespace WebKit

> Source/WebKit/blackberry/Api/BackingStore_p.h:355
> +}

Please add an inline comment on this line: // namespace BlackBerry

[Jacky Jiang]

Created attachment 118601 [details]
Updated patch

Updated the patch according to Dan's comments.

[Daniel Bates]

Comment on attachment 118601 [details]
Updated patch

View in context: https://bugs.webkit.org/attachment.cgi?id=118601&action=review

> Source/WebKit/blackberry/Api/BackingStore.cpp:1957
> +void BackingStorePrivate::actualVisibleSizeChanged(const Platform::IntSize& size)

We should ultimately remove this method since it has an empty implementation. We can do this in a follow up bug.

> Source/WebKit/blackberry/Api/BackingStore.cpp:2054
> +    static int tw = 0;

I would write lines 2054-2056 as:

static int tileWidth = BlackBerry::Platform::Graphics::Screen::landscapeWidth();

> Source/WebKit/blackberry/Api/BackingStore.cpp:2062
> +    static int th = 0;

I would use a similar approach here.

[Jacky Jiang]

Created attachment 118667 [details]
Updated patch

Updated the tileWidth() and tileHeight().

[Daniel Bates]

Comment on attachment 118667 [details]
Updated patch

Thanks Jacky for the updated patch. We should look to further clean up this file and break it into smaller chunks in a follow up patch.

[Jacky Jiang]

(In reply to comment #12)
> (From update of attachment 118667 [details])
> Thanks Jacky for the updated patch. We should look to further clean up this file and break it into smaller chunks in a follow up patch.
OK, thanks for your review Dan!

[WebKit Review Bot]

Comment on attachment 118667 [details]
Updated patch

Clearing flags on attachment: 118667

Committed r102514: <http://trac.webkit.org/changeset/102514>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.