[Extenisons Bindings] Don't throw unchecked errors; add console errors

chrome/browser/extensions/native_bindings_apitest.cc

 // 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.
 
 #include <memory>
 
 #include "base/command_line.h"
 #include "base/run_loop.h"
 #include "chrome/browser/extensions/api/extension_action/extension_action_api.h"
 #include "chrome/browser/extensions/api/file_system/file_system_api.h"
@@ skipping 175 matching lines @@
       TestRenderViewContextMenu::Create(
           web_contents, GURL("https://www.example.com"), GURL(), GURL()));
 
   ExtensionTestMessageListener listener("clicked", false);
   int command_id = ContextMenuMatcher::ConvertToExtensionsCustomCommandId(0);
   EXPECT_TRUE(menu->IsCommandIdEnabled(command_id));
   menu->ExecuteCommand(command_id, 0);
   EXPECT_TRUE(listener.WaitUntilSatisfied());
 }
 
+// Tests that unchecked errors don't impede future calls.
+IN_PROC_BROWSER_TEST_F(NativeBindingsApiTest, ErrorsInCallbackTest) {
+  host_resolver()->AddRule("*", "127.0.0.1");
+  embedded_test_server()->ServeFilesFromDirectory(test_data_dir_);
+  ASSERT_TRUE(StartEmbeddedTestServer());
+
+  TestExtensionDir test_dir;
+  test_dir.WriteManifest(
+      R"({
+           "name": "Errors In Callback",
+           "manifest_version": 2,
+           "version": "0.1",
+           "permissions": ["contextMenus"],
+           "background": {
+             "scripts": ["background.js"]
+           }
+         })");
+  test_dir.WriteFile(
+      FILE_PATH_LITERAL("background.js"),
+      R"(chrome.tabs.query({}, function(tabs) {
+           chrome.tabs.executeScript(tabs[0].id, {code: 'x'}, function() {
+             // There's an error here (we don't have permission to access the
+             // host), but we don't check it so that it gets surfaced as an
+             // unchecked runtime.lastError.
+             // We should still be able to invoke other APIs and get correct
+             // callbacks.
+             chrome.tabs.query({}, function(tabs) {
+               chrome.tabs.query({}, function(tabs) {

[Devlin, 2017/04/14 00:43:17]

Interestingly, this level of nesting is necessary to cause the weirdness with
exceptions.  I would have expected that after the call to execute script, one
additional level would be sufficient (the exception is thrown after the callback
to executeScript finishes), but that alone isn't enough.  From experimentation
in some of the browser tests that failed because of this, I also sometimes saw a
limited number of js calls executing before it broke - but I'm not sure what the
deterministic way of telling how many calls are allowed would be.

I'm probably content to leave this as-is, but let me know if y'all have any
ideas.

+                 chrome.test.sendMessage('callback');
+               });
+             });
+           });
+         });)");
+
+  ui_test_utils::NavigateToURL(
+      browser(), embedded_test_server()->GetURL(
+                     "example.com", "/native_bindings/simple.html"));
+
+  const Extension* extension = nullptr;

[jbroman, 2017/04/19 17:45:04]

nit: Since there's nothing below the below block, this could probably just be:

  ExtensionTestMessageListener listener("callback", false);
  ASSERT_TRUE(LoadExtension(test_dir.UnpackedPath());
  EXPECT_TRUE(listener.WaitUntilSatisfied());

But I don't feel strongly at all.

[Devlin, 2017/04/19 19:45:33]

Whoops, copy-paste leftovers.  Cleaned.

+  {
+    ExtensionTestMessageListener listener("callback", false);
+    extension = LoadExtension(test_dir.UnpackedPath());
+    ASSERT_TRUE(extension);
+    EXPECT_TRUE(listener.WaitUntilSatisfied());
+  }
+}
+
 }  // namespace extensions