NoState Prefetch: nostate prefetch browser tests.

chrome/browser/prerender/prerender_contents.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/prerender/prerender_contents.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 #include <functional>
@@ skipping 168 matching lines @@
   gfx::Size GetSizeForNewRenderView(WebContents* web_contents) const override {
     // Have to set the size of the RenderView on initialization to be sure it is
     // set before the RenderView is hidden on all platforms (esp. Android).
     return prerender_contents_->bounds_.size();
   }
 
  private:
   PrerenderContents* prerender_contents_;
 };
 
+void PrerenderContents::Observer::OnPrerenderStart(
+    PrerenderContents* contents) {}

[pasko, 2016/10/11 13:56:23]

nit: empty bodies would be more readable if they stay in the
prerender_contents.h

[mattcary, 2016/10/11 15:08:58]

I thought we avoided inlining? At any rate, I did this to be consistent with the
existing OnPrerenderStopLoading def.

[pasko, 2016/10/11 15:40:19]

We certainly inline trivial getters, so empty bodies seem natural to inline.

We do both, actually, but your pattern is less common (not sure how much):
shell> git grep 'virtual void.*On.*\(.*\) {}' -- '*.h' | wc -l
553
shell> git grep 'virtual void.*On.*\(.*\);' -- '*.h' | grep -v '= 0' | wc -l
364

I agree that it is good to be consistent here, but the other consistent way to
do it looks more readable/maintainable because:
1. the reader knows that all non-overridden methods are no-ops by just looking
at the interface of this base class
2. less boilerplate-placed-in-correct-namespace to scan through in the .cc file

[mattcary, 2016/10/12 09:02:52]

I like the inlining for empty ftns better myself, done.

+
 void PrerenderContents::Observer::OnPrerenderStopLoading(
-    PrerenderContents* contents) {
+    PrerenderContents* contents) {}
+
+void PrerenderContents::Observer::OnPrerenderDomContentLoaded(
+    PrerenderContents* contents) {}
+
+void PrerenderContents::Observer::OnPrerenderStop(PrerenderContents* contents) {
 }
 
-void PrerenderContents::Observer::OnPrerenderDomContentLoaded(
-    PrerenderContents* contents) {
-}
+PrerenderContents::Observer::Observer() {}
 
-PrerenderContents::Observer::Observer() {
-}
-
-PrerenderContents::Observer::~Observer() {
-}
+PrerenderContents::Observer::~Observer() {}
 
 PrerenderContents::PrerenderContents(
     PrerenderManager* prerender_manager,
     Profile* profile,
     const GURL& url,
     const content::Referrer& referrer,
     Origin origin)
     : prerender_mode_(FULL_PRERENDER),
       prerendering_has_started_(false),
       session_storage_namespace_id_(-1),
@@ skipping 545 matching lines @@
 void PrerenderContents::AddResourceThrottle(
     const base::WeakPtr<PrerenderResourceThrottle>& throttle) {
   resource_throttles_.push_back(throttle);
 }
 
 void PrerenderContents::AddNetworkBytes(int64_t bytes) {
   network_bytes_ += bytes;
 }
 
 }  // namespace prerender