Summary: | Web Inspector: Add InspectorFileSystemAgent::FrontendProvider | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Taiju Tsuiki <tzik> | ||||||||||||
Component: | Web Inspector (Deprecated) | Assignee: | Taiju Tsuiki <tzik> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | apavlov, bweinstein, joepeck, keishi, loislo, pfeldman, pmuellr, rik, timothy, webkit.review.bot, yurys | ||||||||||||
Priority: | P2 | ||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||
Hardware: | All | ||||||||||||||
OS: | All | ||||||||||||||
Bug Depends on: | |||||||||||||||
Bug Blocks: | 68203, 87724 | ||||||||||||||
Attachments: |
|
Description
Taiju Tsuiki
2012-05-28 01:30:15 PDT
Created attachment 144302 [details]
Patch
Created attachment 144307 [details]
Patch
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; Created attachment 144331 [details]
Patch
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! 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) { Created attachment 144337 [details]
Patch
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 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(...) , ... Created attachment 144451 [details]
Patch
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 Comment on attachment 144451 [details] Patch Clearing flags on attachment: 144451 Committed r118783: <http://trac.webkit.org/changeset/118783> All reviewed patches have been landed. Closing bug. |