Fixed behavior in case of disabled devtools

chrome/browser/devtools/devtools_window.h

Index: chrome/browser/devtools/devtools_window.h
diff --git a/chrome/browser/devtools/devtools_window.h b/chrome/browser/devtools/devtools_window.h
index 63922fc96130205204ea5d56b89304950a57d54d..643d486c61e03e4b1e9b704f326f8bee4bfb943f 100644
--- a/chrome/browser/devtools/devtools_window.h
+++ b/chrome/browser/devtools/devtools_window.h
@@ -63,13 +63,11 @@ class DevToolsWindow : public DevToolsUIBindings::Delegate,
   static bool IsDevToolsWindow(content::WebContents* web_contents);
 
   // Open or reveal DevTools window, and perform the specified action.
-  static DevToolsWindow* OpenDevToolsWindow(
-      content::WebContents* inspected_web_contents,
-      const DevToolsToggleAction& action);
+  static void OpenDevToolsWindow(content::WebContents* inspected_web_contents,

[dgozman, 2015/03/03 20:04:54]

Let's just mention in the comment to each method that returned window may be
null, and modify all the callsites to check for that. It's not convenient to use
DevToolsAgentHost and FindDevToolsWindow anytime I need the return value.

[Andrew T Wilson (Slow), 2015/03/04 13:32:42]

In my experience, this will lead to crashes because people will misread the
documentation or forget to do the check correctly, and won't test with dev tools
disabled - this happens all the time, for example, when sync is disabled by
command-line flags/policy. The current CL is a bit more foolproof.

If you feel strongly that we should continue to return a value, then we can make
this change, but I intentionally asked peletskyi@ to remove the return value
since only two callsites use it currently, and one of them already had a
DevToolsAgentHost.

Anyhow, let me know how strongly you feel about this - we can change it back to
returning a window if you still prefer, but I wanted to explain the motivation a
bit more.

[dgozman, 2015/03/04 13:44:48]

I don't feel strongly. Let's go with void.

+                                 const DevToolsToggleAction& action);
 
   // Open or reveal DevTools window, with no special action.
-  static DevToolsWindow* OpenDevToolsWindow(
-      content::WebContents* inspected_web_contents);
+  static void OpenDevToolsWindow(content::WebContents* inspected_web_contents);
 
   // Perform specified action for current WebContents inside a |browser|.
   // This may close currently open DevTools window.
@@ -174,6 +172,8 @@ class DevToolsWindow : public DevToolsUIBindings::Delegate,
   // by user.
   static void OnPageCloseCanceled(content::WebContents* contents);
 
+  static DevToolsWindow* FindDevToolsWindow(content::DevToolsAgentHost*);
+
  private:
   friend class DevToolsWindowTesting;
 
@@ -217,7 +217,7 @@ class DevToolsWindow : public DevToolsUIBindings::Delegate,
                              const std::string& remote_frontend,
                              bool can_dock,
                              const std::string& settings);
-  static DevToolsWindow* FindDevToolsWindow(content::DevToolsAgentHost*);
+
   static DevToolsWindow* AsDevToolsWindow(content::WebContents*);
   static DevToolsWindow* CreateDevToolsWindowForWorker(Profile* profile);
   static DevToolsWindow* ToggleDevToolsWindow(