Fix ResourceLoadTiming resolution lose issue.

Fix ResourceLoadTiming resolution lose issue.

Currently, WebCore::ResourceLoadTiming has a base requestTime with type "double", others are millisecond 
deltas with type "int", it forces following timestamps align with the base time, such as dnsStart. 
So in glue(webkit/glue/weburlloader_impl.cc), we had to calculate delta between the real dnsStart and 
its base time request_start, round to a int value in millisecond, while, lose the resolution. It may cause
timestamps overlap or tricky negative values.
This change removes all "int" deltas and use monotonicallyIncreasing "double" in second instead.

To satisfy test result, we need to add new fields in blink and then switch chrome use the new fields, at last go back blink to remove old fields. This change is the first step and use a macro to guard the new added field.

Contributed by pan.deng@intel.com

BUG=242452
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=150952

[Pan, 2013-05-21 09:36:36]

Please review.

Change in glue should happen at the same time, 
https://codereview.chromium.org/15552003/ I have no idea how to land them
together :(

thanks!
Pan

[eustas, 2013-05-21 09:43:38]

https://codereview.chromium.org/15265004/diff/1/Source/core/page/PerformanceR...
File Source/core/page/PerformanceResourceTiming.cpp (right):

https://codereview.chromium.org/15265004/diff/1/Source/core/page/PerformanceR...
Source/core/page/PerformanceResourceTiming.cpp:123: if (!m_timing ||
m_timing->dnsStart == 0.0)
Doesn't both checks do the same?

[Pan, 2013-05-21 12:28:01]

https://codereview.chromium.org/15265004/diff/1/Source/core/page/PerformanceR...
File Source/core/page/PerformanceResourceTiming.cpp (right):

https://codereview.chromium.org/15265004/diff/1/Source/core/page/PerformanceR...
Source/core/page/PerformanceResourceTiming.cpp:123: if (!m_timing ||
m_timing->dnsStart == 0.0)
On 2013/05/21 09:43:38, eustas.ru wrote:
> Doesn't both checks do the same?

not really, sometimes it's null ptr when glue won't populate m_timing, while
sometimes m_timing is populated but timestamp may be blank as a response is from
network cache.

[James Simonsen, 2013-05-21 17:42:30]

 Tue, May 21, 2013 at 2:36 AM, <pan.deng@intel.com> wrote:

> Change in glue should happen at the same time,
>
https://codereview.chromium.**org/15552003/<https://codereview.chromium.org/1...
have no idea how to land them
> together :(
>

The simple answer is: don't even try. The best thing to do would be to have
3 patches. First add new fields in Chrome. Then switch Blink to use the new
fields. Then remove the old fields.

[Pan, 2013-05-22 08:17:05]

On 2013/05/21 17:42:30, James Simonsen wrote:
>  Tue, May 21, 2013 at 2:36 AM, <mailto:pan.deng@intel.com> wrote:
> 
> > Change in glue should happen at the same time,
> >
>
https://codereview.chromium.**org/15552003/%3Chttps://codereview.chromium.org...
> have no idea how to land them
> > together :(
> >
> 
> The simple answer is: don't even try. The best thing to do would be to have
> 3 patches. First add new fields in Chrome. Then switch Blink to use the new
> fields. Then remove the old fields.

thanks!
If we can't land them at the same time, I'm planning:
step 1) Add double fields in Blink and guard by macros
"ENABLE_DOUBLE_RESOURCE_LOAD_TIMING".
step 2) Change in Chrome glue, pass double fields, at the same time, define and
enable "ENABLE_DOUBLE_RESOURCE_LOAD_TIMING" in gyp file
step 3) Go back Blink remove old fields and macros
step 4) Chrome side remove the "ENABLE_DOUBLE_RESOURCE_LOAD_TIMING" from gyp
file.

Pan

[Pan, 2013-05-22 09:14:00]

please review :)

thanks!
Pan

[abarth-chromium, 2013-05-22 18:19:04]

LGTM with one nit

https://codereview.chromium.org/15265004/diff/7001/Source/core/inspector/Insp...
File Source/core/inspector/InspectorResourceAgent.cpp (right):

https://codereview.chromium.org/15265004/diff/7001/Source/core/inspector/Insp...
Source/core/inspector/InspectorResourceAgent.cpp:125:
.setReceiveHeadersEnd((timing.receiveHeadersEnd - requestTime) * 1000)
Should we create an inline helper function to do this conversion so we don't
need to do it so many times?

[James Simonsen, 2013-05-22 21:36:58]

https://codereview.chromium.org/15265004/diff/7001/Source/core/inspector/Insp...
File Source/core/inspector/InspectorResourceAgent.cpp (right):

https://codereview.chromium.org/15265004/diff/7001/Source/core/inspector/Insp...
Source/core/inspector/InspectorResourceAgent.cpp:125:
.setReceiveHeadersEnd((timing.receiveHeadersEnd - requestTime) * 1000)
Alternatively, why not just store the absolute values everywhere instead of
converting them to deltas (which will then be converted back to absolute values
in window.performance)?

[Pan, 2013-05-23 03:03:39]

On 2013/05/22 21:36:58, James Simonsen wrote:
>
https://codereview.chromium.org/15265004/diff/7001/Source/core/inspector/Insp...
> File Source/core/inspector/InspectorResourceAgent.cpp (right):
> 
>
https://codereview.chromium.org/15265004/diff/7001/Source/core/inspector/Insp...
> Source/core/inspector/InspectorResourceAgent.cpp:125:
> .setReceiveHeadersEnd((timing.receiveHeadersEnd - requestTime) * 1000)
> Alternatively, why not just store the absolute values everywhere instead of
> converting them to deltas (which will then be converted back to absolute
values
> in window.performance)?

thanks!
Several places in DevTools front-end like deltas, so I think it is better to
calculate it in back-end. :)

Pan

[commit-bot: I haz the power, 2013-05-23 03:07:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/pan.deng@intel.com/15265004/6002

[commit-bot: I haz the power, 2013-05-23 03:51:42]

Change committed as 150952