Skip to content

GitLab

T
tangled
Commit 902bffab authored by Dmitry Gozman's avatar Dmitry Gozman Committed by Commit Bot
Browse files
Options

[DevTools] Make Inspect Element work for OOPIF 

To support this, we issue inspect element command to the agent
beforehand, and dispatch it in the next session which attaches
(or the existing one if any). This moves InspectElement from
DevToolsSession interface to DevToolsAgent interface, as there
might be no session yet.

Note this could fail in case of multiple sessions, one of which is
DevTools frontend, by choosing the wrong one. This is a rare
case though, so we can tolerate that.

TBR=alexclarke@chromium.org

Bug: 755515, 804100
Change-Id: I331bfb7f5410868fd47bfe8d558b54b7a30de00c
Reviewed-on: https://chromium-review.googlesource.com/881522Reviewed-by: default avatarAlex Clarke <alexclarke@chromium.org>
Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Reviewed-by: default avatarPavel Feldman <pfeldman@chromium.org>
Commit-Queue: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#532220}
parent 72dea27a
Hide whitespace changes
Inline Side-by-side
Showing with 159 additions and 92 deletions
chrome/browser/devtools/devtools_sanity_browsertest.cc
View file @ 902bffab
......@@ -331,6 +331,20 @@ class DevToolsSanityTest : public InProcessBrowserTest {
DevToolsWindow* window_;
};
class SitePerProcessDevToolsSanityTest : public DevToolsSanityTest {
public:
void SetUpCommandLine(base::CommandLine* command_line) override {
DevToolsSanityTest::SetUpCommandLine(command_line);
content::IsolateAllSitesForTesting(command_line);
};
void SetUpOnMainThread() override {
DevToolsSanityTest::SetUpOnMainThread();
content::SetupCrossSiteRedirector(embedded_test_server());
ASSERT_TRUE(embedded_test_server()->Start());
}
};
// Used to block until a dev tools window gets beforeunload event.
class DevToolsWindowBeforeUnloadObserver
: public content::WebContentsObserver {
......@@ -2133,3 +2147,21 @@ IN_PROC_BROWSER_TEST_F(DevToolsSanityTest, TestOpenInNewTabFilter) {
EXPECT_EQ(opened_url, pair.second);
}
}
IN_PROC_BROWSER_TEST_F(SitePerProcessDevToolsSanityTest, InspectElement) {
GURL url(embedded_test_server()->GetURL("a.com", "/devtools/oopif.html"));
ui_test_utils::NavigateToURLBlockUntilNavigationsComplete(browser(), url, 2);
std::vector<RenderFrameHost*> frames = GetInspectedTab()->GetAllFrames();
ASSERT_EQ(2u, frames.size());
ASSERT_NE(frames[0]->GetProcess(), frames[1]->GetProcess());
RenderFrameHost* frame_host = frames[0]->GetParent() ? frames[0] : frames[1];
DevToolsWindowCreationObserver observer;
DevToolsWindow::InspectElement(frame_host, 100, 100);
observer.WaitForLoad();
DevToolsWindow* window = observer.devtools_window();
DispatchOnTestSuite(window, "testInspectedElementIs", "INSPECTED-DIV");
DevToolsWindowTesting::CloseDevToolsWindowSync(window);
}
chrome/browser/devtools/devtools_window.cc
View file @ 902bffab
......@@ -696,17 +696,15 @@ void DevToolsWindow::InspectElement(
WebContents::FromRenderFrameHost(inspected_frame_host);
scoped_refptr<DevToolsAgentHost> agent(
DevToolsAgentHost::GetOrCreateFor(web_contents));
agent->InspectElement(inspected_frame_host, 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.
OpenDevToolsWindow(web_contents, DevToolsToggleAction::ShowElementsPanel());
DevToolsWindow* window = FindDevToolsWindow(agent.get());
if (window) {
agent->InspectElement(window->bindings_, x, y);
if (should_measure_time)
window->inspect_element_start_time_ = start_time;
}
if (window && should_measure_time)
window->inspect_element_start_time_ = start_time;
}
// static
......
chrome/test/data/devtools/oopif.html 0 → 100644
View file @ 902bffab
<body>
<iframe width=300px height=300px></iframe>
<script>
var url = window.location.href;
url = url.replace(/a\.com/, "b.com");
url = url.replace(/oopif\.html/, "oopif_frame.html");
document.querySelector('iframe').setAttribute('src', url);
</script>
</body>
chrome/test/data/devtools/oopif_frame.html 0 → 100644
View file @ 902bffab
<inspected-div style="width: 500px; height: 500px; background: red; display: block;">some content</inspected-div>
content/browser/devtools/devtools_agent_host_impl.cc
View file @ 902bffab
......@@ -242,14 +242,9 @@ bool DevToolsAgentHostImpl::IsAttached() {
return !sessions_.empty();
}
void DevToolsAgentHostImpl::InspectElement(
DevToolsAgentHostClient* client,
int x,
int y) {
DevToolsSession* session = SessionByClient(client);
if (session)
InspectElement(session, x, y);
}
void DevToolsAgentHostImpl::InspectElement(RenderFrameHost* frame_host,
int x,
int y) {}
std::string DevToolsAgentHostImpl::GetId() {
return id_;
......@@ -319,12 +314,6 @@ void DevToolsAgentHostImpl::DispatchProtocolMessage(
DevToolsSession* session,
const std::string& message) {}
void DevToolsAgentHostImpl::InspectElement(
DevToolsSession* session,
int x,
int y) {
}
// static
void DevToolsAgentHost::DetachAllClients() {
if (!g_devtools_instances.IsCreated())
......
content/browser/devtools/devtools_agent_host_impl.h
View file @ 902bffab
......@@ -42,7 +42,7 @@ class CONTENT_EXPORT DevToolsAgentHostImpl : public DevToolsAgentHost {
bool DispatchProtocolMessage(DevToolsAgentHostClient* client,
const std::string& message) override;
bool IsAttached() override;
void InspectElement(DevToolsAgentHostClient* client, int x, int y) override;
void InspectElement(RenderFrameHost* frame_host, int x, int y) override;
std::string GetId() override;
std::string GetParentId() override;
std::string GetOpenerId() override;
......@@ -67,7 +67,6 @@ class CONTENT_EXPORT DevToolsAgentHostImpl : public DevToolsAgentHost {
virtual void DetachSession(DevToolsSession* session);
virtual void DispatchProtocolMessage(DevToolsSession* session,
const std::string& message);
virtual void InspectElement(DevToolsSession* session, int x, int y);
void NotifyCreated();
void NotifyNavigated(DevToolsAgentHostImpl* host);
......
content/browser/devtools/devtools_session.cc
View file @ 902bffab
......@@ -174,11 +174,6 @@ void DevToolsSession::ResumeSendingMessagesToAgent() {
suspended_messages_.clear();
}
void DevToolsSession::InspectElement(const gfx::Point& point) {
if (session_ptr_)
session_ptr_->InspectElement(point);
}
void DevToolsSession::sendProtocolResponse(
int call_id,
std::unique_ptr<protocol::Serializable> message) {
......
content/browser/devtools/devtools_session.h
View file @ 902bffab
......@@ -38,7 +38,6 @@ class DevToolsSession : public protocol::FrontendChannel,
void AttachToAgent(const blink::mojom::DevToolsAgentAssociatedPtr& agent);
void DispatchProtocolMessage(const std::string& message);
void InspectElement(const gfx::Point& point);
void SuspendSendingMessagesToAgent();
void ResumeSendingMessagesToAgent();
......
content/browser/devtools/render_frame_devtools_agent_host.cc
View file @ 902bffab
......@@ -364,25 +364,27 @@ void RenderFrameDevToolsAgentHost::DispatchProtocolMessage(
session->DispatchProtocolMessage(message);
}
void RenderFrameDevToolsAgentHost::InspectElement(
DevToolsSession* session,
int x,
int y) {
// When there are nested WebContents, the renderer expects coordinates
// relative to the main frame, so we need to transform the coordinates from
// the root space to the current WebContents' main frame space.
if (frame_tree_node_) {
if (auto* main_view =
frame_tree_node_->frame_tree()->GetMainFrame()->GetView()) {
gfx::Point transformed_point = gfx::ToRoundedPoint(
main_view->TransformRootPointToViewCoordSpace(gfx::PointF(x, y)));
x = transformed_point.x();
y = transformed_point.y();
void RenderFrameDevToolsAgentHost::InspectElement(RenderFrameHost* frame_host,
int x,
int y) {
FrameTreeNode* ftn =
static_cast<RenderFrameHostImpl*>(frame_host)->frame_tree_node();
RenderFrameDevToolsAgentHost* host =
static_cast<RenderFrameDevToolsAgentHost*>(GetOrCreateFor(ftn).get());
gfx::Point point(x, y);
// The renderer expects coordinates relative to the local frame root,
// so we need to transform the coordinates from the root space
// to the local frame root widget's space.
if (host->frame_host_) {
if (RenderWidgetHostView* view = host->frame_host_->GetView()) {
point = gfx::ToRoundedPoint(
view->TransformRootPointToViewCoordSpace(gfx::PointF(point)));
}
}
session->InspectElement(gfx::Point(x, y));
if (host->EnsureAgent())
host->agent_ptr_->InspectElement(point);
}
RenderFrameDevToolsAgentHost::~RenderFrameDevToolsAgentHost() {
......
content/browser/devtools/render_frame_devtools_agent_host.h
View file @ 902bffab
......@@ -101,7 +101,7 @@ class CONTENT_EXPORT RenderFrameDevToolsAgentHost
// DevToolsAgentHostImpl overrides.
void AttachSession(DevToolsSession* session) override;
void DetachSession(DevToolsSession* session) override;
void InspectElement(DevToolsSession* session, int x, int y) override;
void InspectElement(RenderFrameHost* frame_host, int x, int y) override;
void DispatchProtocolMessage(DevToolsSession* session,
const std::string& message) override;
......
content/public/browser/devtools_agent_host.h
View file @ 902bffab
......@@ -31,6 +31,7 @@ namespace content {
class BrowserContext;
class DevToolsExternalAgentProxyDelegate;
class DevToolsSocketFactory;
class RenderFrameHost;
class WebContents;
// Describes interface for managing devtools agents from browser process.
......@@ -133,10 +134,9 @@ class CONTENT_EXPORT DevToolsAgentHost
virtual bool DispatchProtocolMessage(DevToolsAgentHostClient* client,
const std::string& message) = 0;
// Starts inspecting element at position (|x|, |y|).
virtual void InspectElement(DevToolsAgentHostClient* client,
int x,
int y) = 0;
// Starts inspecting element at position (|x|, |y|) in the frame
// represented by |frame_host|.
virtual void InspectElement(RenderFrameHost* frame_host, int x, int y) = 0;
// Returns the unique id of the agent.
virtual std::string GetId() = 0;
......
content/shell/browser/shell_devtools_bindings.cc
View file @ 902bffab
......@@ -109,7 +109,7 @@ const size_t kMaxMessageChunkSize = IPC::Channel::kMaximumMessageSize / 4;
void ShellDevToolsBindings::InspectElementAt(int x, int y) {
if (agent_host_) {
agent_host_->InspectElement(this, x, y);
agent_host_->InspectElement(inspected_contents_->GetFocusedFrame(), x, y);
} else {
inspect_element_at_x_ = x;
inspect_element_at_y_ = y;
......@@ -160,8 +160,8 @@ void ShellDevToolsBindings::Attach() {
agent_host_ = DevToolsAgentHost::GetOrCreateFor(inspected_contents_);
agent_host_->AttachClient(this);
if (inspect_element_at_x_ != -1) {
agent_host_->InspectElement(this, inspect_element_at_x_,
inspect_element_at_y_);
agent_host_->InspectElement(inspected_contents_->GetFocusedFrame(),
inspect_element_at_x_, inspect_element_at_y_);
inspect_element_at_x_ = -1;
inspect_element_at_y_ = -1;
}
......
headless/public/util/testing/mock_devtools_agent_host.h
View file @ 902bffab
......@@ -27,7 +27,7 @@ class MockDevToolsAgentHost : public content::DevToolsAgentHost {
bool(int session_id, const std::string& message));
MOCK_METHOD0(IsAttached, bool());
MOCK_METHOD3(InspectElement,
void(content::DevToolsAgentHostClient* client, int x, int y));
void(content::RenderFrameHost* frame_host, int x, int y));
MOCK_METHOD0(GetId, std::string());
MOCK_METHOD0(GetParentId, std::string());
MOCK_METHOD0(GetOpenerId, std::string());
......
third_party/WebKit/Source/core/exported/WebDevToolsAgentImpl.cpp
View file @ 902bffab
......@@ -228,7 +228,6 @@ class WebDevToolsAgentImpl::Session : public GarbageCollectedFinalized<Session>,
void DispatchProtocolMessage(int call_id,
const String& method,
const String& message) override;
void InspectElement(const WebPoint& point_in_root_frame) override;
// InspectorSession::Client implementation.
void SendProtocolMessage(int session_id,
......@@ -299,8 +298,6 @@ class WebDevToolsAgentImpl::Session::IOSession
session_, call_id, method, message));
}
void InspectElement(const WebPoint&) override { NOTREACHED(); }
private:
scoped_refptr<base::SingleThreadTaskRunner> session_task_runner_;
scoped_refptr<WebTaskRunner> agent_task_runner_;
......@@ -359,35 +356,6 @@ void WebDevToolsAgentImpl::Session::DispatchProtocolMessage(
DispatchProtocolMessageInternal(call_id, method, message);
}
void WebDevToolsAgentImpl::Session::InspectElement(
const WebPoint& point_in_root_frame) {
WebPoint point = point_in_root_frame;
if (frame_->ViewImpl() && frame_->ViewImpl()->Client()) {
WebFloatRect rect(point.x, point.y, 0, 0);
frame_->ViewImpl()->Client()->ConvertWindowToViewport(&rect);
point = WebPoint(rect.x, rect.y);
}
HitTestRequest::HitTestRequestType hit_type =
HitTestRequest::kMove | HitTestRequest::kReadOnly |
HitTestRequest::kAllowChildFrameContent;
HitTestRequest request(hit_type);
WebMouseEvent dummy_event(WebInputEvent::kMouseDown,
WebInputEvent::kNoModifiers,
WTF::CurrentTimeTicksInMilliseconds());
dummy_event.SetPositionInWidget(point.x, point.y);
IntPoint transformed_point = FlooredIntPoint(
TransformWebMouseEvent(frame_->GetFrameView(), dummy_event)
.PositionInRootFrame());
HitTestResult result(
request, frame_->GetFrameView()->RootFrameToContents(transformed_point));
frame_->GetFrame()->ContentLayoutObject()->HitTest(result);
Node* node = result.InnerNode();
if (!node && frame_->GetFrame()->GetDocument())
node = frame_->GetFrame()->GetDocument()->documentElement();
overlay_agent_->Inspect(node);
}
void WebDevToolsAgentImpl::Session::SendProtocolMessage(int session_id,
int call_id,
const String& response,
......@@ -582,6 +550,7 @@ void WebDevToolsAgentImpl::Trace(blink::Visitor* visitor) {
visitor->Trace(resource_content_loader_);
visitor->Trace(inspected_frames_);
visitor->Trace(resource_container_);
visitor->Trace(node_to_inspect_);
}
void WebDevToolsAgentImpl::WillBeDestroyed() {
......@@ -604,13 +573,59 @@ void WebDevToolsAgentImpl::BindRequest(
void WebDevToolsAgentImpl::AttachDevToolsSession(
mojom::blink::DevToolsSessionHostAssociatedPtrInfo host,
mojom::blink::DevToolsSessionAssociatedRequest session,
mojom::blink::DevToolsSessionRequest io_session,
mojom::blink::DevToolsSessionAssociatedRequest session_request,
mojom::blink::DevToolsSessionRequest io_session_request,
const String& reattach_state) {
if (!sessions_.size())
Platform::Current()->CurrentThread()->AddTaskObserver(this);
sessions_.insert(new Session(this, std::move(host), std::move(session),
std::move(io_session), reattach_state));
Session* session =
new Session(this, std::move(host), std::move(session_request),
std::move(io_session_request), reattach_state);
sessions_.insert(session);
if (node_to_inspect_) {
session->overlay_agent()->Inspect(node_to_inspect_);
node_to_inspect_ = nullptr;
}
}
void WebDevToolsAgentImpl::InspectElement(const WebPoint& point_in_local_root) {
WebPoint point = point_in_local_root;
// TODO(dgozman): the ViewImpl() check must not be necessary,
// but it is required when attaching early to a provisional frame.
// We should clean this up once provisional frames are gone.
// See https://crbug.com/578349.
if (web_local_frame_impl_->ViewImpl() &&
web_local_frame_impl_->ViewImpl()->Client()) {
WebFloatRect rect(point.x, point.y, 0, 0);
web_local_frame_impl_->ViewImpl()->Client()->ConvertWindowToViewport(&rect);
point = WebPoint(rect.x, rect.y);
}
HitTestRequest::HitTestRequestType hit_type =
HitTestRequest::kMove | HitTestRequest::kReadOnly |
HitTestRequest::kAllowChildFrameContent;
HitTestRequest request(hit_type);
WebMouseEvent dummy_event(WebInputEvent::kMouseDown,
WebInputEvent::kNoModifiers,
WTF::CurrentTimeTicksInSeconds());
dummy_event.SetPositionInWidget(point.x, point.y);
IntPoint transformed_point = FlooredIntPoint(
TransformWebMouseEvent(web_local_frame_impl_->GetFrameView(), dummy_event)
.PositionInRootFrame());
HitTestResult result(
request, web_local_frame_impl_->GetFrameView()->RootFrameToContents(
transformed_point));
web_local_frame_impl_->GetFrame()->ContentLayoutObject()->HitTest(result);
Node* node = result.InnerNode();
if (!node && web_local_frame_impl_->GetFrame()->GetDocument())
node = web_local_frame_impl_->GetFrame()->GetDocument()->documentElement();
if (!sessions_.IsEmpty()) {
for (auto& session : sessions_)
session->overlay_agent()->Inspect(node);
} else {
node_to_inspect_ = node;
}
}
void WebDevToolsAgentImpl::DetachSession(Session* session) {
......
third_party/WebKit/Source/core/exported/WebDevToolsAgentImpl.h
View file @ 902bffab
......@@ -105,6 +105,7 @@ class CORE_EXPORT WebDevToolsAgentImpl final
mojom::blink::DevToolsSessionAssociatedRequest main_session,
mojom::blink::DevToolsSessionRequest io_session,
const String& reattach_state) override;
void InspectElement(const WebPoint& point_in_local_root) override;
// InspectorTracingAgent::Client implementation.
void ShowReloadingBlanket() override;
......@@ -130,6 +131,7 @@ class CORE_EXPORT WebDevToolsAgentImpl final
Member<InspectorResourceContentLoader> resource_content_loader_;
Member<InspectedFrames> inspected_frames_;
Member<InspectorResourceContainer> resource_container_;
Member<Node> node_to_inspect_;
bool include_view_agents_;
int layer_tree_id_;
};
......
third_party/WebKit/Source/devtools/front_end/Tests.js
View file @ 902bffab
......@@ -1106,6 +1106,15 @@
throw 'Some expected events are not found: ' + Array.from(expectedEvents.keys()).join(',');
};
TestSuite.prototype.testInspectedElementIs = async function(nodeName) {
this.takeControl();
await self.runtime.loadModulePromise('elements');
if (!Elements.ElementsPanel._firstInspectElementNodeNameForTest)
await new Promise(f => this.addSniffer(Elements.ElementsPanel, '_firstInspectElementCompletedForTest', f));
this.assertEquals(nodeName, Elements.ElementsPanel._firstInspectElementNodeNameForTest);
this.releaseControl();
};
/**
* Serializes array of uiSourceCodes to string.
* @param {!Array.<!Workspace.UISourceCode>} uiSourceCodes
......
third_party/WebKit/Source/devtools/front_end/elements/ElementsPanel.js
View file @ 902bffab
......@@ -707,8 +707,11 @@ Elements.ElementsPanel = class extends UI.Panel {
this.selectDOMNode(node, focus);
delete this._omitDefaultSelection;
if (!this._notFirstInspectElement)
if (!this._notFirstInspectElement) {
Elements.ElementsPanel._firstInspectElementNodeNameForTest = node.nodeName();
Elements.ElementsPanel._firstInspectElementCompletedForTest();
InspectorFrontendHost.inspectElementCompleted();
}
this._notFirstInspectElement = true;
});
}
......@@ -850,6 +853,9 @@ Elements.ElementsPanel._splitMode = {
Slim: Symbol('Slim'),
};
// Sniffed in tests.
Elements.ElementsPanel._firstInspectElementCompletedForTest = function() {};
/**
* @implements {UI.ContextMenu.Provider}
* @unrestricted
......
third_party/WebKit/public/web/devtools_agent.mojom
View file @ 902bffab
......@@ -32,6 +32,8 @@ struct DevToolsMessageChunk {
// the navigation commits in the frame.
//
// This interface is implemented in renderer hosting entity under debug.
// Note that this interface just provides a mean to start a debugging session,
// so the presence of it does not mean the entity is under debug just yet.
interface DevToolsAgent {
// Attaches a new debugging session. This session speaks remote debugging
// protocol and restores all the changes to original state once destroyed.
......@@ -67,19 +69,28 @@ interface DevToolsAgent {
associated DevToolsSession& session,
DevToolsSession& io_session,
string? reattach_state);
// Requests an element at specific position to be inspected in every
// attached session (or the next attached one if none yet).
//
// Note that inspecting element does not start/stop any debugging sessions
// by itself, and has no effect unless a debugging session is
// or will be attached.
InspectElement(gfx.mojom.Point point);
};
// Represents an attached session which exposes remote debugging protocol.
// This interface is implemented in renderer hosting entity under debug.
//
// Lifetime of the session exactly matches the debugging time span. In other
// words, the entity is under debug if and only if there is at least one
// session.
interface DevToolsSession {
// Dispatches protocol message from a client to a debugging target.
// |method| is a method name as defined in protocol (e.g. "Runtime.evaluate").
// |call_id| is a command id as defined in protocol, and is going to be
// reported back to host in a response message (see DevToolsMessageChunk).
DispatchProtocolMessage(int32 call_id, string method, string message);
// Requests an element at specific position to be inspected.
InspectElement(gfx.mojom.Point point);
};
// A peer of DevToolsSession representing a remote debugging client
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment