[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 #29 from Brian Burg <burg at cs.washington.edu>  2013-05-10 13:25:44 PST ---
(In reply to comment #27)
> (From update of attachment 201268 [details])
> 
> > Source/WebKit2/UIProcess/WebInspectorProxy.h:169
> > +    void platformSave(const String& filename, const String& content, bool forceSaveAs);
> > +    void platformAppend(const String& filename, const String& content);
> 
> I find it intrinsically weird to pass any file content as String. I am not aware of any API that works like that.

The use case is that the Inspector serializes collected runtime data (ie., Timeline profiles) into a JSON string and it's written to disk by InspectorFrontendHost. The API is not used for saving any other type of data currently.

I guess it could be encoded earlier and passed around as a blob behind the scenes, at least on WK2. Is there a preferred way to do that? CoreIPC::DataReference? Or going further, should we push blob construction down into the inspector frontend?

> > Source/WebKit2/UIProcess/WebInspectorProxy.h:179
> > +    void canSave(bool&);
> 
> Where is this defined?

Oops- left over from previous design. Will remove.

> > Source/WebKit2/UIProcess/mac/WebInspectorProxyMac.mm:515
> > +    // Necessary for the block below.
> > +    String suggestedURLCopy = suggestedURL;
> > +    String contentCopy = content;
> > +
> > +    auto saveToURL = ^(NSURL *actualURL) {
> > +        ASSERT(actualURL);
> > +        
> > +        m_suggestedToActualURLMap.set(suggestedURLCopy, actualURL);
> > +        [contentCopy writeToURL:actualURL atomically:YES encoding:NSUTF8StringEncoding error:NULL];
> > +        m_page->process()->send(Messages::WebInspector::DidSave([actualURL absoluteString]), m_page->pageID());
> > +    };
> 
> Please reduce the use span for this, move that code closer to where it is used: after if (!platformURL).

OK

> > Source/WebKit2/UIProcess/mac/WebInspectorProxyMac.mm:550
> > +    // do not append unless the user has already confirmed this filename in save().
> 
> do -> Do

OK

> > Source/WebKit2/WebProcess/WebPage/WebInspector.h:100
> > +    // Implemented in platform WebInspector file
> 
> Missing period.
> 
> That comment is useless really.

I agree. Will remove.

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