Create CoreTabHelper, move remaining core TCW functionality into it.

chrome/browser/ui/tab_contents/core_tab_helper.h

Index: chrome/browser/ui/tab_contents/core_tab_helper.h
diff --git a/chrome/browser/ui/tab_contents/core_tab_helper.h b/chrome/browser/ui/tab_contents/core_tab_helper.h
new file mode 100644
index 0000000000000000000000000000000000000000..fc40239381bea5f08096f04edf04c780a955f234
--- /dev/null
+++ b/chrome/browser/ui/tab_contents/core_tab_helper.h
@@ -0,0 +1,78 @@
+// Copyright (c) 2011 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.
+
+#ifndef CHROME_BROWSER_UI_TAB_CONTENTS_CORE_TAB_HELPER_H_
+#define CHROME_BROWSER_UI_TAB_CONTENTS_CORE_TAB_HELPER_H_
+#pragma once
+
+#include "chrome/browser/prefs/pref_change_registrar.h"
+#include "content/browser/tab_contents/tab_contents_observer.h"
+#include "content/public/browser/notification_registrar.h"
+
+class CoreTabHelperDelegate;
+class SkBitmap;
+class TabContentsWrapper;
+
+// Per-tab class to handle functionality that is either core to the operation of
+// tabs or doesn't have anywhere else to go.
+class CoreTabHelper : public TabContentsObserver,

[Jói, 2011/12/08 11:23:37]

Both the name of this class (...Helper) and its comment bug me a bit; the
responsibility of the class is quite unclear, and the current set of
functionality is quite a mixed bag.  Since we're refactoring existing code I
guess it can't be perfect, but is there any way to better express the nature of
this class?  Should it perhaps be more than one class, each with a clearly
defined responsibility?

[Avi (use Gerrit), 2011/12/08 15:27:38]

I know.

Right now, TCW has on it random bits of functionality that haven't fit anywhere
else, one-off functionality that made it a dumping ground. I can pull things
like error page url, snapshots, etc off into their own classes, but that would
mean a class just to listen to one message. (Although that's certainly cleaner
and would definitely be making a point.)

I think "core" things like titles and status text belong together on a 'core'
TH.

BTW, anything that hangs off the TCW and provides per-tab service is called a
TabHelper. We ran out of terminology a long time ago.

[Jói, 2011/12/08 15:31:52]

I'm fine with doing it this way for now, and maybe splitting into separate
classes down the line as clarity emerges.  How about that?

+                      public content::NotificationObserver {
+ public:
+  explicit CoreTabHelper(TabContentsWrapper* wrapper);
+  virtual ~CoreTabHelper();
+
+  CoreTabHelperDelegate* delegate() const { return delegate_; }
+  void set_delegate(CoreTabHelperDelegate* d) { delegate_ = d; }

[Jói, 2011/12/08 11:23:37]

A comment that caller must ensure lifetime of delegate exceeds lifetime of
CoreTabHelper would be nice.

[Avi (use Gerrit), 2011/12/08 15:27:38]

Is that comment found elsewhere with delegates? (And it's actually not true in
this case...)

[Jói, 2011/12/08 15:31:52]

I've seen (and myself added) similar comments about lifetime responsibilities. 
The fact that I misunderstood the lifetime contract indicates there is probably
a need for a comment :)  I guess a more correct description is that whoever
calls set_delegate must ensure the lifetime of the delegate until the
CoreTabHelper is deleted, or set_delegate is set with a different value.

+
+  static void RegisterUserPrefs(PrefService* prefs);
+
+  // Initial title assigned to NavigationEntries from Navigate.
+  static string16 GetDefaultTitle();
+
+  // Returns a human-readable description the tab's loading state.
+  string16 GetStatusText() const;
+
+  // Captures a snapshot of the page.
+  void CaptureSnapshot();
+
+  // Stop this tab rendering in fullscreen mode.
+  void ExitFullscreenMode();
+
+ private:
+  // TabContentsObserver overrides:
+  virtual void RenderViewCreated(RenderViewHost* render_view_host) OVERRIDE;
+  virtual void DidBecomeSelected() OVERRIDE;
+  virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
+
+  // content::NotificationObserver overrides:
+  virtual void Observe(int type,
+                       const content::NotificationSource& source,
+                       const content::NotificationDetails& details) OVERRIDE;
+
+  // Internal helpers ----------------------------------------------------------
+
+  // Message handlers.
+  void OnSnapshot(const SkBitmap& bitmap);
+  void OnPDFHasUnsupportedFeature();
+
+  // Returns the server that can provide alternate error pages.  If the returned
+  // URL is empty, the default error page built into WebKit will be used.
+  GURL GetAlternateErrorPageURL() const;
+
+  // Send the alternate error page URL to the renderer.
+  void UpdateAlternateErrorPageURL(RenderViewHost* rvh);
+
+  // Delegate for notifying our owner about stuff. Not owned by us.
+  CoreTabHelperDelegate* delegate_;
+
+  // Our owning TabContentsWrapper.
+  TabContentsWrapper* wrapper_;
+
+  content::NotificationRegistrar registrar_;
+  PrefChangeRegistrar pref_change_registrar_;
+
+  DISALLOW_COPY_AND_ASSIGN(CoreTabHelper);
+};
+
+#endif  // CHROME_BROWSER_UI_TAB_CONTENTS_CORE_TAB_HELPER_H_