RESOLVED FIXED 87635
Web Inspector: Add InspectorFileSystemAgent::FrontendProvider 
https://bugs.webkit.org/show_bug.cgi?id=87635
Summary Web Inspector: Add InspectorFileSystemAgent::FrontendProvider
Taiju Tsuiki
Reported 2012-05-28 01:30:15 PDT
InspectorFileSystemAgent needs weak reference to its frontend to perform asynchronous operation.
Attachments
Patch (5.71 KB, patch)
2012-05-28 01:33 PDT, Taiju Tsuiki 		no flags
DetailsFormatted DiffDiff
Patch (5.69 KB, patch)
2012-05-28 01:56 PDT, Taiju Tsuiki 		no flags
DetailsFormatted DiffDiff
Patch (4.84 KB, patch)
2012-05-28 04:25 PDT, Taiju Tsuiki 		no flags
DetailsFormatted DiffDiff
Patch (4.82 KB, patch)
2012-05-28 05:21 PDT, Taiju Tsuiki 		no flags
DetailsFormatted DiffDiff
Patch (5.01 KB, patch)
2012-05-28 23:12 PDT, Taiju Tsuiki 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Taiju Tsuiki
Comment 1 2012-05-28 01:33:20 PDT
Created attachment 144302 [details] Patch
Taiju Tsuiki
Comment 2 2012-05-28 01:56:48 PDT
Created attachment 144307 [details] Patch
Yury Semikhatsky
Comment 3 2012-05-28 03:32:19 PDT
Comment on attachment 144307 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144307&action=review > Source/WebCore/inspector/InspectorFileSystemAgent.cpp:54 > + InspectorFrontend::FileSystem* getFrontend() const style nit: we don't use get prefixes in WebKit, simply frontend() > Source/WebCore/inspector/InspectorFileSystemAgent.cpp:102 > + m_frontendProvider->enable(); You could store a pointer to the agent itself, that way you wouldn't need to propagate enable/disable calls to the FrontendProvider, the getFrontend() method would look like if (m_agent && m_agent->m_enabled) return m_agent->mfrontend; return 0;
Taiju Tsuiki
Comment 4 2012-05-28 04:25:44 PDT
Created attachment 144331 [details] Patch
Taiju Tsuiki
Comment 5 2012-05-28 04:28:10 PDT
Comment on attachment 144307 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144307&action=review >> Source/WebCore/inspector/InspectorFileSystemAgent.cpp:54 >> + InspectorFrontend::FileSystem* getFrontend() const > > style nit: we don't use get prefixes in WebKit, simply frontend() Done >> Source/WebCore/inspector/InspectorFileSystemAgent.cpp:102 >> + m_frontendProvider->enable(); > > You could store a pointer to the agent itself, that way you wouldn't need to propagate enable/disable calls to the FrontendProvider, the getFrontend() method would look like > > if (m_agent && m_agent->m_enabled) > return m_agent->mfrontend; > return 0; Done Looks cleaner. Thanks!
Yury Semikhatsky
Comment 6 2012-05-28 05:13:59 PDT
Comment on attachment 144331 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144331&action=review > Source/WebCore/inspector/InspectorFileSystemAgent.cpp:82 > + if (m_frontendProvider.get()) No need for .get(), if (m_frontendProvider) should work > Source/WebCore/inspector/InspectorFileSystemAgent.cpp:111 > + if (m_frontendProvider.get()) { if (m_frontendProvider.get()) { -> if (m_frontendProvider) {
Taiju Tsuiki
Comment 7 2012-05-28 05:21:27 PDT
Created attachment 144337 [details] Patch
Taiju Tsuiki
Comment 8 2012-05-28 05:22:25 PDT
Comment on attachment 144331 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144331&action=review >> Source/WebCore/inspector/InspectorFileSystemAgent.cpp:82 >> + if (m_frontendProvider.get()) > > No need for .get(), if (m_frontendProvider) should work Done >> Source/WebCore/inspector/InspectorFileSystemAgent.cpp:111 >> + if (m_frontendProvider.get()) { > > if (m_frontendProvider.get()) { -> if (m_frontendProvider) { Done
Kinuko Yasuda
Comment 9 2012-05-28 21:44:57 PDT
Comment on attachment 144337 [details] Patch Drive-by comments. View in context: https://bugs.webkit.org/attachment.cgi?id=144337&action=review > Source/WebCore/inspector/InspectorFileSystemAgent.cpp:68 > + FrontendProvider(InspectorFileSystemAgent* agent, InspectorFrontend::FileSystem* frontend) : m_agent(agent), m_frontend(frontend) { } style-nit: each initializer should be on a separate line > Source/WebCore/inspector/InspectorFileSystemAgent.cpp:125 > : InspectorBaseAgent<InspectorFileSystemAgent>("FileSystem", instrumentingAgents, state), style-nit: constructor style looks different from the style guide? MyClass::MyClass(...) : m_member1(...) , m_member2(...) , ...
Taiju Tsuiki
Comment 10 2012-05-28 23:12:55 PDT
Created attachment 144451 [details] Patch
Taiju Tsuiki
Comment 11 2012-05-28 23:15:29 PDT
Comment on attachment 144337 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=144337&action=review >> Source/WebCore/inspector/InspectorFileSystemAgent.cpp:68 >> + FrontendProvider(InspectorFileSystemAgent* agent, InspectorFrontend::FileSystem* frontend) : m_agent(agent), m_frontend(frontend) { } > > style-nit: each initializer should be on a separate line Done >> Source/WebCore/inspector/InspectorFileSystemAgent.cpp:125 >> : InspectorBaseAgent<InspectorFileSystemAgent>("FileSystem", instrumentingAgents, state), > > style-nit: constructor style looks different from the style guide? > > MyClass::MyClass(...) > : m_member1(...) > , m_member2(...) > , ... Done
WebKit Review Bot
Comment 12 2012-05-29 09:00:51 PDT
Comment on attachment 144451 [details] Patch Clearing flags on attachment: 144451 Committed r118783: <http://trac.webkit.org/changeset/118783>
WebKit Review Bot
Comment 13 2012-05-29 09:00:56 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.