Bug 124987 - Nix Upstream: Adding missing nix new files to WebCore
Summary: Nix Upstream: Adding missing nix new files to WebCore
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Thiago de Barros Lacerda
URL:
Keywords:
Depends on:
Blocks: 124950
  Show dependency treegraph
 
Reported: 2013-11-28 11:03 PST by Thiago de Barros Lacerda
Modified: 2013-12-04 06:27 PST (History)
4 users (show)

See Also:


Attachments
Patch (30.48 KB, patch)
2013-11-28 11:05 PST, Thiago de Barros Lacerda
no flags Details | Formatted Diff | Diff
Patch (30.54 KB, patch)
2013-12-02 15:27 PST, Thiago de Barros Lacerda
benjamin: review+
benjamin: commit-queue-
Details | Formatted Diff | Diff
Patch for landing (30.56 KB, patch)
2013-12-02 20:30 PST, Thiago de Barros Lacerda
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Thiago de Barros Lacerda 2013-11-28 11:03:13 PST
Nix specific files
Comment 1 Thiago de Barros Lacerda 2013-11-28 11:05:23 PST
Created attachment 218020 [details]
Patch
Comment 2 Benjamin Poulain 2013-12-02 14:50:16 PST
Comment on attachment 218020 [details]
Patch

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

> Source/WebCore/platform/nix/FileSystemNix.cpp:49
> +/* On linux file names are just raw bytes, so also strings that cannot be encoded in any way
> + * are valid file names. This mean that we cannot just store a file name as-is in a String
> + * but we have to escape it.
> + * On Windows the GLib file name encoding is always UTF-8 so we can optimize this case. */

This file is in WebCore/platform. It should use regular comment style.

> Source/WebCore/platform/nix/FileSystemNix.cpp:208
> +    static CString cachedPath;

This needs DEFINE_STATIC_LOCAL.

> Source/WebCore/platform/nix/FileSystemNix.cpp:230
> +    /* No null checking needed */

Comment style.

This comment does not add much. It does not explain the "why" of the code.

> Source/WebCore/platform/nix/MIMETypeRegistryNix.cpp:87
> +    String s = ext.lower();
> +    const ExtensionMap *e = extensionMap;
> +    while (e->extension) {
> +        if (s == e->extension)
> +            return e->mimeType;
> +        ++e;
> +    }

Name the variables, "s" and "e" is no good.

> Source/WebCore/platform/nix/SharedTimerNix.cpp:45
> +void setSharedTimerFiredFunction(void (*f)())
> +{
> +    sharedTimerFiredFunction = f;
> +}

Name "f"?
Comment 3 Thiago de Barros Lacerda 2013-12-02 15:27:59 PST
Created attachment 218226 [details]
Patch
Comment 4 Benjamin Poulain 2013-12-02 15:29:53 PST
Comment on attachment 218226 [details]
Patch

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

LGTM.

> Source/WebCore/platform/nix/FileSystemNix.cpp:137
> +

Extra blank line here.

> Source/WebCore/platform/nix/FileSystemNix.cpp:155
> +

Ditto.
Comment 5 Thiago de Barros Lacerda 2013-12-02 20:30:10 PST
Created attachment 218261 [details]
Patch for landing
Comment 6 WebKit Commit Bot 2013-12-03 06:23:08 PST
Comment on attachment 218261 [details]
Patch for landing

Clearing flags on attachment: 218261

Committed r160001: <http://trac.webkit.org/changeset/160001>