Fixed behavior in case of disabled devtools

BUG=460543

Review URL: https://codereview.chromium.org/972123003

Cr-Commit-Position: refs/heads/master@{#320257}

5 files changed, 85 insertions, 42 deletions

diff --git a/chrome/browser/devtools/devtools_sanity_browsertest.cc b/chrome/browser/devtools/devtools_sanity_browsertest.cc
index e8500cf..78bd4c7 100644
--- a/chrome/browser/devtools/devtools_sanity_browsertest.cc
+++ b/chrome/browser/devtools/devtools_sanity_browsertest.cc
@@ -27,6 +27,7 @@
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
+#include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/test_switches.h"
@@ -812,7 +813,6 @@ IN_PROC_BROWSER_TEST_F(DevToolsSanityTest, DISABLED_TestDeviceEmulation) {
   RunTest("testDeviceMetricsOverrides", "about:blank");
 }
 
-
 // Tests that external navigation from inspector page is always handled by
 // DevToolsWindow and results in inspected page navigation.
 IN_PROC_BROWSER_TEST_F(DevToolsSanityTest, TestDevToolsExternalNavigation) {
@@ -953,3 +953,16 @@ IN_PROC_BROWSER_TEST_F(RemoteDebuggingTest, MAYBE_RemoteDebugger) {
 
   ASSERT_TRUE(RunExtensionTest("target_list")) << message_;
 }
+
+using DevToolsPolicyTest = InProcessBrowserTest;
+IN_PROC_BROWSER_TEST_F(DevToolsPolicyTest, PolicyTrue) {
+  browser()->profile()->GetPrefs()->SetBoolean(prefs::kDevToolsDisabled, true);
+  ui_test_utils::NavigateToURL(browser(), GURL("about:blank"));
+  content::WebContents* web_contents =
+      browser()->tab_strip_model()->GetWebContentsAt(0);
+  scoped_refptr<content::DevToolsAgentHost> agent(
+      content::DevToolsAgentHost::GetOrCreateFor(web_contents));
+  DevToolsWindow::OpenDevToolsWindow(web_contents);
+  DevToolsWindow* window = DevToolsWindow::FindDevToolsWindow(agent.get());
+  ASSERT_FALSE(window);
+}
diff --git a/chrome/browser/devtools/devtools_window.cc b/chrome/browser/devtools/devtools_window.cc
index e7df5db..0d18677 100644
--- a/chrome/browser/devtools/devtools_window.cc
+++ b/chrome/browser/devtools/devtools_window.cc
@@ -171,7 +171,6 @@ BrowserWindow* DevToolsToolboxDelegate::GetInspectedBrowserWindow() {
     return browser->window();
   return NULL;
 }
-
 }  // namespace
 
 // DevToolsEventForwarder -----------------------------------------------------
@@ -388,17 +387,17 @@ bool DevToolsWindow::IsDevToolsWindow(content::WebContents* web_contents) {
 }
 
 // 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);
+    if (!window)
+      return;
     window->bindings_->AttachTo(worker_agent);
   }
   window->ScheduleShow(DevToolsToggleAction::Show());
-  return window;
 }
 
 // static
@@ -409,45 +408,45 @@ DevToolsWindow* DevToolsWindow::CreateDevToolsWindowForWorker(
 }
 
 // static
