Bug 113754 – Remove code for 10.5 and earlier from Source/WebCore

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Ryosuke Niwa]

Remove code for 10.5 and earlier from Source/WebCore

[Ryosuke Niwa]

Created attachment 196083 [details]
Removed the code

[Early Warning System Bot]

Comment on attachment 196083 [details]
Removed the code

Attachment 196083 [details] did not pass qt-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17314680

[Early Warning System Bot]

Comment on attachment 196083 [details]
Removed the code

Attachment 196083 [details] did not pass qt-wk2-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17392179

[WebKit Review Bot]

Comment on attachment 196083 [details]
Removed the code

Attachment 196083 [details] did not pass cr-linux-debug-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/17392187

[EFL EWS Bot]

Comment on attachment 196083 [details]
Removed the code

Attachment 196083 [details] did not pass efl-ews (efl):
Output: http://webkit-commit-queue.appspot.com/results/17250987

[Ryosuke Niwa]

Created attachment 196095 [details]
Fixed builds

[Darin Adler]

Comment on attachment 196095 [details]
Fixed builds

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

I’m reviewing the mechanics of this patch, not the concept of whether it’s OK to do it. I assume you already know it’s OK.

> Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp:1472
>      UNUSED_PARAM(allowsFontSmoothing);

Please remove this line. It was only there to quiet warnings in older versions of OS X.

> Source/WebCore/platform/graphics/cg/PathCG.cpp:197
>      CGRect bound = CGRectZero;
> -#if !PLATFORM(MAC) || __MAC_OS_X_VERSION_MIN_REQUIRED >= 1060
>      bound = CGPathGetPathBoundingBox(m_path);

Please merge these into a single line. There is no reason to set CGRect to CGRectZero and then overwrite it.

> Source/WebCore/platform/graphics/cocoa/FontPlatformDataCocoa.mm:249
>  static bool canSetCascadeListForCustomFont()
>  {
> -#if PLATFORM(IOS) || __MAC_OS_X_VERSION_MIN_REQUIRED >= 1060
>      return true;
> -#else
> -    return false;
> -#endif
>  }

Please remove this function entirely, in a followup patch if not in this first cut.

> Source/WebCore/platform/graphics/mac/FontCustomPlatformData.cpp:121
>      cgFontRef.adoptCF(CGFontCreateWithDataProvider(dataProvider.get()));

Please move the definition of cgFontRef down here now that there is no #if.

Also, modern style is to use the free function adoptCF, not the member function, which we should deprecate. So this should say:

    RetainPtr<CGFontRef> cgFontRef = adoptCF(CGFontCreateWithDataProvider(dataProvider.get()));

One advantage of using the free function is that you can cut down on local variables. For example, we could write something like this:

    RetainPtr<CGFontRef> cgFontRef = adoptCF(CGFontCreateWithDataProvider(adoptCF(CGDataProviderCreateWithCFData(adoptCF(buffer->createCFData()).get())).get());

That’s probably a little too long to be readable, but I like having that option.

> Source/WebCore/platform/mac/CursorMac.mm:-259
> -        m_platformCursor = createNamedCursor("contextMenuCursor", 3, 2);

We should also remove these cursor image files from the repository and the project file. Either in this patch or in a followup.

> Source/WebCore/platform/mac/CursorMac.mm:-283
> -        m_platformCursor = createNamedCursor("noDropCursor", 3, 1);

We should also remove these cursor image files from the repository and the project file.

> Source/WebCore/platform/mac/CursorMac.mm:-291
> -        m_platformCursor = createNamedCursor("copyCursor", 3, 2);

We should also remove these cursor image files from the repository and the project file.

> Source/WebCore/platform/mac/CursorMac.mm:-303
> -        m_platformCursor = createNamedCursor("notAllowedCursor", 11, 11);

We should also remove these cursor image files from the repository and the project file.

> Source/WebCore/platform/mac/WebVideoFullscreenController.mm:-58
>  @interface WebVideoFullscreenWindow : NSWindow
> -#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 1060
>  <NSAnimationDelegate>
> -#endif

Please merge this into a single line.

> Source/WebCore/platform/mac/WebVideoFullscreenHUDWindowController.mm:49
>  @interface WebVideoFullscreenHUDWindowController (Private)
> -#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 1060
>  <NSWindowDelegate>

Please merge this into a single line.

[Ryosuke Niwa]

I think I messed up this patch for Windows and iOS. Let me fix that and reupload it.

[Ryosuke Niwa]

Created attachment 196275 [details]
Cleanup

[Benjamin Poulain]

Comment on attachment 196275 [details]
Cleanup

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

Looks great!

> Source/WebCore/platform/graphics/mac/FontCustomPlatformData.cpp:119
> -    cgFontRef.adoptCF(CGFontCreateWithDataProvider(dataProvider.get()));
> +    RetainPtr<CGFontRef> cgFontRef = adoptCF(CGFontCreateWithDataProvider(dataProvider.get()));

RetainPtr<CGFontRef> cgFontRef(AdoptCF, CGFontCreateWithDataProvider(dataProvider.get())); ?

> Source/WebCore/platform/graphics/mac/MediaPlayerPrivateQTKit.mm:1009
>          initialSize.width = naturalSize.width;
>          initialSize.height = naturalSize.height;
>      }

Not sure why this is not
    initialSize = naturalSize;

But that's unrelated :)

> Source/WebCore/platform/mac/WebVideoFullscreenHUDWindowController.mm:372
> +

I would remove that blank line. The object is initialized in the following lines.

[Ryosuke Niwa]

Committed r147710: <http://trac.webkit.org/changeset/147710>