Bug 214947

Summary:

REGRESSION (r264925): run-safari --debug no longer works

Product:

WebKit

Reporter:

Simon Fraser (smfr) <simon.fraser>

Component:

WebKit Misc.

Assignee:

Kate Cheney <katherine_cheney>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

darin, simon.fraser, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

Safari Technology Preview

Hardware:

Unspecified

OS:

Unspecified

See Also:

https://bugs.webkit.org/show_bug.cgi?id=214772

Attachments:

Description	Flags
Patch	none
Patch for landing	none

Description Simon Fraser (smfr) 2020-07-29 16:11:30 PDT

$ ./Tools/Scripts/run-safari --debug
Starting SafariForWebKitDevelopment with DYLD_FRAMEWORK_PATH set to point to built WebKit in /Users/kbr/src/WebKit/WebKitBuild/Debug.
dyld: Symbol not found: _WKBundlePageCopyRenderLayerTree
  Referenced from: /System/Library/PrivateFrameworks/Safari.framework/Versions/A/Safari
  Expected in: /Users/kbr/src/WebKit/WebKitBuild/Debug/WebKit.framework/Versions/A/WebKit
 in /System/Library/PrivateFrameworks/Safari.framework/Versions/A/Safari

Comment 1 Radar WebKit Bug Importer 2020-07-29 16:14:04 PDT

<rdar://problem/66297850>

Comment 2 Kate Cheney 2020-07-29 17:53:41 PDT

Created attachment 405535 [details]
Patch

Comment 3 Darin Adler 2020-07-29 18:04:00 PDT

Comment on attachment 405535 [details]
Patch

This is fine:

- If these functions are linked but not called, you might want to put ASSERT_NOT_REACHED in them to help people understand they are intentionally left blank, or a comment, or both. I did that recently with some SPI that I had to leave around for the same reason.

- It’s also nice to leave out the argument names; if we complied with "unused argument" warnings, that would prevent them.

- Also, sometimes we move these things out of the SPI headers into an "old Safari depends on these" block, often not even in a header file, to help make sure no one accidentally adds new calls to the functions.

Not saying you need to do any of these things. Seems like you could land this just like it is.

Comment 4 Kate Cheney 2020-07-30 09:03:17 PDT

Created attachment 405575 [details]
Patch for landing

Comment 5 Kate Cheney 2020-07-30 09:08:04 PDT

(In reply to Darin Adler from comment #3)
> Comment on attachment 405535 [details]
> Patch
> 

Thanks for the review, and the suggestions.

> This is fine:
> 
> - If these functions are linked but not called, you might want to put
> ASSERT_NOT_REACHED in them to help people understand they are intentionally
> left blank, or a comment, or both. I did that recently with some SPI that I
> had to leave around for the same reason.
> 

I added comments to explain why the functions were intentionally left blank. Adding ASSERT_NOT_REACHED() caused the compiler to warn that the function is "noreturn".

Comment 6 EWS 2020-07-30 09:31:45 PDT

Committed r265085: <https://trac.webkit.org/changeset/265085>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 405575 [details].