Fixed behavior in case of disabled devtools

chrome/browser/devtools/devtools_window.cc

 // 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.
 
 #include "chrome/browser/devtools/devtools_window.h"
 
 #include <algorithm>
 
 #include "base/json/json_reader.h"
 #include "base/metrics/histogram.h"
@@ skipping 154 matching lines @@
       inspected_contents_observer_->web_contents();
   if (!inspected_contents)
     return NULL;
   Browser* browser = NULL;
   int tab = 0;
   if (FindInspectedBrowserAndTabIndex(inspected_contents, &browser, &tab))
     return browser->window();
   return NULL;
 }
 
+bool AreDeveloperToolsDisabled(
+    const WebContents* const inspected_web_contents) {
+  if (!inspected_web_contents) return false;
+  Profile* profile =
+      Profile::FromBrowserContext(inspected_web_contents->GetBrowserContext());
+  return profile->GetPrefs()->GetBoolean(prefs::kDevToolsDisabled);
+}
+
 }  // namespace
 
 // DevToolsEventForwarder -----------------------------------------------------
 
 class DevToolsEventForwarder {
  public:
   explicit DevToolsEventForwarder(DevToolsWindow* window)
      : devtools_window_(window) {}
 
   // Registers whitelisted shortcuts with the forwarder.
@@ skipping 196 matching lines @@
   for (DevToolsWindows::iterator it(instances->begin()); it != instances->end();
        ++it) {
     if ((*it)->main_web_contents_ == web_contents ||
         (*it)->toolbox_web_contents_ == web_contents)
       return true;
   }
   return false;
 }
 
 // static
