Moves RenderProcessHostImpl::init_time() to RenderProcessHost to fix broken test.

Moves RenderProcessHostImpl::init_time() to RenderProcessHost to fix broken test.

BUG=424575
Committed: https://crrev.com/b62f729b1611b64d2cb1c99892e669c1d4e59668
Cr-Commit-Position: refs/heads/master@{#300514}

[carlosk, 2014-10-20 13:01:01]

carlosk@chromium.org changed reviewers:
+ nasko@chromium.org

[carlosk, 2014-10-20 13:01:01]

nasko: PTAL.

[nasko, 2014-10-20 18:03:39]

https://codereview.chromium.org/665923002/diff/1/content/public/browser/rende...
File content/public/browser/render_process_host.h (right):

https://codereview.chromium.org/665923002/diff/1/content/public/browser/rende...
content/public/browser/render_process_host.h:236: // Note: Will disappear after
PlzNavitate is completed.
nit: I'd say let's add "Do not use." to the comment to make it even a bit
stronger.

https://codereview.chromium.org/665923002/diff/1/content/public/browser/rende...
content/public/browser/render_process_host.h:237: virtual const base::TimeTicks&
init_time() const = 0;
Public API methods should be CamelCased, not hacker_cased.

[carlosk, 2014-10-20 19:42:55]

All done. Let me know if anything else should be changed. Thanks.

https://codereview.chromium.org/665923002/diff/1/content/public/browser/rende...
File content/public/browser/render_process_host.h (right):

https://codereview.chromium.org/665923002/diff/1/content/public/browser/rende...
content/public/browser/render_process_host.h:236: // Note: Will disappear after
PlzNavitate is completed.
On 2014/10/20 18:03:39, nasko wrote:
> nit: I'd say let's add "Do not use." to the comment to make it even a bit
> stronger.

Done.

https://codereview.chromium.org/665923002/diff/1/content/public/browser/rende...
content/public/browser/render_process_host.h:237: virtual const base::TimeTicks&
init_time() const = 0;
On 2014/10/20 18:03:39, nasko wrote:
> Public API methods should be CamelCased, not hacker_cased.

Done.

[carlosk, 2014-10-20 19:42:56]

All done. Let me know if anything else should be changed. Thanks.

[nasko, 2014-10-20 19:53:57]

LGTM with a couple of nits.

https://codereview.chromium.org/665923002/diff/20001/content/browser/renderer...
File content/browser/renderer_host/render_process_host_impl.h (right):

https://codereview.chromium.org/665923002/diff/20001/content/browser/renderer...
content/browser/renderer_host/render_process_host_impl.h:143: 
nit: No need for extra empty line here.

https://codereview.chromium.org/665923002/diff/20001/content/public/browser/r...
File content/public/browser/render_process_host.h (right):

https://codereview.chromium.org/665923002/diff/20001/content/public/browser/r...
content/public/browser/render_process_host.h:237: virtual const base::TimeTicks&
GetInitTime() const = 0;
I like Avi's suggestion. Let's decorate the name with something like
ForNavigationTiming or anything you think is better.

[carlosk, 2014-10-21 12:23:54]

Thanks. Finally the method name is GetInitTimeForPlzNavigate.

https://codereview.chromium.org/665923002/diff/20001/content/browser/renderer...
File content/browser/renderer_host/render_process_host_impl.h (right):

https://codereview.chromium.org/665923002/diff/20001/content/browser/renderer...
content/browser/renderer_host/render_process_host_impl.h:143: 
On 2014/10/20 19:53:57, nasko wrote:
> nit: No need for extra empty line here.

Done.

https://codereview.chromium.org/665923002/diff/20001/content/public/browser/r...
File content/public/browser/render_process_host.h (right):

https://codereview.chromium.org/665923002/diff/20001/content/public/browser/r...
content/public/browser/render_process_host.h:237: virtual const base::TimeTicks&
GetInitTime() const = 0;
On 2014/10/20 19:53:57, nasko wrote:
> I like Avi's suggestion. Let's decorate the name with something like
> ForNavigationTiming or anything you think is better.

I suffixed it with ForPlzNavigate. IMO the odd sounding name should ring a bell
to anyone considering using it.

[carlosk, 2014-10-21 14:12:20]

The CQ bit was checked by carlosk@chromium.org

[commit-bot: I haz the power, 2014-10-21 14:13:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/665923002/80001

[nasko, 2014-10-21 14:13:55]

https://codereview.chromium.org/665923002/diff/20001/content/public/browser/r...
File content/public/browser/render_process_host.h (right):

https://codereview.chromium.org/665923002/diff/20001/content/public/browser/r...
content/public/browser/render_process_host.h:237: virtual const base::TimeTicks&
GetInitTime() const = 0;
On 2014/10/21 12:23:54, carlosk wrote:
> On 2014/10/20 19:53:57, nasko wrote:
> > I like Avi's suggestion. Let's decorate the name with something like
> > ForNavigationTiming or anything you think is better.
> 
> I suffixed it with ForPlzNavigate. IMO the odd sounding name should ring a
bell
> to anyone considering using it.

I agree with jam@ that using the project name is not really useful. Suffixing it
will already show some purpose, but we avoid using project names.

[carlosk, 2014-10-21 14:18:51]

The CQ bit was unchecked by carlosk@chromium.org

[nasko, 2014-10-21 14:50:12]

https://codereview.chromium.org/665923002/diff/100001/content/browser/frame_h...
File content/browser/frame_host/navigator_impl.cc (right):

https://codereview.chromium.org/665923002/diff/100001/content/browser/frame_h...
content/browser/frame_host/navigator_impl.cc:902: navigation_data_->start_time_;
nit: indent by 4 spaces

[carlosk, 2014-10-21 15:28:13]

Done!

https://codereview.chromium.org/665923002/diff/20001/content/public/browser/r...
File content/public/browser/render_process_host.h (right):

https://codereview.chromium.org/665923002/diff/20001/content/public/browser/r...
content/public/browser/render_process_host.h:237: virtual const base::TimeTicks&
GetInitTime() const = 0;
On 2014/10/21 14:13:55, nasko wrote:
> On 2014/10/21 12:23:54, carlosk wrote:
> > On 2014/10/20 19:53:57, nasko wrote:
> > > I like Avi's suggestion. Let's decorate the name with something like
> > > ForNavigationTiming or anything you think is better.
> > 
> > I suffixed it with ForPlzNavigate. IMO the odd sounding name should ring a
> bell
> > to anyone considering using it.
> 
> I agree with jam@ that using the project name is not really useful. Suffixing
it
> will already show some purpose, but we avoid using project names.

OK, replaced it with GetInitTimeForNavigationMetrics (sounded better than
time-for-timing).

https://codereview.chromium.org/665923002/diff/100001/content/browser/frame_h...
File content/browser/frame_host/navigator_impl.cc (right):

https://codereview.chromium.org/665923002/diff/100001/content/browser/frame_h...
content/browser/frame_host/navigator_impl.cc:902: navigation_data_->start_time_;
On 2014/10/21 14:50:12, nasko wrote:
> nit: indent by 4 spaces

But this is idented by 4 spaces... I even ran git cl format in this case to
check this was correct! :)

[nasko, 2014-10-21 15:56:14]

Still LGTM

https://codereview.chromium.org/665923002/diff/20001/content/public/browser/r...
File content/public/browser/render_process_host.h (right):

https://codereview.chromium.org/665923002/diff/20001/content/public/browser/r...
content/public/browser/render_process_host.h:237: virtual const base::TimeTicks&
GetInitTime() const = 0;
On 2014/10/21 15:28:13, carlosk wrote:
> On 2014/10/21 14:13:55, nasko wrote:
> > On 2014/10/21 12:23:54, carlosk wrote:
> > > On 2014/10/20 19:53:57, nasko wrote:
> > > > I like Avi's suggestion. Let's decorate the name with something like
> > > > ForNavigationTiming or anything you think is better.
> > > 
> > > I suffixed it with ForPlzNavigate. IMO the odd sounding name should ring a
> > bell
> > > to anyone considering using it.
> > 
> > I agree with jam@ that using the project name is not really useful.
Suffixing
> it
> > will already show some purpose, but we avoid using project names.
> 
> OK, replaced it with GetInitTimeForNavigationMetrics (sounded better than
> time-for-timing).

Ah, I like that better too!

https://codereview.chromium.org/665923002/diff/100001/content/browser/frame_h...
File content/browser/frame_host/navigator_impl.cc (right):

https://codereview.chromium.org/665923002/diff/100001/content/browser/frame_h...
content/browser/frame_host/navigator_impl.cc:902: navigation_data_->start_time_;
On 2014/10/21 15:28:13, carlosk wrote:
> On 2014/10/21 14:50:12, nasko wrote:
> > nit: indent by 4 spaces
> 
> But this is idented by 4 spaces... I even ran git cl format in this case to
> check this was correct! :)

