| Summary: | Web Inspector: InspectorController is difficult to get to on the Page | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | James Robinson <jamesr> | ||||
| Component: | Web Inspector | Assignee: | Nobody <webkit-unassigned> | ||||
| Status: | RESOLVED WONTFIX | ||||||
| Severity: | Normal | CC: | ap, bburg, graouts, joepeck, timothy, webkit-bug-importer | ||||
| Priority: | P2 | Keywords: | InRadar | ||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| Attachments: |
|
||||||
Created attachment 29150 [details]
Adds an inspectorController() const getter on Frame to clean up callsites
This patch adds Frame::inspectorController() const, which simply reaches up to the m_page for the InspectorController*.
Comment on attachment 29150 [details]
Adds an inspectorController() const getter on Frame to clean up callsites
This looks fine. I assume that Page::inspectorController() can never be null?
Also, you should consider adding a comment to Frame::inspectorController() in the header, noting that it can return 0.
Otherwise looks good. r+. If you don't have commit bit, please upload a new patch if you act on the above comments (or need to make other changes).
Comment on attachment 29150 [details]
Adds an inspectorController() const getter on Frame to clean up callsites
Since I don't think James has commit bit, I shouldn't really have marked this r+.
James, do you intend to follow up on this? |
The InspectorController object is accessed through the Page object, which is not trivial to get to from all places that want to since they sometimes have to walk through a number of objects just to get up that high. Here's an example from ContextMenu.cpp: Node* node = m_hitTestResult.innerNonSharedNode(); if (!node) return; Frame* frame = node->document()->frame(); if (!frame) return; Page* page = frame->page(); if (!page) return; if (!page->inspectorController()) return;