Bug 119869 - [Windows] Consolidate WebKit Bundle handling
Summary: [Windows] Consolidate WebKit Bundle handling
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brent Fulgham
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2013-08-15 15:20 PDT by Brent Fulgham
Modified: 2013-08-15 15:43 PDT (History)
4 users (show)

See Also:


Attachments
Patch (13.39 KB, patch)
2013-08-15 15:28 PDT, Brent Fulgham
thorton: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brent Fulgham 2013-08-15 15:20:28 PDT
The code to find and use the WebKit.resources bundle on Windows was replicated in a couple of places, and incompletely done in another.  This patch provides a single implementation of this logic, and updates the bundle referencing sites to use this one (correct) implementation.

This change also allows the WinLauncher executable to find and use the Inspector.
Comment 1 Radar WebKit Bug Importer 2013-08-15 15:20:58 PDT
<rdar://problem/14751077>
Comment 2 Brent Fulgham 2013-08-15 15:28:41 PDT
Created attachment 208863 [details]
Patch
Comment 3 Tim Horton 2013-08-15 15:34:02 PDT
Comment on attachment 208863 [details]
Patch

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

> Source/WebCore/WebCore.vcxproj/WebCore.vcxproj.filters:7069
> +    <ClCompile Include="..\css\CSSFilterImageValue.cpp" />

I think these are unrelated, but maybe expected automatic VS changes?

> Source/WebCore/platform/win/WebCoreBundleWin.cpp:2
> + * Copyright (C) 2013 Apple Inc. All rights reserved.

The license from the old file should carry over (so this should be '2011, 2013').

> Source/WebKit/win/WebCoreSupport/WebInspectorClient.cpp:161
> +    RetainPtr<CFURLRef> htmlURLRef = adoptCF(CFBundleCopyResourceURL(WebCore::webKitBundle(), CFSTR("inspector"), CFSTR("html"), CFSTR("inspector")));

why do you need the explicit namespace? This file 'using namespace WebCore's.
Comment 4 Brent Fulgham 2013-08-15 15:40:56 PDT
Comment on attachment 208863 [details]
Patch

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

>> Source/WebCore/WebCore.vcxproj/WebCore.vcxproj.filters:7069
>> +    <ClCompile Include="..\css\CSSFilterImageValue.cpp" />
> 
> I think these are unrelated, but maybe expected automatic VS changes?

VS does annoying things when you add/remove files.  These may have been added to the vcxproj in an earlier change, but not added to the *filters file (*cough* krit *cough*) and are getting picked up with this commit.

>> Source/WebCore/platform/win/WebCoreBundleWin.cpp:2
>> + * Copyright (C) 2013 Apple Inc. All rights reserved.
> 
> The license from the old file should carry over (so this should be '2011, 2013').

Will do.

>> Source/WebKit/win/WebCoreSupport/WebInspectorClient.cpp:161
>> +    RetainPtr<CFURLRef> htmlURLRef = adoptCF(CFBundleCopyResourceURL(WebCore::webKitBundle(), CFSTR("inspector"), CFSTR("html"), CFSTR("inspector")));
> 
> why do you need the explicit namespace? This file 'using namespace WebCore's.

Huh.  I'll correct.
Comment 5 Brent Fulgham 2013-08-15 15:43:57 PDT
Committed r154146: <http://trac.webkit.org/changeset/154146>