Bug 152462 – [Mac] WebKit contains dead source code for OS X Mavericks and earlier

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

Remove code used only when targeting OS X 10.9 and earlier.

Created attachment 267706 [details]
Remove and simplify

[Alexey Proskuryakov]

Comment on attachment 267706 [details]
Remove and simplify

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

Very nice.

> Source/WebKit2/Configurations/BaseXPCService.xcconfig:59
>  WK_XPC_SERVICE_INFOPLIST_SUFFIX_101000 = -10.9-10.10;

Should the plist be renamed in a follow-up?

> Source/WebKit2/WebProcess/WebPage/WebPage.cpp:5013
>  bool WebPage::synchronousMessagesShouldSpinRunLoop()
>  {
> -#if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED < 101000
> -    return WebCore::AXObjectCache::accessibilityEnabled();
> -#endif
>      return false;
>  }

This function can (and should) be deleted now, there is quite a bit of confusing code guarded by it.

> Source/WebKit/mac/WebCoreSupport/WebInspectorClient.mm:277
>  void WebInspectorFrontendClient::setToolbarHeight(unsigned height)
>  {
> -#if __MAC_OS_X_VERSION_MIN_REQUIRED <= 1090
> -    [[m_frontendWindowController window] setContentBorderThickness:height forEdge:NSMaxYEdge];
> -#endif
>  }

Can setToolbarHeight be deleted now?

> WebKitLibraries/ChangeLog:8
> +        * libWebKitSystemInterfaceMavericks.a: Removed.

This should be (roughly) synchronized with removing script code that touches this file.

(In reply to comment #2)
> Comment on attachment 267706 [details]
> Remove and simplify
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=267706&action=review
> 
> Very nice.

Thank you, and thanks for the review!

> 
> > Source/WebKit2/Configurations/BaseXPCService.xcconfig:59
> >  WK_XPC_SERVICE_INFOPLIST_SUFFIX_101000 = -10.9-10.10;
> 
> Should the plist be renamed in a follow-up?

Didn’t seem important.

> 
> > Source/WebKit2/WebProcess/WebPage/WebPage.cpp:5013
> >  bool WebPage::synchronousMessagesShouldSpinRunLoop()
> >  {
> > -#if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED < 101000
> > -    return WebCore::AXObjectCache::accessibilityEnabled();
> > -#endif
> >      return false;
> >  }
> 
> This function can (and should) be deleted now, there is quite a bit of
> confusing code guarded by it.

Good to know! I might do that next.

> 
> > Source/WebKit/mac/WebCoreSupport/WebInspectorClient.mm:277
> >  void WebInspectorFrontendClient::setToolbarHeight(unsigned height)
> >  {
> > -#if __MAC_OS_X_VERSION_MIN_REQUIRED <= 1090
> > -    [[m_frontendWindowController window] setContentBorderThickness:height forEdge:NSMaxYEdge];
> > -#endif
> >  }
> 
> Can setToolbarHeight be deleted now?

Will look into that too.

> 
> > WebKitLibraries/ChangeLog:8
> > +        * libWebKitSystemInterfaceMavericks.a: Removed.
> 
> This should be (roughly) synchronized with removing script code that touches
> this file.

I didn’t see anything in Tools/Scripts that referred to this file. Did I miss something, or are you referring to something internal to Apple?

[WebKit Commit Bot]

Comment on attachment 267706 [details]
Remove and simplify

Clearing flags on attachment: 267706

Committed r194318: <http://trac.webkit.org/changeset/194318>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

(In reply to comment #3)
> (In reply to comment #2)
> > Comment on attachment 267706 [details]
> > > Source/WebKit2/WebProcess/WebPage/WebPage.cpp:5013
> > >  bool WebPage::synchronousMessagesShouldSpinRunLoop()
> > >  {
> > > -#if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED < 101000
> > > -    return WebCore::AXObjectCache::accessibilityEnabled();
> > > -#endif
> > >      return false;
> > >  }
> > 
> > This function can (and should) be deleted now, there is quite a bit of
> > confusing code guarded by it.
> 
> Good to know! I might do that next.

Taking over bug 126021.

(In reply to comment #3)
> (In reply to comment #2)
> > > Source/WebKit/mac/WebCoreSupport/WebInspectorClient.mm:277
> > >  void WebInspectorFrontendClient::setToolbarHeight(unsigned height)
> > >  {
> > > -#if __MAC_OS_X_VERSION_MIN_REQUIRED <= 1090
> > > -    [[m_frontendWindowController window] setContentBorderThickness:height forEdge:NSMaxYEdge];
> > > -#endif
> > >  }
> > 
> > Can setToolbarHeight be deleted now?
> 
> Will look into that too.

Filed bug 152466.