Patch will be submitted within 1~2 weeks.
Created attachment 144993 [details] Prototype patch This patch is just prototype. There is still crash issue and needs to implement network info spec. further.
Created attachment 145763 [details] Prototype patch - 2
Created attachment 146175 [details] Patch
I implement bandwidth() function first. And also, bandwidth() supports ethernet network for now. I need to investigate how to support cellular network and meter() function. It seems bandwidth() needs to be landed first.
Created attachment 146176 [details] Patch
Comment on attachment 146176 [details] Patch Attachment 146176 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/12910181
(In reply to comment #6) > (From update of attachment 146176 [details]) > Attachment 146176 [details] did not pass efl-ews (efl): > Output: http://queues.webkit.org/results/12910181 This patch includes eeze dependency related to jhbuild. When this patch is landed, this patch can be passed. I don't wanna install eeze library to eflews unnecessarily.
Created attachment 146457 [details] Patch
Comment on attachment 146457 [details] Patch Attachment 146457 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/12924198
Created attachment 146493 [details] Patch
Comment on attachment 146493 [details] Patch Attachment 146493 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/12924287
Kubo, could you review this patch informally ?
Comment on attachment 146493 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=146493&action=review > Source/WebKit/efl/WebCoreSupport/NetworkInfoClientEfl.cpp:56 > + if (eeze_init()) Are we really updating anything after eeze_init() is called? I would move this to the constructor and leave this function as notImplemented(). > Source/WebKit/efl/WebCoreSupport/NetworkInfoClientEfl.cpp:62 > + eeze_shutdown(); I would move this to the destructor and leave this as notImplemented() for now. > Source/WebKit/efl/WebCoreSupport/NetworkInfoClientEfl.cpp:67 > + unsigned int bandwidth = -1; It is unsigned so initializing it to -1 does not make sense here. > Source/WebKit/efl/WebCoreSupport/NetworkInfoClientEfl.cpp:70 > + Eeze_Net* ethNet = eeze_net_new(ethernetInterface); This will return NULL if eth0 does not exist. Maybe if could add a NULL check right after? > Source/WebKit/efl/WebCoreSupport/NetworkInfoClientEfl.cpp:73 > + // FIXME : The eeze library doesn't support EEZE_NET_ADDR_TYPE_IP type yet. So, EEZE_NET_ADDR_TYPE_BROADCAST Based on the code, it looks as if it is implemented in v1.2.0. Have you tried? If so, please disregard my comment. > Source/WebKit/efl/WebCoreSupport/NetworkInfoClientEfl.cpp:76 > + if (String::fromUTF8(address).isEmpty()) if (!address || !*address) might be lighter here? > Source/WebKit/efl/WebCoreSupport/NetworkInfoClientEfl.cpp:81 > + bandwidth = atoi(attribute); How about using String::fromUTF8(attribute).toUIntStrict(&ok) to check for success and fallback to default value otherwise? > Source/WebKit/efl/WebCoreSupport/NetworkInfoClientEfl.cpp:82 > + else As per WebKit coding style, you should use curly brackets for this else case (because of the comment line). > Source/WebKit/efl/WebCoreSupport/NetworkInfoClientEfl.cpp:84 > + bandwidth = std::numeric_limits<double>::infinity(); bandwidth is an unsigned integer so I'm unsure what the result of this line is. Maybe we could use UINT_MAX from limits.h instead? or maybe a reasonable default value like 100Mb/s. > Source/WebKit/efl/WebCoreSupport/NetworkInfoClientEfl.cpp:88 > + return bandwidth / 10; // MB/s Why are you dividing by 10 here? What's the unit of the value returned by eeze_net_attribute_get(ethNet, "speed")? Maybe you meant to divide by 8 to convert from Mbits to Mbytes? Also, you probably don't want to divide in the case where bandwidth was unknown.
Created attachment 146767 [details] Patch
(In reply to comment #13) > (From update of attachment 146493 [details]) > > Source/WebKit/efl/WebCoreSupport/NetworkInfoClientEfl.cpp:62 > > + eeze_shutdown(); > I would move this to the destructor and leave this as notImplemented() for now. I think stopUpdating() is proper place. Because NetworkInfoController in WebCore manages life cycle of network info via startUpdating() and stopUpdating(). See also, BatteryClientEfl.cpp. This patch is based on latest editor draft. - 0 if the user is currently offline; - Infinity if the bandwidth is unknown; - an estimation of the current bandwidth in MB/s (Megabytes per seconds) available for communication with the browsing context active document's domain. http://dvcs.w3.org/hg/dap/raw-file/tip/network-api/Overview.html
Comment on attachment 146767 [details] Patch Attachment 146767 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/12939258
CC'ing Antonio.
Comment on attachment 146767 [details] Patch 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?
Created attachment 149267 [details] Patch
(In reply to comment #18) > (From update of attachment 146767 [details]) > 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 I fix all you mentioned. > > 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? I think we don't need to initialize ok variable. So, I modify it. Thanks.
Comment on attachment 149267 [details] Patch Attachment 149267 [details] did not pass efl-ews (efl): Output: http://queues.webkit.org/results/13090112
Created attachment 149295 [details] Patch
Comment on attachment 149295 [details] Patch Clearing flags on attachment: 149295 Committed r121170: <http://trac.webkit.org/changeset/121170>
All reviewed patches have been landed. Closing bug.