Hmm, I was under the impression that each breakup of a logical block (imho the
stuff after equals) needs to be indented by 4 more spaces. clang-format is the
king though, so I learned something new : ).

[carlosk, 2014-10-21 16:26:36]

On 2014/10/21 15:56:14, nasko wrote:
> Still LGTM
> 
>
https://codereview.chromium.org/665923002/diff/20001/content/public/browser/r...
> File content/public/browser/render_process_host.h (right):
> 
>
https://codereview.chromium.org/665923002/diff/20001/content/public/browser/r...
> content/public/browser/render_process_host.h:237: virtual const
base::TimeTicks&
> GetInitTime() const = 0;
> On 2014/10/21 15:28:13, carlosk wrote:
> > On 2014/10/21 14:13:55, nasko wrote:
> > > On 2014/10/21 12:23:54, carlosk wrote:
> > > > On 2014/10/20 19:53:57, nasko wrote:
> > > > > I like Avi's suggestion. Let's decorate the name with something like
> > > > > ForNavigationTiming or anything you think is better.
> > > > 
> > > > I suffixed it with ForPlzNavigate. IMO the odd sounding name should ring
a
> > > bell
> > > > to anyone considering using it.
> > > 
> > > I agree with jam@ that using the project name is not really useful.
> Suffixing
> > it
> > > will already show some purpose, but we avoid using project names.
> > 
> > OK, replaced it with GetInitTimeForNavigationMetrics (sounded better than
> > time-for-timing).
> 
> Ah, I like that better too!
> 
>
https://codereview.chromium.org/665923002/diff/100001/content/browser/frame_h...
> File content/browser/frame_host/navigator_impl.cc (right):
> 
>
https://codereview.chromium.org/665923002/diff/100001/content/browser/frame_h...
> content/browser/frame_host/navigator_impl.cc:902:
navigation_data_->start_time_;
> On 2014/10/21 15:28:13, carlosk wrote:
> > On 2014/10/21 14:50:12, nasko wrote:
> > > nit: indent by 4 spaces
> > 
> > But this is idented by 4 spaces... I even ran git cl format in this case to
> > check this was correct! :)
> 
> Hmm, I was under the impression that each breakup of a logical block (imho the
> stuff after equals) needs to be indented by 4 more spaces. clang-format is the
> king though, so I learned something new : ).

