Summary: | REGRESSION(r243947) Epson software updater fails to install new version | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Alex Christensen <achristensen> | ||||||||
Component: | New Bugs | Assignee: | Alex Christensen <achristensen> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | beidson, commit-queue, ggaren, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Alex Christensen
2019-11-04 06:37:04 PST
Created attachment 382735 [details]
Patch
Created attachment 382742 [details]
Patch
Comment on attachment 382742 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=382742&action=review > Source/WebKitLegacy/mac/Misc/WebDownload.mm:122 > + if (isDelegateThread()) Is there any reason we even have "isDelegateThread"? Couldn't we just always use "callOnDelegateThread"? Even the forms of this that need a sync return value should be able to work with that, right? Comment on attachment 382742 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=382742&action=review >> Source/WebKitLegacy/mac/Misc/WebDownload.mm:122 >> + if (isDelegateThread()) > > Is there any reason we even have "isDelegateThread"? > > Couldn't we just always use "callOnDelegateThread"? > > Even the forms of this that need a sync return value should be able to work with that, right? We would need callOnDelegateThread and callOnDelegateThreadAndWait. Do you think that would be better? (In reply to Alex Christensen from comment #5) > Comment on attachment 382742 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=382742&action=review > > >> Source/WebKitLegacy/mac/Misc/WebDownload.mm:122 > >> + if (isDelegateThread()) > > > > Is there any reason we even have "isDelegateThread"? > > > > Couldn't we just always use "callOnDelegateThread"? > > > > Even the forms of this that need a sync return value should be able to work with that, right? > > We would need callOnDelegateThread and callOnDelegateThreadAndWait. Do you > think that would be better? Oh, I was (incorrectly) thinking the sync behavior was managed through a local variable of some sort. I think it'd be better. It would clean up every single call site into two methods with the complexity. Created attachment 382746 [details]
Patch
Comment on attachment 382746 [details]
Patch
I think this is much better.
Comment on attachment 382746 [details] Patch Clearing flags on attachment: 382746 Committed r252000: <https://trac.webkit.org/changeset/252000> All reviewed patches have been landed. Closing bug. Comment on attachment 382746 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=382746&action=review > Source/WebKitLegacy/mac/Misc/WebDownload.mm:50 > + static bool isOldEpsonSoftwareUpdater = WebCore::MacApplication::isEpsonSoftwareUpdater() && dyld_get_program_sdk_version() < DYLD_MACOSX_VERSION_10_15; > + return isOldEpsonSoftwareUpdater; Even though this is LegacyWebKit code, which technically works with dyld_get_program_sdk_version(), it's better style to use applicationSDKVersion(), which works for LegacyWebKit and WebKit. |