Removes --enable-stacked-tab-strip flag (Stacked Tabs)

chrome/browser/ui/tabs/tab_strip_layout_type_prefs.cc

-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Copyright 2014 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/ui/tabs/tab_strip_layout_type_prefs.h"
 
 #include "base/prefs/pref_registry_simple.h"
-#include "chrome/browser/ui/tabs/tab_strip_layout_type.h"
+#include "base/prefs/pref_service.h"
 #include "chrome/common/pref_names.h"
 
 namespace chrome {
+namespace tab_strip_layout_type_prefs {
 
-void RegisterTabStripLayoutTypePrefs(PrefRegistrySimple* registry) {
-  // This value is device dependant, so it goes in local state.
-  registry->RegisterIntegerPref(prefs::kTabStripLayoutType,
-                                static_cast<int>(TAB_STRIP_LAYOUT_SHRINK));
+namespace {
+
+// DEPRECATED: this is replaced by kTabStripStackedLayout and exists for
+// backwards compatibility.
+// Old values: 0 = SHRINK (default),  1 = STACKED

[Peter Kasting, 2014/05/23 22:38:43]

This comment will be obvious once this constant is declared in the function
below (see next comment), and should be removed.

[varkha, 2014/05/23 22:50:56]

See my next comment.

+const char kTabStripLayoutType[] = "tab_strip_layout_type";

[Peter Kasting, 2014/05/23 22:38:43]

Declare constants as locally as possible, in this case in the function below.

[varkha, 2014/05/23 22:50:56]

There are two functions that use the constant.

+
+}  // namespace
+
+void RegisterPrefs(PrefRegistrySimple* registry) {
+  // This value is device dependent, so it goes in local state.

[Peter Kasting, 2014/05/23 22:38:43]

This function doesn't necessarily know whether |registry| represents local
state, so this comment doesn't belong in this function.  I would just remove
this.  If it needs to be said, it should be said in a header or something.

[varkha, 2014/05/23 22:50:56]

Done (removed).

+  registry->RegisterIntegerPref(kTabStripLayoutType, 0);
+  registry->RegisterBooleanPref(prefs::kTabStripStackedLayout, false);
 }
 
+void MigratePrefs(PrefService* pref) {

[Peter Kasting, 2014/05/23 22:38:43]

Nit: Call this "prefs" -- "pref" sounds like one pref.

[varkha, 2014/05/23 22:50:56]

Done.

+  // Convert possibly existing non-default value for tabstrip layout type into
+  // boolean value indicating stacked layout preference.

[Peter Kasting, 2014/05/23 22:38:43]

This comment is confusing and redundant with the code; remove it.

[varkha, 2014/05/23 22:50:56]

Moved to header and simplified.

+  if (pref->HasPrefPath(kTabStripLayoutType)) {
+    if (pref->GetInteger(kTabStripLayoutType) != 0)
+      pref->SetBoolean(prefs::kTabStripStackedLayout, true);

[Peter Kasting, 2014/05/23 22:38:43]

Nit: Should we just do:

    pref->SetBoolean(prefs::kTabStripStackedLayout,
                     pref->GetInteger(kTabStripLayoutType) != 0);

[varkha, 2014/05/23 22:50:56]

Done.

+    pref->ClearPref(kTabStripLayoutType);
+  }
+}
+
+}  // namespace tab_strip_layout_type_prefs
 }  // namespace chrome