-DevToolsWindow* DevToolsWindow::OpenDevToolsWindowForWorker(
+void DevToolsWindow::OpenDevToolsWindowForWorker(
     Profile* profile,
     const scoped_refptr<DevToolsAgentHost>& worker_agent) {
   DevToolsWindow* window = FindDevToolsWindow(worker_agent.get());
   if (!window) {
     window = DevToolsWindow::CreateDevToolsWindowForWorker(profile);
+
     DCHECK(window);

[dgozman, 2015/03/10 19:36:34]

Remove DCHECK.

+    if (!window)
+      return;
+
     window->bindings_->AttachTo(worker_agent);
   }
   window->ScheduleShow(DevToolsToggleAction::Show());
-  return window;
 }
 
 // static
 DevToolsWindow* DevToolsWindow::CreateDevToolsWindowForWorker(
     Profile* profile) {
   content::RecordAction(base::UserMetricsAction("DevTools_InspectWorker"));
   return Create(profile, GURL(), NULL, true, std::string(), false, "");
 }
 
 // static
-DevToolsWindow* DevToolsWindow::OpenDevToolsWindow(
+void DevToolsWindow::OpenDevToolsWindow(
     content::WebContents* inspected_web_contents) {
-  return ToggleDevToolsWindow(
-      inspected_web_contents, true, DevToolsToggleAction::Show(), "");
+  DevToolsWindow::OpenDevToolsWindow(inspected_web_contents,

[dgozman, 2015/03/10 19:36:34]

nit: let's revert back to ToggleDevToolsWindow

+                                     DevToolsToggleAction::Show());
 }
 
 // static
-DevToolsWindow* DevToolsWindow::OpenDevToolsWindow(
+void DevToolsWindow::OpenDevToolsWindow(
     content::WebContents* inspected_web_contents,
     const DevToolsToggleAction& action) {
-  return ToggleDevToolsWindow(inspected_web_contents, true, action, "");
+  ToggleDevToolsWindow(inspected_web_contents, true, action, "");
 }
 
 // static
-DevToolsWindow* DevToolsWindow::OpenDevToolsWindow(
+void DevToolsWindow::OpenDevToolsWindow(
     Profile* profile,
     const scoped_refptr<content::DevToolsAgentHost>& agent_host) {
   DevToolsWindow* window = FindDevToolsWindow(agent_host.get());
   if (!window) {
     window = DevToolsWindow::Create(
         profile, GURL(), nullptr, false, std::string(), false, std::string());
+
     DCHECK(window);

[dgozman, 2015/03/10 19:36:34]

remove DCHECK

+    if (!window)
+      return;
+
     window->bindings_->AttachTo(agent_host);
   }
   window->ScheduleShow(DevToolsToggleAction::Show());
-  return window;
 }
 
 // static
-DevToolsWindow* DevToolsWindow::ToggleDevToolsWindow(
+void DevToolsWindow::ToggleDevToolsWindow(
     Browser* browser,
     const DevToolsToggleAction& action) {
   if (action.type() == DevToolsToggleAction::kToggle &&
       browser->is_devtools()) {
     browser->tab_strip_model()->CloseAllTabs();
-    return NULL;
+    return;
   }
 
-  return ToggleDevToolsWindow(
+  ToggleDevToolsWindow(
       browser->tab_strip_model()->GetActiveWebContents(),
       action.type() == DevToolsToggleAction::kInspect,
       action, "");
 }
 
 // static
 void DevToolsWindow::OpenExternalFrontend(
     Profile* profile,
     const std::string& frontend_url,
     const scoped_refptr<content::DevToolsAgentHost>& agent_host,
     bool isWorker) {
   DevToolsWindow* window = FindDevToolsWindow(agent_host.get());
   if (!window) {
     window = Create(profile, GURL(), nullptr, isWorker,
         DevToolsUI::GetProxyURL(frontend_url).spec(), false, std::string());
-    if (window)
-      window->bindings_->AttachTo(agent_host);
+
+    if (!window)
+      return;
+
+    window->bindings_->AttachTo(agent_host);
   }
-  if (window)
-    window->ScheduleShow(DevToolsToggleAction::Show());
+
+  window->ScheduleShow(DevToolsToggleAction::Show());
 }
 
 // static
-DevToolsWindow* DevToolsWindow::ToggleDevToolsWindow(
+void DevToolsWindow::ToggleDevToolsWindow(
     content::WebContents* inspected_web_contents,
     bool force_open,
     const DevToolsToggleAction& action,
     const std::string& settings) {
   scoped_refptr<DevToolsAgentHost> agent(
       DevToolsAgentHost::GetOrCreateFor(inspected_web_contents));
   DevToolsWindow* window = FindDevToolsWindow(agent.get());
   bool do_open = force_open;
   if (!window) {
     Profile* profile = Profile::FromBrowserContext(
         inspected_web_contents->GetBrowserContext());
     content::RecordAction(
         base::UserMetricsAction("DevTools_InspectRenderer"));
     window = Create(profile, GURL(), inspected_web_contents,
                     false, std::string(), true, settings);
+
     DCHECK(window);

[dgozman, 2015/03/10 19:36:34]

remove DCHECK

+    if (!window)
+      return;
+
     window->bindings_->AttachTo(agent.get());
     do_open = true;
   }
 
   // Update toolbar to reflect DevTools changes.
   window->UpdateBrowserToolbar();
 
   // If window is docked and visible, we hide it on toggle. If window is
   // undocked, we show (activate) it.
   if (!window->is_docked_ || do_open)
     window->ScheduleShow(action);
   else
     window->CloseWindow();
-
-  return window;
 }
 
 // static
 void DevToolsWindow::InspectElement(
     content::WebContents* inspected_web_contents,
     int x,
     int y) {
   scoped_refptr<DevToolsAgentHost> agent(
       DevToolsAgentHost::GetOrCreateFor(inspected_web_contents));
   agent->InspectElement(x, y);
   bool should_measure_time = FindDevToolsWindow(agent.get()) == NULL;
   base::TimeTicks start_time = base::TimeTicks::Now();
   // TODO(loislo): we should initiate DevTools window opening from within
   // renderer. Otherwise, we still can hit a race condition here.
-  DevToolsWindow* window = OpenDevToolsWindow(inspected_web_contents);
-  if (should_measure_time)
+  OpenDevToolsWindow(inspected_web_contents);
+
+  DevToolsWindow* window = FindDevToolsWindow(agent.get());
+  if (should_measure_time && window)
     window->inspect_element_start_time_ = start_time;
 }
 
 void DevToolsWindow::ScheduleShow(const DevToolsToggleAction& action) {
   if (life_stage_ == kLoadCompleted) {
     Show(action);
     return;
   }
 
   // Action will be done only after load completed.
@@ skipping 183 matching lines @@
 
 // static
 DevToolsWindow* DevToolsWindow::Create(
     Profile* profile,
     const GURL& frontend_url,
     content::WebContents* inspected_web_contents,
     bool shared_worker_frontend,
     const std::string& remote_frontend,
     bool can_dock,
     const std::string& settings) {
+  // If development tools disabled by policy don't open the window.
+  if (AreDeveloperToolsDisabled(inspected_web_contents))

[dgozman, 2015/03/10 19:36:34]

I'd just inline the function here. Note that |profile| is already passed as a
parameter.

+    return nullptr;
   if (inspected_web_contents) {
     // Check for a place to dock.
     Browser* browser = NULL;
     int tab;
     if (!FindInspectedBrowserAndTabIndex(inspected_web_contents,
                                          &browser, &tab) ||
         browser->is_type_popup()) {
       can_dock = false;
     }
   }
@@ skipping 485 matching lines @@
       closure.Run();
     return;
   }
   load_completed_callback_ = closure;
 }
 
 bool DevToolsWindow::ForwardKeyboardEvent(
     const content::NativeWebKeyboardEvent& event) {
   return event_forwarder_->ForwardEvent(event);
 }