Summary: | Generate messages sent to the WebProcess class. | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Sam Weinig <sam> | ||||||
Component: | WebKit2 | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | abecsi, aroben, ossy, webkit-ews, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | PC | ||||||||
OS: | OS X 10.5 | ||||||||
Attachments: |
|
Description
Sam Weinig
2010-10-04 11:25:46 PDT
Created attachment 69651 [details]
Patch
Attachment 69651 [details] did not build on qt: Build output: http://queues.webkit.org/results/4146072 Comment on attachment 69651 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=69651&action=review > WebKit2/WebKit2.pro:180 > HEADERS += \ > Shared/CoreIPCSupport/DrawingAreaMessageKinds.h \ > Shared/CoreIPCSupport/DrawingAreaProxyMessageKinds.h \ > Shared/CoreIPCSupport/WebPageProxyMessageKinds.h \ > - Shared/CoreIPCSupport/WebProcessMessageKinds.h \ > Shared/CacheModel.h \ > Shared/DrawingAreaBase.h \ > Shared/ImmutableArray.h \ Seems like you need to add the new generated files. > WebKit2/Platform/CoreIPC/HandleMessage.h:48 > + // NOTE: Return value ignored. > + (object->*function)(firstArgument, secondArgument); > +} Why throw away the return value? Why not return it? > WebKit2/Scripts/webkit2/messages.py:232 > + if type.find("<") != -1: > + headers.update(headers_for_type(type)) > + continue Please use single quotes. Please add tests that show that your new code works. (And make sure you didn't break any existing tests.) > WebKit2/UIProcess/mac/WebProcessProxyMac.mm:42 > + send(Messages::WebProcess::SetupAcceleratedCompositingPort(CoreIPC::MachPort(renderServerPort, MACH_MSG_TYPE_COPY_SEND)), 0); Should be "SetUpAcceleratedCompositingPort", with a capital "U". > WebKit2/WebProcess/WebProcess.cpp:194 > -void WebProcess::setCacheModel(CacheModel cacheModel) > +void WebProcess::setCacheModel(uint32_t cacheModel) > { > - if (!m_hasSetCacheModel || cacheModel != m_cacheModel) { > + if (!m_hasSetCacheModel || static_cast<CacheModel>(cacheModel) != m_cacheModel) { > m_hasSetCacheModel = true; > - m_cacheModel = cacheModel; > - platformSetCacheModel(cacheModel); > + m_cacheModel = static_cast<CacheModel>(cacheModel); > + platformSetCacheModel(static_cast<CacheModel>(cacheModel)); > } > } Doing the cast once and storing the result in a local variable would be nicer. > WebKit2/win/WebKit2.vcproj:766 > > > </File> > <File > - RelativePath="..\Shared\CoreIPCSupport\WebProcessMessageKinds.h" > - > > - </File> > - <File > RelativePath="..\Shared\CoreIPCSupport\WebProcessProxyMessageKinds.h" > > > </File> > Seems like you need to add the new generated files. Created attachment 69660 [details]
Patch
Attachment 69660 [details] did not pass style-queue:
Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit2/Scripts/webkit2/messages.py:230: trailing whitespace [pep8/W291] [5]
Total errors found: 1 in 17 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Attachment 69660 [details] did not build on qt: Build output: http://queues.webkit.org/results/4177071 Comment on attachment 69660 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=69660&action=review > WebKit2/ChangeLog:13 > + * Scripts/webkit2/messages.py: > + * Scripts/webkit2/messages_unittest.py: > + Make this script work for passing a templated type. It would be nice to add the names of functions you changed in messages.py. > WebKit2/WebProcess/WebProcess.cpp:194 > -void WebProcess::setCacheModel(CacheModel cacheModel) > +void WebProcess::setCacheModel(uint32_t cacheModel) > { > - if (!m_hasSetCacheModel || cacheModel != m_cacheModel) { > + if (!m_hasSetCacheModel || static_cast<CacheModel>(cacheModel) != m_cacheModel) { > m_hasSetCacheModel = true; > - m_cacheModel = cacheModel; > - platformSetCacheModel(cacheModel); > + m_cacheModel = static_cast<CacheModel>(cacheModel); > + platformSetCacheModel(static_cast<CacheModel>(cacheModel)); > } > } I still think a local variable with type CacheModel would be better. (In reply to comment #8) > Landed in r69029. Hey, you shouldn't have commit it, because Qt EWS bubble is red. It wasn't a fair play ... (In reply to comment #9) > (In reply to comment #8) > > Landed in r69029. > > Hey, you shouldn't have commit it, because Qt EWS bubble is red. > It wasn't a fair play ... Sorry, but I couldn't figure out how to make it work on Qt. We have a DerivedSources.make file that the Qt build does not seem to be using. If adding stuff to the project is prohibitively hard, it is likely that things will break. > Sorry, but I couldn't figure out how to make it work on Qt. We have a DerivedSources.make file that the Qt build does not seem to be using. If adding stuff to the project is prohibitively hard, it is likely that things will break. I prefer poking our WebKit2 guys and ask them to fix Qt build system instead of breaking Qt buildbots and EWS. Next time, please cc Zoltan (zoltan@webkit.org), Balazs (kbalazs@webkit.org) and Andras (abecsi@webkit.org), they can help you with fixing Qt-WebKit2 build system. Fix landed in http://trac.webkit.org/changeset/69046 |