Bug 152462 - [Mac] WebKit contains dead source code for OS X Mavericks and earlier
Summary: [Mac] WebKit contains dead source code for OS X Mavericks and earlier
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: mitz
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-12-19 18:16 PST by mitz
Modified: 2015-12-20 09:21 PST (History)
6 users (show)

See Also:


Attachments
Remove and simplify (176.92 KB, patch)
2015-12-19 18:45 PST, mitz
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description mitz 2015-12-19 18:16:34 PST
Remove code used only when targeting OS X 10.9 and earlier.
Comment 1 mitz 2015-12-19 18:45:27 PST
Created attachment 267706 [details]
Remove and simplify
Comment 2 Alexey Proskuryakov 2015-12-19 20:51:32 PST
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.
Comment 3 mitz 2015-12-19 20:56:02 PST
(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?
Comment 4 WebKit Commit Bot 2015-12-19 22:27:33 PST
Comment on attachment 267706 [details]
Remove and simplify

Clearing flags on attachment: 267706

Committed r194318: <http://trac.webkit.org/changeset/194318>
Comment 5 WebKit Commit Bot 2015-12-19 22:27:36 PST
All reviewed patches have been landed.  Closing bug.
Comment 6 mitz 2015-12-20 00:13:45 PST
(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.
Comment 7 mitz 2015-12-20 09:21:27 PST
(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.