Bug 196286 - [ContentChangeObserver] Do not use the global _WKContentChange in WebKitLegacy
Summary: [ContentChangeObserver] Do not use the global _WKContentChange in WebKitLegacy
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: zalan
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-03-26 21:07 PDT by zalan
Modified: 2019-04-24 09:09 PDT (History)
6 users (show)

See Also:


Attachments
Patch (10.55 KB, patch)
2019-03-27 21:22 PDT, zalan
no flags Details | Formatted Diff | Diff
Patch (10.71 KB, patch)
2019-03-27 21:49 PDT, zalan
no flags Details | Formatted Diff | Diff
Patch (12.05 KB, patch)
2019-03-28 11:31 PDT, zalan
no flags Details | Formatted Diff | Diff
Patch (11.26 KB, patch)
2019-04-23 21:50 PDT, zalan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description zalan 2019-03-26 21:07:17 PDT
ssia
Comment 1 Radar WebKit Bug Importer 2019-03-27 19:11:16 PDT
<rdar://problem/49364417>
Comment 2 zalan 2019-03-27 21:22:41 PDT
Created attachment 366150 [details]
Patch
Comment 3 Simon Fraser (smfr) 2019-03-27 21:25:20 PDT
Comment on attachment 366150 [details]
Patch

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

> Source/WebCore/ChangeLog:3
> +        [ContentChangeObserver] Do not use the global _WKContentChange in WebKitLegacy

This seems like the wrong title.

> Source/WebCore/ChangeLog:9
> +        With this patch all the content observation decision making paths are asynchronous both in WebKit and WebkitLegacy.

Or is this wrong?
Or are you doing two things at once and not clearly saying so?
Comment 4 zalan 2019-03-27 21:49:18 PDT
Created attachment 366151 [details]
Patch
Comment 5 zalan 2019-03-28 11:31:26 PDT
Created attachment 366187 [details]
Patch
Comment 6 zalan 2019-03-31 10:17:40 PDT
iOS builds require WebKitBuild/Release-iphoneos//DerivedSources/WebKitLegacy/WebKitLegacy.generated.exp to be removed first (or nuke WebKitBuild alternatively)
Comment 7 Ryosuke Niwa 2019-04-22 17:05:48 PDT
Can we land this change?
Comment 8 zalan 2019-04-22 18:08:19 PDT
(In reply to Ryosuke Niwa from comment #7)
> Can we land this change?
Absolutely!
Comment 9 zalan 2019-04-23 21:50:24 PDT
Created attachment 368112 [details]
Patch
Comment 10 zalan 2019-04-24 08:38:27 PDT
Committed r244588: <https://trac.webkit.org/changeset/244588>
Comment 11 Shawn Roberts 2019-04-24 08:58:28 PDT
After changes in https://trac.webkit.org/changeset/244588

We are seeing build failures on iOS Simulator Release. Other queues have not caught up so it may be affecting other OpenSource builders. Is also causing internal build failures.

https://build.webkit.org/builders/Apple%20iOS%2012%20Release%20%28Build%29/builds/5629

   
https://build.webkit.org/builders/Apple%20iOS%2012%20Release%20%28Build%29/builds/5629/steps/compile-webkit/logs/stdio

ld: warning: directory not found for option '-F/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS12.0.sdk/System/Library/PrivateFrameworks'
Undefined symbols for architecture arm64:
  "_WKObservedContentChange", referenced from:
     -exported_symbol[s_list] command line option
  "_WKSetObservedContentChange", referenced from:
     -exported_symbol[s_list] command line option
ld: symbol(s) not found for architecture arm64
Comment 12 Brent Fulgham 2019-04-24 09:08:23 PDT
If you happen to run into a linker error like this (x86 or arm64):

Undefined symbols for architecture x86_64:
 "_WKObservedContentChange", referenced from:
    -exported_symbol[s_list] command line option
 "_WKSetObservedContentChange", referenced from:
    -exported_symbol[s_list] command line option
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)

please delete [...]/DerivedSources/WebKitLegacy/WebKitLegacy.generated.exp (or alternatively trigger a clean build).
Comment 13 Shawn Roberts 2019-04-24 09:09:42 PDT
(In reply to Brent Fulgham from comment #12)
> If you happen to run into a linker error like this (x86 or arm64):
> 
> Undefined symbols for architecture x86_64:
>  "_WKObservedContentChange", referenced from:
>     -exported_symbol[s_list] command line option
>  "_WKSetObservedContentChange", referenced from:
>     -exported_symbol[s_list] command line option
> ld: symbol(s) not found for architecture x86_64
> clang: error: linker command failed with exit code 1 (use -v to see
> invocation)
> 
> please delete [...]/DerivedSources/WebKitLegacy/WebKitLegacy.generated.exp
> (or alternatively trigger a clean build).

Just started clean builds on the affected queue's so we should be good soon!