[Webkit-unassigned] [Bug 115564] Web Inspector: Implement WK2 version of WebInspectorFrontendClient::save

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=115564





--- Comment #13 from Timothy Hatcher <timothy at apple.com>  2013-05-07 18:04:53 PST ---
(From update of attachment 201005)
View in context: https://bugs.webkit.org/attachment.cgi?id=201005&action=review

Needs reviewed by a WebKit2 owner.

> Source/WebKit2/UIProcess/WebInspectorProxy.h:229
> +    HashMap<String, RetainPtr<NSURL>> m_saveURLs;  

m_savedURLs? m_savedURLToFileURLMap?

> Source/WebKit2/UIProcess/mac/WebInspectorProxyMac.mm:508
> +    String url = refURL;

Having both url and URL is confusing (and violates style guidelines). Are there more descriptive names you could use?

> Source/WebKit2/UIProcess/mac/WebInspectorProxyMac.mm:515
> +        m_page->process()->send(Messages::WebInspector::DidSave([URL absoluteString]),
> +                                m_page->pageID());

I wouldn't wrap this across two lines.

> Source/WebKit2/UIProcess/mac/WebInspectorProxyMac.mm:521
> +        URL = [NSURL URLWithString:url];

We should ASSERT and return early if the NSURL is nil after this.

> Source/WebKit2/UIProcess/mac/WebInspectorProxyMac.mm:543
> +        URL = [NSURL URLWithString:url];

Ditto.

> Source/WebKit2/WebProcess/WebPage/WebInspector.cpp:128
> +    bool result;
> +    WebProcess::shared().connection()->sendSync(Messages::WebInspectorProxy::CanSave(), result, m_page->pageID());
> +    return result;

sendSync == bad*. We should avoid it or cache it after the first time, since the UIProcess returns a static true or false.

* Frowned upon.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.