Bug 173003 - [Win] Link errors on tag WebKit-603.3.4
Summary: [Win] Link errors on tag WebKit-603.3.4
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Per Arne Vollan
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-06-06 10:27 PDT by Per Arne Vollan
Modified: 2023-10-27 18:09 PDT (History)
4 users (show)

See Also:


Attachments
Patch (1.93 KB, patch)
2017-06-06 10:32 PDT, Per Arne Vollan
bfulgham: review+
bfulgham: commit-queue-
Details | Formatted Diff | Diff
Patch (3.29 KB, patch)
2017-06-06 12:28 PDT, Per Arne Vollan
bfulgham: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Per Arne Vollan 2017-06-06 10:27:00 PDT
The symbols WebPreferences::allowsPageCacheWithWindowOpener and WebPreferences::setAllowsPageCacheWithWindowOpener are unresolved.
Comment 1 Per Arne Vollan 2017-06-06 10:32:24 PDT
Created attachment 312086 [details]
Patch
Comment 2 Per Arne Vollan 2017-06-06 10:35:20 PDT
<rdar://problem/32585333>
Comment 3 Brent Fulgham 2017-06-06 10:52:04 PDT
Comment on attachment 312086 [details]
Patch

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

r=me, but please figure out why the patch won't apply. :-)

> Source/WebKit/win/ChangeLog:10
> +        on trunk.

Hold on! This will break any existing clients that have linkage that knows about 'allowsPageCacheWithWindowOpener' and 'setAllowsPageCacheWithWindowOpener'. This shouldn't have been removed in Trunk. Instead, these should be No-op stubs.

Can you please make that change in trunk and we can make sure that correct change is merged back to 603 branch?
Comment 4 Brent Fulgham 2017-06-06 10:52:30 PDT
Comment on attachment 312086 [details]
Patch

And by r=me, I obviously meant "r-". Sorry!
Comment 5 Per Arne Vollan 2017-06-06 11:12:42 PDT
(In reply to Brent Fulgham from comment #3)
> Comment on attachment 312086 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=312086&action=review
> 
> r=me, but please figure out why the patch won't apply. :-)
> 
> > Source/WebKit/win/ChangeLog:10
> > +        on trunk.
> 
> Hold on! This will break any existing clients that have linkage that knows
> about 'allowsPageCacheWithWindowOpener' and
> 'setAllowsPageCacheWithWindowOpener'. This shouldn't have been removed in
> Trunk. Instead, these should be No-op stubs.
> 
> Can you please make that change in trunk and we can make sure that correct
> change is merged back to 603 branch?

It seems these methods existed in trunk for a only short period of time, and other methods have been added to the IWebPreferencesPrivate4 later. I don't think any clients are using these methods. Should we still add them?

Thanks for reviewing!
Comment 6 Per Arne Vollan 2017-06-06 12:28:34 PDT
Created attachment 312106 [details]
Patch
Comment 7 Brent Fulgham 2017-06-06 12:45:15 PDT
(In reply to Per Arne Vollan from comment #5)
> > Can you please make that change in trunk and we can make sure that correct
> > change is merged back to 603 branch?
> 
> It seems these methods existed in trunk for a only short period of time, and
> other methods have been added to the IWebPreferencesPrivate4 later. I don't
> think any clients are using these methods. Should we still add them?

Oh! If we never shipped with those methods, then it is safe to remove them.

So let me completely reverse myself: Please DO remove them in 603 branch so that we don't create a stub that we have to maintain forever after!

We only want to add these stubs if the two methods were added as part of something that shipped to users. Based on the history you describe, that is not the case.

So I r+ your OLD patch, and r- THIS patch.
Comment 8 Ahmad Saleem 2023-10-27 18:09:15 PDT
Seems to be AppleWin port related, I think we can mark this as 'RESOLVED CONFIGURATION CHANGED'.