Theme Preferences for Distilled Pages

components/dom_distiller/core/reader_mode_preferences.h

+// 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.
+
+#ifndef COMPONENTS_DOM_DISTILLER_CORE_READER_MODE_PREFERENCES_H_
+#define COMPONENTS_DOM_DISTILLER_CORE_READER_MODE_PREFERENCES_H_
+
+#include "base/macros.h"
+#include "base/observer_list.h"
+
+class PrefService;
+
+namespace user_prefs {
+class PrefRegistrySyncable;
+}
+
+namespace dom_distiller {
+
+// Interface for preferences used in reader mode view

[nyquist, 2014/06/26 20:39:59]

Nit: All comments like this should end with a period.

[smaslo, 2014/06/27 18:33:18]

Done.

+class ReaderModePrefs {
+ public:
+
+  // Possible themes for reader mode

[nyquist, 2014/06/26 20:39:59]

Nit: end with period

[smaslo, 2014/06/27 18:33:18]

Done.

+  enum Theme {
+    kNormal,

[nyquist, 2014/06/26 20:39:58]

See comment in CSS about naming. Make sure these two enum values are also
updated.

[smaslo, 2014/06/27 18:33:18]

Done.

+    kHighContrast,
+    kSepia
+  };
+
+  explicit ReaderModePrefs(PrefService* pref_service);
+  ~ReaderModePrefs();
+  static void RegisterProfilePrefs(user_prefs::PrefRegistrySyncable* registry);
+
+  void SetThemePref(Theme new_theme);
+
+  class Observer {

[nyquist, 2014/06/26 20:39:58]

I think the observer class usually goes at the top.

[smaslo, 2014/06/27 18:33:18]

I put it below the enum because it needs to know about the enum.

+   public:
+    virtual void OnChangeTheme(ReaderModePrefs* reader_mode_prefs) = 0;

[nyquist, 2014/06/26 20:39:58]

Do you need the pointer to the ReaderModePrefs? Would it be better to help the
observer by passing in the new theme?

[smaslo, 2014/06/27 18:33:18]

Done.

+  };
+
+  void AddObserver(Observer* obs);
+  void RemoveObserver(Observer* obs);
+
+  const char* GetBodyCssClass();

[nyquist, 2014/06/26 20:39:59]

Could this method make take in a theme as an argument, and then move this method
to the viewer.[cc|h]?
Can this be const?

[smaslo, 2014/06/27 18:33:18]

I made it a static method rather than a const method.

+  Theme GetThemePref();

[nyquist, 2014/06/26 20:39:58]

Can this return const and also be const itself?

[smaslo, 2014/06/27 18:33:18]

Done.

+
+ private:
+  PrefService* pref_service_;
+  ObserverList<Observer> observer_list_;
+
+  void NotifyOnChangeTheme();
+
+  DISALLOW_COPY_AND_ASSIGN(ReaderModePrefs);
+};
+
+}  // namespace dom_distiller
+
+#endif  // COMPONENTS_DOM_DISTILLER_CORE_READER_MODE_PREFERENCES_H_