From e7057530af5dbb422f1de1c72785c5c1b12396ae Mon Sep 17 00:00:00 2001 From: "rdevlin.cronin" Date: Thu, 6 Aug 2015 19:13:34 -0700 Subject: [PATCH] [Extensions] Fix a developerPrivate api bug with app window inspection The developerPrivate was accidentally using the web contents of an app window's routing id as the render frame routing id. Correct this mistake, and add a test. BUG=512602 Review URL: https://codereview.chromium.org/1276103002 Cr-Commit-Position: refs/heads/master@{#342266} --- .../developer_private/developer_private_apitest.cc | 60 ++++++++++++++++++++++ .../developer_private/inspectable_views_finder.cc | 8 ++- chrome/browser/extensions/extension_browsertest.cc | 17 ++++++ chrome/browser/extensions/extension_browsertest.h | 3 ++ 4 files changed, 83 insertions(+), 5 deletions(-) diff --git a/chrome/browser/extensions/api/developer_private/developer_private_apitest.cc b/chrome/browser/extensions/api/developer_private/developer_private_apitest.cc index ffd931ae3b76..68f1b2f3d90d 100644 --- a/chrome/browser/extensions/api/developer_private/developer_private_apitest.cc +++ b/chrome/browser/extensions/api/developer_private/developer_private_apitest.cc @@ -2,7 +2,15 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "base/path_service.h" +#include "base/strings/stringprintf.h" +#include "chrome/browser/devtools/devtools_window.h" +#include "chrome/browser/extensions/api/developer_private/developer_private_api.h" #include "chrome/browser/extensions/extension_apitest.h" +#include "chrome/browser/extensions/extension_function_test_utils.h" +#include "chrome/common/chrome_paths.h" +#include "extensions/browser/app_window/app_window.h" +#include "extensions/browser/app_window/app_window_registry.h" namespace extensions { @@ -21,4 +29,56 @@ IN_PROC_BROWSER_TEST_F(DeveloperPrivateApiTest, Basics) { "developer/test", kFlagLoadAsComponent)); } +// Tests opening the developer tools for an app window. +IN_PROC_BROWSER_TEST_F(DeveloperPrivateApiTest, InspectAppWindowView) { + base::FilePath dir; + PathService::Get(chrome::DIR_TEST_DATA, &dir); + dir = dir.AppendASCII("extensions") + .AppendASCII("platform_apps") + .AppendASCII("minimal"); + + // Load and launch a platform app. + const Extension* app = LoadAndLaunchApp(dir); + + // Get the info about the app, including the inspectable views. + scoped_refptr function( + new api::DeveloperPrivateGetExtensionInfoFunction()); + scoped_ptr result( + extension_function_test_utils::RunFunctionAndReturnSingleResult( + function.get(), base::StringPrintf("[\"%s\"]", app->id().c_str()), + browser())); + ASSERT_TRUE(result); + scoped_ptr info = + api::developer_private::ExtensionInfo::FromValue(*result); + ASSERT_TRUE(info); + + // There should be two inspectable views - the background page and the app + // window. Find the app window. + ASSERT_EQ(2u, info->views.size()); + api::developer_private::ExtensionView* window_view = nullptr; + for (const auto& view : info->views) { + if (view->type == api::developer_private::VIEW_TYPE_APP_WINDOW) { + window_view = view.get(); + break; + } + } + ASSERT_TRUE(window_view); + + // Inspect the app window. + function = new api::DeveloperPrivateOpenDevToolsFunction(); + extension_function_test_utils::RunFunction( + function.get(), + base::StringPrintf("[{\"renderViewId\": %d, \"renderProcessId\": %d}]", + window_view->render_view_id, + window_view->render_process_id), + browser(), extension_function_test_utils::NONE); + + // Verify that dev tools opened. + std::list app_windows = + AppWindowRegistry::Get(profile())->GetAppWindowsForApp(app->id()); + ASSERT_EQ(1u, app_windows.size()); + EXPECT_TRUE(DevToolsWindow::GetInstanceForInspectedWebContents( + (*app_windows.begin())->web_contents())); +} + } // namespace extensions diff --git a/chrome/browser/extensions/api/developer_private/inspectable_views_finder.cc b/chrome/browser/extensions/api/developer_private/inspectable_views_finder.cc index a13a662ea1de..98f21933b505 100644 --- a/chrome/browser/extensions/api/developer_private/inspectable_views_finder.cc +++ b/chrome/browser/extensions/api/developer_private/inspectable_views_finder.cc @@ -175,11 +175,9 @@ void InspectableViewsFinder::GetAppWindowViewsForExtension( url = window->initial_url(); content::RenderProcessHost* process = web_contents->GetRenderProcessHost(); - result->push_back(ConstructView(url, - process->GetID(), - web_contents->GetRoutingID(), - false, - GetViewType(web_contents))); + result->push_back(ConstructView( + url, process->GetID(), web_contents->GetMainFrame()->GetRoutingID(), + false, GetViewType(web_contents))); } } diff --git a/chrome/browser/extensions/extension_browsertest.cc b/chrome/browser/extensions/extension_browsertest.cc index 2f29b806f3f4..78287e06634c 100644 --- a/chrome/browser/extensions/extension_browsertest.cc +++ b/chrome/browser/extensions/extension_browsertest.cc @@ -30,6 +30,7 @@ #include "chrome/browser/profiles/profile_manager.h" #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser_window.h" +#include "chrome/browser/ui/extensions/application_launch.h" #include "chrome/browser/ui/tabs/tab_strip_model.h" #include "chrome/common/chrome_paths.h" #include "chrome/common/chrome_switches.h" @@ -292,6 +293,22 @@ const Extension* ExtensionBrowserTest::LoadExtensionAsComponent( extensions::kManifestFilename); } +const Extension* ExtensionBrowserTest::LoadAndLaunchApp( + const base::FilePath& path) { + const Extension* app = LoadExtension(path); + CHECK(app); + content::WindowedNotificationObserver app_loaded_observer( + content::NOTIFICATION_LOAD_COMPLETED_MAIN_FRAME, + content::NotificationService::AllSources()); + AppLaunchParams params(profile(), app, extensions::LAUNCH_CONTAINER_NONE, + NEW_WINDOW, extensions::SOURCE_TEST); + params.command_line = *base::CommandLine::ForCurrentProcess(); + OpenApplication(params); + app_loaded_observer.Wait(); + + return app; +} + base::FilePath ExtensionBrowserTest::PackExtension( const base::FilePath& dir_path) { base::FilePath crx_path = temp_dir_.path().AppendASCII("temp.crx"); diff --git a/chrome/browser/extensions/extension_browsertest.h b/chrome/browser/extensions/extension_browsertest.h index c10220ed2d49..3645bfde3e03 100644 --- a/chrome/browser/extensions/extension_browsertest.h +++ b/chrome/browser/extensions/extension_browsertest.h @@ -111,6 +111,9 @@ class ExtensionBrowserTest : virtual public InProcessBrowserTest { const extensions::Extension* LoadExtensionAsComponent( const base::FilePath& path); + // Loads and launches the app from |path|, and returns it. + const extensions::Extension* LoadAndLaunchApp(const base::FilePath& path); + // Pack the extension in |dir_path| into a crx file and return its path. // Return an empty FilePath if there were errors. base::FilePath PackExtension(const base::FilePath& dir_path); -- 2.11.4.GIT