Bug 49296 – FileSystem: Fix a link error for non-chromium WebKit build

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Kavita Kanetkar]

WebKit build with --file-system gets link error.

[Kavita Kanetkar]

Created attachment 73452 [details]
patch

[Kinuko Yasuda]

Comment on attachment 73452 [details]
patch

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

> WebCore/inspector/InspectorFileSystemAgent.cpp:123
> +    // FIXME: Remove guard when file system is supported for non-chromium platforms.

This comment does not look really accurate; must be like 'when revealFolderInOS is implemented on non-chromium platforms'.

> WebCore/inspector/InspectorFileSystemAgent.cpp:124
>      WebCore::revealFolderInOS(path);

I don't think you need WebCore:: here.  (Aren't we in WebCore namespace?)

[Kavita Kanetkar]

Created attachment 73506 [details]
patch

The WebCore:: is for resolving scope, since InspectorFileSystemAgent's method name is the same.

[Kinuko Yasuda]

Comment on attachment 73506 [details]
patch

lgtm!

[Jian Li]

Comment on attachment 73506 [details]
patch

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

Looks good except the minor issue. Please fix it before you land the patch.

> WebCore/inspector/InspectorFileSystemAgent.cpp:127
> +void InspectorFileSystemAgent::revealFolderInOS(const String&) { }

Please add "notImplemented();".

[Kavita Kanetkar]

Created attachment 73668 [details]
patch

I am not a committer so it needs to go through cq. Please take a look.

Thanks.

[Dumitru Daniliuc]

Comment on attachment 73668 [details]
patch

rs=me, based on jian's and kinuko's reviews.

[Eric Seidel (no email)]

Comment on attachment 73506 [details]
patch

Cleared Jian Li's review+ from obsolete attachment 73506 [details] so that this bug does not appear in http://webkit.org/pending-commit.

[WebKit Commit Bot]

Comment on attachment 73668 [details]
patch

Clearing flags on attachment: 73668

Committed r71885: <http://trac.webkit.org/changeset/71885>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.