Refactor SyncedWindowDelegateAndroid into TabModel

chrome/browser/ui/android/tab_model/tab_model_android.h

+// 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.
+
+#ifndef CHROME_BROWSER_UI_ANDROID_TAB_MODEL_TAB_MODEL_ANDROID_H_
+#define CHROME_BROWSER_UI_ANDROID_TAB_MODEL_TAB_MODEL_ANDROID_H_
+#pragma once

[sky, 2012/07/17 00:32:12]

no more #pragma once (see style guide).

[David Trainor- moved to gerrit, 2012/07/18 23:59:04]

Done.

+
+#include "base/memory/scoped_ptr.h"
+#include "chrome/browser/sessions/session_id.h"
+
+namespace browser_sync {
+class SyncedWindowDelegateAndroid;
+class SyncedWindowDelegate;
+}
+
+namespace content {
+class WebContents;
+}
+
+class Profile;
+class TabContents;
+
+class TabModelAndroid {

[sky, 2012/07/17 00:32:12]

Add a description.

[David Trainor- moved to gerrit, 2012/07/18 23:59:04]

Done.

+ public:
+  TabModelAndroid();
+  virtual SessionID::id_type GetSessionId() const = 0;

[sky, 2012/07/17 00:32:12]

nit: newline between 26/27.

[David Trainor- moved to gerrit, 2012/07/18 23:59:04]

Done.

+  virtual int GetTabCount() const = 0;
+  virtual int GetActiveIndex() const = 0;
+  virtual TabContents* GetTabContentsAt(int index) const = 0;
+  virtual SessionID::id_type GetTabIdAt(int index) const = 0;
+
+  // Used for restoring tabs from synced foreign sessions.
+  virtual void CreateTab(content::WebContents* web_contents) = 0;
+
+  // Returns true when all tabs have been restored from disk.
+  virtual bool AreAllTabsLoaded() const = 0;
+
+  virtual void OpenClearBrowsingData() const = 0;
+  virtual Profile* profile() const = 0;

[sky, 2012/07/17 00:32:12]

virtual methods should not be unix_hacker_style.

[David Trainor- moved to gerrit, 2012/07/18 23:59:04]

Done.

+
+  browser_sync::SyncedWindowDelegate* synced_window_delegate() const;

[sky, 2012/07/17 00:32:12]

Its generally dangerous to have a const method return a non-const object.

[David Trainor- moved to gerrit, 2012/07/18 23:59:04]

Done.

+
+ protected:
+  void BroadcastAllTabsLoaded();
+
+  virtual ~TabModelAndroid();

[sky, 2012/07/17 00:32:12]

destructor before other methods.

[David Trainor- moved to gerrit, 2012/07/18 23:59:04]

Done.

+
+ private:
+  scoped_ptr<browser_sync::SyncedWindowDelegateAndroid> synced_window_delegate_;

[sky, 2012/07/17 00:32:12]

Try to keep your interface pure virtual. Can this be pulled else where?

[David Trainor- moved to gerrit, 2012/07/18 23:59:04]

I can pull out synced_window_delegate_ and BroadcastAllTabsLoaded.  I'll have to
duplicate the logic if we need two implementations of AndroidTabModel, but if
that's preferable to having the interface not be pure virtual that's fine with
me :).

+};
+#endif  // CHROME_BROWSER_UI_ANDROID_TAB_MODEL_TAB_MODEL_ANDROID_H_

[sky, 2012/07/17 00:32:12]

newline between 52/52.

[David Trainor- moved to gerrit, 2012/07/18 23:59:04]

Done.