If you bring up the Web Inspector, and then drag a URL into it, the page located at the URL will be loaded inside the inspector.
<rdar://problem/5283620>
<rdar://problem/5712808>
Created attachment 19217 [details] implement drag destination delegate Two questions: - On Windows, do I actually need to implement the ref counting or does the Drosera approach work here as well (that code & comments are just copied and pasted from Drosera) - I don't think I do, but I'm not 100% sure. - Which radar is the right one to put in the ChangeLogs?
Comment on attachment 19217 [details] implement drag destination delegate Looks good. + * ChangeLog: Should not be in there. +#ifndef WebInspectorBaseDelegate_H Should be _h, not _H. + // COM ref-counting isn't useful to us because we're in charge of the lifetime of the WebView. Are you sure this is OK? r=me
(In reply to comment #3) > - On Windows, do I actually need to implement the ref counting or does the > Drosera approach work here as well (that code & comments are just copied and > pasted from Drosera) - I don't think I do, but I'm not 100% sure. The WebInspectorClient must not be deleted before a) inspectorDestroyed() is called, and b) the last reference to it is released. Currently we delete the WebInspectorClient within inspectorDestroyed(). There's nothing in this patch that guarantees that the last reference has been released by then, so I think we're playing with fire here. I think the simplest solution is to make a separate object that functions as the UI delegate for this new WebView.
Comment on attachment 19217 [details] implement drag destination delegate Clearing review flag based on Adam's comment
Created attachment 19369 [details] revised patch Revised to address Adam's comments
Forgot to mention in the mac ChangeLog that removing this line: 166 [preferences setLoadsImagesAutomatically:YES]; is removing a dupe setting, not removing that setting :)
We should really do this with a policy delegate too to prevent loading any page that isn't the inspector.html file.
Comment on attachment 19369 [details] revised patch + , m_delegate(new WebInspectorDelegate()) I don't think there's any need for the WebInspectorClient to hold onto the delegate. +class WebInspectorDelegate : public IWebUIDelegate { +public: + WebInspectorDelegate(); Our COM classes normally have a private constructor/destructor and a static createInstance() method that returns a new instance with a refcount of 1. You should make this class follow that pattern. Other than those comments, this looks good to me. r- so we can get those things fixed up.
Created attachment 19407 [details] revised patch round 2
Comment on attachment 19407 [details] revised patch round 2 + if (FAILED(m_webView->setUIDelegate(WebInspectorDelegate::createInstance()))) + return 0; I think this is now leaking the delegate. On Windows, WebView retains its delegates (this doesn't match the Mac model, and we may change it someday). You should be able to fix the leak by doing this: m_webView->setUIDelegate(COMPtr<WebInspectorDelegate>(AdoptCOM, WebInspectorDelegate::createInstance()).get()) +WebInspectorDelegate::WebInspectorDelegate() + :m_refCount(1) +{ +} + +WebInspectorDelegate* WebInspectorDelegate::createInstance() +{ + WebInspectorDelegate* instance = new WebInspectorDelegate(); + return instance; +} Normally our COM classes start out with a refcount of 0 and then we call AddRef inside the createInstance method. You can also just say "new WebInspectorDelegate" without the parentheses. r=me if you fix the issues above.
Landed in <http://trac.webkit.org/projects/webkit/changeset/30631> with the changes Adam mentioned.
(In reply to comment #9) > We should really do this with a policy delegate too to prevent loading any page > that isn't the inspector.html file. > Filed bug 17577.