MD Settings: Update some Settings handlers to use new JS lifecycle management.

chrome/browser/ui/webui/settings/chromeos/device_keyboard_handler.h

 // Copyright 2016 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_WEBUI_SETTINGS_CHROMEOS_DEVICE_KEYBOARD_HANDLER_H_
 #define CHROME_BROWSER_UI_WEBUI_SETTINGS_CHROMEOS_DEVICE_KEYBOARD_HANDLER_H_
 
 #include "base/macros.h"
+#include "base/scoped_observer.h"
 #include "chrome/browser/ui/webui/settings/settings_page_ui_handler.h"
 #include "ui/events/devices/input_device_event_observer.h"
 
 namespace base {
 class ListValue;
 }
 
 namespace content {
 class WebUI;
 }
 
+namespace ui {
+class DeviceDataManager;
+}
+
 class Profile;
 
 namespace chromeos {
 namespace settings {
 
 // Chrome OS "Keyboard" settings page UI handler.
 class KeyboardHandler
     : public ::settings::SettingsPageUIHandler,
       public ui::InputDeviceEventObserver {
  public:
   explicit KeyboardHandler(content::WebUI* webui);
   ~KeyboardHandler() override;
 
   // SettingsPageUIHandler implementation.
   void RegisterMessages() override;
+  void OnJavascriptAllowed() override;
+  void OnJavascriptDisallowed() override;

[michaelpg, 2016/05/11 17:31:21]

if Allowed has been called, will Disallowed always be called before the
destructor?

[tommycli, 2016/05/11 17:32:44]

No.

I wish I could make it always be called, but I couldn't figure out how to.
Destructors can't call virtual functions.

[Dan Beam, 2016/05/11 17:33:48]

not currently.  thought it *might* if we start to make RenderProcessGone() call
OnJavascriptDisallowed() and this happens predictably before the webcontents
going away (I'm not sure about that).

[Dan Beam, 2016/05/11 17:34:06]

though it might*

 
   // ui::InputDeviceEventObserver implementation.
   void OnKeyboardDeviceConfigurationChanged() override;
 
  private:
   // Initializes the page with the current keyboard information.
   void HandleInitialize(const base::ListValue* args);
 
   // Shows the Ash keyboard shortcuts overlay.
   void HandleShowKeyboardShortcutsOverlay(const base::ListValue* args) const;
 
   // Shows or hides the Caps Lock and Diamond key settings based on whether the
   // system status.
-  void UpdateShowKeys() const;
+  void UpdateShowKeys();
 
   Profile* profile_;  // Weak pointer.
 
+  ScopedObserver<ui::DeviceDataManager, KeyboardHandler> observer_;

[michaelpg, 2016/05/11 17:31:21]

the OnJavascriptAllowed/Disallowed stuff is great.

for my lernings: does the ScopedObserver do anything we couldn't do before? ie,
is it just "sugar" so we don't have to call AddObserver and RemoveObserver
manually from the destructor and the OnJavascript*llowed functions? (sugar is
cool, just curious if there's behavior i'm not seeing)

[tommycli, 2016/05/11 17:32:44]

The only reason I had to change it: OnJavascriptDisallowed is not guaranteed to
be called before destruction.

[tommycli, 2016/05/11 17:35:20]

(If I don't have a ScopedObserver, since OnJavascriptDisallowed is not called
before destruction, there can be a dangling reference to this object)

[Dan Beam, 2016/05/11 17:35:22]

this sugar is less error-prone as well

+
   DISALLOW_COPY_AND_ASSIGN(KeyboardHandler);
 };
 
 }  // namespace settings
 }  // namespace chromeos
 
 #endif  // CHROME_BROWSER_UI_WEBUI_SETTINGS_CHROMEOS_DEVICE_KEYBOARD_HANDLER_H_