-DevToolsWindow* DevToolsWindow::OpenDevToolsWindow(
+void DevToolsWindow::OpenDevToolsWindow(
     content::WebContents* inspected_web_contents) {
-  return ToggleDevToolsWindow(
-      inspected_web_contents, true, DevToolsToggleAction::Show(), "");
+  ToggleDevToolsWindow(
+        inspected_web_contents, true, 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);
+    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, "");
@@ -463,15 +462,16 @@ void DevToolsWindow::OpenExternalFrontend(
   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,
@@ -487,7 +487,8 @@ DevToolsWindow* DevToolsWindow::ToggleDevToolsWindow(
         base::UserMetricsAction("DevTools_InspectRenderer"));
     window = Create(profile, GURL(), inspected_web_contents,
                     false, std::string(), true, settings);
-    DCHECK(window);
+    if (!window)
+      return;
     window->bindings_->AttachTo(agent.get());
     do_open = true;
   }
@@ -501,8 +502,6 @@ DevToolsWindow* DevToolsWindow::ToggleDevToolsWindow(
     window->ScheduleShow(action);
   else
     window->CloseWindow();
-
-  return window;
 }
 
 // static
@@ -517,8 +516,10 @@ void DevToolsWindow::InspectElement(
   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;
 }
 
@@ -722,6 +723,10 @@ DevToolsWindow* DevToolsWindow::Create(
     const std::string& remote_frontend,
     bool can_dock,
     const std::string& settings) {
+  // If developer tools disabled by policy don't open the window.
+  if (profile->GetPrefs()->GetBoolean(prefs::kDevToolsDisabled))
+    return nullptr;
+
   if (inspected_web_contents) {
     // Check for a place to dock.
     Browser* browser = NULL;
diff --git a/chrome/browser/devtools/devtools_window.h b/chrome/browser/devtools/devtools_window.h
index ed6b166..f7ca8be 100644
--- a/chrome/browser/devtools/devtools_window.h
+++ b/chrome/browser/devtools/devtools_window.h
@@ -62,24 +62,37 @@ class DevToolsWindow : public DevToolsUIBindings::Delegate,
 
   static bool IsDevToolsWindow(content::WebContents* web_contents);
   static DevToolsWindow* AsDevToolsWindow(content::WebContents* web_contents);
+  static DevToolsWindow* FindDevToolsWindow(content::DevToolsAgentHost*);
 
   // Open or reveal DevTools window, and perform the specified action.
-  static DevToolsWindow* OpenDevToolsWindow(
-      content::WebContents* inspected_web_contents,
-      const DevToolsToggleAction& action);
+  // How to get pointer to the created window see comments for
+  // ToggleDevToolsWindow().
+  static void OpenDevToolsWindow(content::WebContents* inspected_web_contents,
+                                 const DevToolsToggleAction& action);
 
   // Open or reveal DevTools window, with no special action.
-  static DevToolsWindow* OpenDevToolsWindow(
-      content::WebContents* inspected_web_contents);
+  // How to get pointer to the created window see comments for
+  // ToggleDevToolsWindow().
+  static void OpenDevToolsWindow(content::WebContents* inspected_web_contents);
 
   // Open or reveal DevTools window. This window will be undocked.
-  static DevToolsWindow* OpenDevToolsWindow(
+  static void OpenDevToolsWindow(
       Profile* profile,
       const scoped_refptr<content::DevToolsAgentHost>& agent_host);
 
   // Perform specified action for current WebContents inside a |browser|.
   // This may close currently open DevTools window.
-  static DevToolsWindow* ToggleDevToolsWindow(
+  // If DeveloperToolsDisabled policy is set, no DevTools window created.
+  // In case if needed pointer to the created window one should use
+  // DevToolsAgentHost and DevToolsWindow::FindDevToolsWindow(). E.g.:
+  //
+  // scoped_refptr<content::DevToolsAgentHost> agent(
+  //   content::DevToolsAgentHost::GetOrCreateFor(inspected_web_contents));
+  // DevToolsWindow::ToggleDevToolsWindow(
+  //   inspected_web_contents, DevToolsToggleAction::Show());
+  // DevToolsWindow* window = DevToolsWindow::FindDevToolsWindow(agent.get());
+  //
+  static void ToggleDevToolsWindow(
       Browser* browser,
       const DevToolsToggleAction& action);
 
@@ -91,7 +104,7 @@ class DevToolsWindow : public DevToolsUIBindings::Delegate,
       bool isWorker);
 
   // Worker frontend is always undocked.
-  static DevToolsWindow* OpenDevToolsWindowForWorker(
+  static void OpenDevToolsWindowForWorker(
       Profile* profile,
       const scoped_refptr<content::DevToolsAgentHost>& worker_agent);
 
@@ -228,9 +241,9 @@ class DevToolsWindow : public DevToolsUIBindings::Delegate,
                              const std::string& remote_frontend,
                              bool can_dock,
                              const std::string& settings);
-  static DevToolsWindow* FindDevToolsWindow(content::DevToolsAgentHost*);
+
   static DevToolsWindow* CreateDevToolsWindowForWorker(Profile* profile);
-  static DevToolsWindow* ToggleDevToolsWindow(
+  static void ToggleDevToolsWindow(
       content::WebContents* web_contents,
       bool force_open,
       const DevToolsToggleAction& action,
diff --git a/chrome/browser/devtools/devtools_window_testing.cc b/chrome/browser/devtools/devtools_window_testing.cc
index 3df89b45..26fa6e3 100644
--- a/chrome/browser/devtools/devtools_window_testing.cc
+++ b/chrome/browser/devtools/devtools_window_testing.cc
@@ -106,8 +106,11 @@ DevToolsWindow* DevToolsWindowTesting::OpenDevToolsWindowSync(
   std::string settings = is_docked ?
       "{\"currentDockState\":\"\\\"bottom\\\"\"}" :
       "{\"currentDockState\":\"\\\"undocked\\\"\"}";
-  DevToolsWindow* window = DevToolsWindow::ToggleDevToolsWindow(
-      inspected_web_contents, true, DevToolsToggleAction::Show(), settings);
+  scoped_refptr<content::DevToolsAgentHost> agent(
+      content::DevToolsAgentHost::GetOrCreateFor(inspected_web_contents));
+  DevToolsWindow::ToggleDevToolsWindow(
+        inspected_web_contents, true, DevToolsToggleAction::Show(), settings);
+  DevToolsWindow* window = DevToolsWindow::FindDevToolsWindow(agent.get());
   WaitForDevToolsWindowLoad(window);
   return window;
 }
@@ -123,8 +126,9 @@ DevToolsWindow* DevToolsWindowTesting::OpenDevToolsWindowSync(
 // static
 DevToolsWindow* DevToolsWindowTesting::OpenDevToolsWindowForWorkerSync(
     Profile* profile, content::DevToolsAgentHost* worker_agent) {
-  DevToolsWindow* window = DevToolsWindow::OpenDevToolsWindowForWorker(
+  DevToolsWindow::OpenDevToolsWindowForWorker(
       profile, worker_agent);
+  DevToolsWindow* window = DevToolsWindow::FindDevToolsWindow(worker_agent);
   WaitForDevToolsWindowLoad(window);
   return window;
 }
diff --git a/chrome/browser/ui/apps/chrome_app_window_client.cc b/chrome/browser/ui/apps/chrome_app_window_client.cc
index fc9eb50..bcdecbd 100644
--- a/chrome/browser/ui/apps/chrome_app_window_client.cc
+++ b/chrome/browser/ui/apps/chrome_app_window_client.cc
@@ -8,6 +8,7 @@
 #include "chrome/browser/apps/scoped_keep_alive.h"
 #include "chrome/browser/devtools/devtools_window.h"
 #include "chrome/common/extensions/features/feature_channel.h"
+#include "content/public/browser/devtools_agent_host.h"
 #include "extensions/browser/app_window/app_window.h"
 #include "extensions/common/extension.h"
 
@@ -55,9 +56,16 @@ extensions::NativeAppWindow* ChromeAppWindowClient::CreateNativeAppWindow(
 void ChromeAppWindowClient::OpenDevToolsWindow(
     content::WebContents* web_contents,
     const base::Closure& callback) {
-  DevToolsWindow* devtools_window = DevToolsWindow::OpenDevToolsWindow(
-      web_contents, DevToolsToggleAction::ShowConsole());
-  devtools_window->SetLoadCompletedCallback(callback);
+  scoped_refptr<content::DevToolsAgentHost> agent(
+      content::DevToolsAgentHost::GetOrCreateFor(web_contents));
+  DevToolsWindow::OpenDevToolsWindow(web_contents);
+
+  DevToolsWindow* devtools_window =
+      DevToolsWindow::FindDevToolsWindow(agent.get());
+  if (devtools_window)
+    devtools_window->SetLoadCompletedCallback(callback);
+  else
+    callback.Run();
 }
 
 bool ChromeAppWindowClient::IsCurrentChannelOlderThanDev() {