Integrate ErrorConsole with Apps Dev Tool

chrome/browser/ui/webui/extensions/extension_error_ui_util.cc

Index: chrome/browser/ui/webui/extensions/extension_error_ui_util.cc
diff --git a/chrome/browser/ui/webui/extensions/extension_error_ui_util.cc b/chrome/browser/ui/webui/extensions/extension_error_ui_util.cc
new file mode 100644
index 0000000000000000000000000000000000000000..b695ffee0f199d80f63c5dab49655e58e1103c8e
--- /dev/null
+++ b/chrome/browser/ui/webui/extensions/extension_error_ui_util.cc
@@ -0,0 +1,187 @@
+// 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.
+
+#include "chrome/browser/ui/webui/extensions/extension_error_ui_util.h"
+
+#include "base/bind.h"
+#include "base/file_util.h"
+#include "base/files/file_path.h"
+#include "base/location.h"
+#include "base/strings/utf_string_conversions.h"
+#include "base/threading/sequenced_worker_pool.h"
+#include "base/values.h"
+#include "chrome/browser/devtools/devtools_window.h"
+#include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_finder.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/render_view_host.h"
+#include "content/public/browser/web_contents.h"
+#include "extensions/browser/extension_error.h"
+#include "extensions/browser/extension_system.h"
+#include "extensions/browser/file_highlighter.h"
+#include "extensions/common/constants.h"
+#include "extensions/common/extension.h"
+
+namespace extensions {
+namespace error_ui_util {
+
+namespace {
+
+// Keys for objects passed to and from extension error UI.
+const char kPathSuffixKey[] = "pathSuffix";
+const char kTitleKey[] = "title";
+
+std::string ReadFileToString(const base::FilePath& path) {
+  std::string data;
+  base::ReadFileToString(path, &data);

[Dan Beam, 2014/02/11 02:43:49]

i have a feeling this method uses |data| as an out-param because it requires
less copying -- why are you undoing this?

[Devlin, 2014/02/11 18:41:16]

base::ReadFileToString() actually returns a bool indicating the success, but, in
this case, if we fail, we don't really care, because we handle empty strings. 
This way, we can do the far more elegant PostTaskAndReply() at line 127, rather
than having a PostTask() with a manual reply, or using owned ptrs (which get
hairy in threads).  Also, with RVO, it actually shouldn't make a difference
(ReadFileToString() uses |data| as an out-param so it can return a bool, not for
speed purposes).

+  return data;
+}
+
+void GetManifestFileCallback(base::DictionaryValue* results,
+                             const std::string& key,
+                             const std::string& specific,
+                             const JavascriptResponseCallback& response,
+                             const std::string& contents) {
+  ManifestHighlighter highlighter(contents, key, specific);
+  highlighter.SetHighlightedRegions(results);
+  response.Run(*results);
+}
+
+void GetSourceFileCallback(base::DictionaryValue* results,
+                           int line_number,
+                           const JavascriptResponseCallback& response,
+                           const std::string& contents) {
+  SourceHighlighter highlighter(contents, line_number);
+  highlighter.SetHighlightedRegions(results);
+  response.Run(*results);
+}
+
+}  // namespace
+
+void HandleRequestFileSource(const base::DictionaryValue* args,
+                             Profile* profile,
+                             const JavascriptResponseCallback& response) {
+  // Three required arguments: extension_id, path_suffix, and error_message.
+  std::string extension_id;
+  base::FilePath::StringType path_suffix_string;
+  base::string16 error_message;
+
+  if (!args->GetString(kPathSuffixKey, &path_suffix_string) ||
+      !args->GetString(ExtensionError::kExtensionIdKey, &extension_id) ||
+      !args->GetString(ExtensionError::kMessageKey, &error_message)) {
+    NOTREACHED();
+    return;
+  }
+
+  const Extension* extension =
+      ExtensionSystem::Get(profile)->extension_service()->GetExtensionById(

[Dan Beam, 2014/02/11 02:43:49]

^ what if |extension_service()| is NULL?

[Devlin, 2014/02/11 18:41:16]

Hmm... I wonder if this can happen (this is only called by the chrome:extensions
page, which very much requires ExtensionService to be alive).  But, potentially,
if everything was closing down, maybe... Added a graceful handling to be safe.

+          extension_id, true /* include disabled */);
+
+  // Under no circumstances should we ever need to reference a file outside of
+  // the extension's directory. If it tries to, abort.
+  base::FilePath path_suffix(path_suffix_string);
+  if (path_suffix.ReferencesParent())
+    return;
+
+  base::FilePath path = extension->path().Append(path_suffix);
+
+  // Setting the title and the error message is the same for all file types.
+  scoped_ptr<base::DictionaryValue> results(new base::DictionaryValue);
+  results->SetString(kTitleKey,
+                     base::UTF8ToUTF16(extension->name()) +
+                         base::ASCIIToUTF16(": ") +
+                         path.BaseName().LossyDisplayName());
+  results->SetString(ExtensionError::kMessageKey, error_message);
+
+  base::Callback<void(const std::string&)> reply;
+  if (path_suffix_string == kManifestFilename) {
+    std::string manifest_key;
+    if (!args->GetString(ManifestError::kManifestKeyKey, &manifest_key)) {
+      NOTREACHED();
+      return;
+    }
+
+    // A "specific" location is optional.
+    std::string specific;
+    args->GetString(ManifestError::kManifestSpecificKey, &specific);
+
+    reply = base::Bind(&GetManifestFileCallback,
+                       base::Owned(results.release()),
+                       manifest_key,
+                       specific,
+                       response);
+  } else {
+    int line_number = 0;
+    args->GetInteger(RuntimeError::kLineNumberKey, &line_number);
+
+    reply = base::Bind(&GetSourceFileCallback,
+                       base::Owned(results.release()),
+                       line_number,
+                       response);
+  }
+
+  base::PostTaskAndReplyWithResult(content::BrowserThread::GetBlockingPool(),
+                                   FROM_HERE,
+                                   base::Bind(&ReadFileToString, path),
+                                   reply);
+}
+
+void HandleOpenDevTools(const base::DictionaryValue* args) {
+  int render_process_id = 0;
+  int render_view_id = 0;
+
+  // The render view and render process ids are required.
+  if (!args->GetInteger(RuntimeError::kRenderProcessIdKey,
+                        &render_process_id) ||
+      !args->GetInteger(RuntimeError::kRenderViewIdKey, &render_view_id)) {
+    NOTREACHED();
+    return;
+  }
+
+  content::RenderViewHost* rvh =
+      content::RenderViewHost::FromID(render_process_id, render_view_id);
+
+  // It's possible that the render view was closed since we last updated the
+  // links. Handle this gracefully.
+  if (!rvh)
+    return;
+
+  // If we include a url, we should inspect it specifically (and not just the
+  // render view).
+  base::string16 url;
+  if (args->GetString(RuntimeError::kUrlKey, &url)) {
+    // Line and column numbers are optional; default to the first line.
+    int line_number = 1;
+    int column_number = 1;
+    args->GetInteger(RuntimeError::kLineNumberKey, &line_number);
+    args->GetInteger(RuntimeError::kColumnNumberKey, &column_number);
+
+    // Line/column numbers are reported in display-friendly 1-based numbers,
+    // but are inspected in zero-based numbers.
+    DevToolsWindow::OpenDevToolsWindow(
+        rvh,
+        DevToolsToggleAction::Reveal(url, line_number - 1, column_number - 1));
+  } else {
+    DevToolsWindow::OpenDevToolsWindow(rvh);
+  }
+
+  // Once we open the inspector, we focus on the appropriate tab...
+  content::WebContents* web_contents =
+      content::WebContents::FromRenderViewHost(rvh);
+  Browser* browser = chrome::FindBrowserWithWebContents(web_contents);
+  // ... but background pages have no associated browser (and the inspector
+  // opens in its own window), so our work is done.
+  if (!browser)
+    return;
+
+  TabStripModel* tab_strip = browser->tab_strip_model();
+  tab_strip->ActivateTabAt(tab_strip->GetIndexOfWebContents(web_contents),

[Dan Beam, 2014/02/11 02:43:49]

^ what if !browser->is_type_tabbed() (i.e. it's a popup or an app?)

[Devlin, 2014/02/11 18:41:16]

It actually works fine now - for Browsers::TYPE_POPUP,
FindBrowserWithWebContents() returns null, DevTools opens in a new window, and
we return early.  But just in case that's actually a bug in FindBrowser...(), I
updated the comment and added a check to make sure it's tabbed.

+                           false);  // Not through direct user gesture.

[Dan Beam, 2014/02/11 02:43:49]

^ didn't the user click "Open dev tools"?

[Devlin, 2014/02/11 18:41:16]

Yeah, kinda anti-intuitive.

"|user_gesture| is true if the user actually clicked on the tab or navigated to
it using a keyboard command, false if the tab was activated as a by-product of
some other action."

I think this more falls into the "by-product of some other action" (since they
didn't click the tab, and I think the "keyboard commands" referenced would be
like Ctrl+2).

https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...

+}
+
+}  // namespace error_ui_util
+}  // namespace extensions