[webkit-reviews] review granted: [Bug 87067] [EFL] Implement Network Information API : [Attachment 146767] Patch

[bugzilla-daemon at webkit.org]

Kenneth Rohde Christiansen <kenneth at webkit.org> has granted Gyuyoung Kim
<gyuyoung.kim at samsung.com>'s request for review:
Bug 87067: [EFL] Implement Network Information API
https://bugs.webkit.org/show_bug.cgi?id=87067

Attachment 146767: Patch
https://bugs.webkit.org/attachment.cgi?id=146767&action=review

------- Additional Comments from Kenneth Rohde Christiansen
<kenneth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=146767&action=review


> Source/WebKit/efl/WebCoreSupport/NetworkInfoClientEfl.cpp:57
> +    if (eeze_init())
> +	   return;

Maybe it is worth printing an error to the console?

> Source/WebKit/efl/WebCoreSupport/NetworkInfoClientEfl.cpp:67
> +    // FIXME : This function should consider cellular network as well. For
example, 2G, 3G and 4G.

Add bugzilla link?

> Source/WebKit/efl/WebCoreSupport/NetworkInfoClientEfl.cpp:75
> +    // FIXME : The eeze library doesn't support EEZE_NET_ADDR_TYPE_IP type
yet. So, EEZE_NET_ADDR_TYPE_BROADCAST
> +    // is used for now.

bug link pls

> Source/WebKit/efl/WebCoreSupport/NetworkInfoClientEfl.cpp:84
> +	   bool ok = false;
> +	   bandwidth = String::fromUTF8(attribute).toUIntStrict(&ok);

Seem to be no reason to initialize that? why not just call it bool ignoredOK;
But are you sure it is needed?