Just as a verification experiment I tried adding 4 extra spaces and running git
cl format and it reverted the change(!):

$ git diff
diff --git a/content/browser/frame_host/navigator_impl.cc
b/content/browser/frame_host/navigator_impl.cc
index 20eb28a..c109c47 100644
--- a/content/browser/frame_host/navigator_impl.cc
+++ b/content/browser/frame_host/navigator_impl.cc
@@ -899,7 +899,7 @@ void NavigatorImpl::RecordNavigationMetrics(
   }
   bool navigation_created_new_renderer_process =
       site_instance->GetProcess()->GetInitTimeForNavigationMetrics() >
-      navigation_data_->start_time_;
+          navigation_data_->start_time_;
   if (navigation_created_new_renderer_process) {
     UMA_HISTOGRAM_TIMES(
         "Navigation.TimeToCommit_NewRenderer_BeforeUnloadDiscounted",
$ git cl format
$ git diff
$

[carlosk, 2014-10-21 16:27:10]

The CQ bit was checked by carlosk@chromium.org

[commit-bot: I haz the power, 2014-10-21 16:28:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/665923002/120001

[commit-bot: I haz the power, 2014-10-21 17:35:48]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2014-10-21 17:36:36]

Patchset 7 (id:??) landed as
https://crrev.com/b62f729b1611b64d2cb1c99892e669c1d4e59668
Cr-Commit-Position: refs/heads/master@{#300514}