| Summary: | Modern IDB (Workers): Get the IDBConnectionProxy from the Document to the WorkerGlobalScope | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Brady Eidson <beidson> | ||||
| Component: | WebCore Misc. | Assignee: | Brady Eidson <beidson> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | achristensen, alecflett, bfulgham, commit-queue, jsbell, ossy, thorton | ||||
| Priority: | P2 | ||||||
| Version: | WebKit Nightly Build | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Bug Depends on: | |||||||
| Bug Blocks: | 149117, 149953 | ||||||
| Attachments: |
|
||||||
|
Description
Brady Eidson
2016-04-21 16:15:01 PDT
Created attachment 276981 [details]
Patch v1
Comment on attachment 276981 [details] Patch v1 Clearing flags on attachment: 276981 Committed r199853: <http://trac.webkit.org/changeset/199853> All reviewed patches have been landed. Closing bug. (In reply to comment #2) > Comment on attachment 276981 [details] > Patch v1 > > Clearing flags on attachment: 276981 > > Committed r199853: <http://trac.webkit.org/changeset/199853> It broke the Apple Mac cmake build: /Volumes/Data/slave/elcapitan-cmake-debug/build/Source/WebCore/workers/WorkerGlobalScope.cpp:66:219: warning: unused parameter 'connectionProxy' [-Wunused-parameter] WorkerGlobalScope::WorkerGlobalScope(const URL& url, const String& userAgent, WorkerThread& thread, bool shouldBypassMainWorldContentSecurityPolicy, PassRefPtr<SecurityOrigin> topOrigin, IDBClient::IDBConnectionProxy* connectionProxy) ^ /Volumes/Data/slave/elcapitan-cmake-debug/build/Source/WebCore/workers/WorkerMessagingProxy.cpp:81:246: error: no member named 'idbConnectionProxy' in 'WebCore::Document' RefPtr<DedicatedWorkerThread> thread = DedicatedWorkerThread::create(scriptURL, userAgent, sourceCode, *this, *this, startMode, contentSecurityPolicyResponseHeaders, shouldBypassMainWorldContentSecurityPolicy, document.topOrigin(), document.idbConnectionProxy()); ~~~~~~~~ ^ /Volumes/Data/slave/elcapitan-cmake-debug/build/Source/WebCore/workers/WorkerThread.cpp:96:411: warning: unused parameter 'connectionProxy' [-Wunused-parameter] WorkerThread::WorkerThread(const URL& scriptURL, const String& userAgent, const String& sourceCode, WorkerLoaderProxy& workerLoaderProxy, WorkerReportingProxy& workerReportingProxy, WorkerThreadStartMode startMode, const ContentSecurityPolicyResponseHeaders& contentSecurityPolicyResponseHeaders, bool shouldBypassMainWorldContentSecurityPolicy, const SecurityOrigin* topOrigin, IDBClient::IDBConnectionProxy* connectionProxy) ^ 1 error generated. make[2]: *** [Source/WebCore/CMakeFiles/WebCore.dir/workers/WorkerMessagingProxy.cpp.o] Error 1 (In reply to comment #3) > All reviewed patches have been landed. Closing bug. And it broke the Apple Windows build too: C:\cygwin\home\buildbot\slave\win-release\build\Source\WebCore\workers\WorkerMessagingProxy.cpp(81): error C2039: 'idbConnectionProxy': is not a member of 'WebCore::Document' [C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\Release\Source\WebCore\WebCore.vcxproj] Comment on attachment 276981 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=276981&action=review > Source/WebCore/workers/WorkerMessagingProxy.cpp:81 > + RefPtr<DedicatedWorkerThread> thread = DedicatedWorkerThread::create(scriptURL, userAgent, sourceCode, *this, *this, startMode, contentSecurityPolicyResponseHeaders, shouldBypassMainWorldContentSecurityPolicy, document.topOrigin(), document.idbConnectionProxy()); Using document.idbConnectionProxy() is incorrect here for !ENABLE(INDEXED_DATABASE) ports: http://trac.webkit.org/browser/trunk/Source/WebCore/dom/Document.h#L659 (In reply to comment #4) > (In reply to comment #2) > > Comment on attachment 276981 [details] > > Patch v1 > > > > Clearing flags on attachment: 276981 > > > > Committed r199853: <http://trac.webkit.org/changeset/199853> > > It broke the Apple Mac cmake build: Once keeping that build working is a directive of the project and not just the side project of one engineer, we'll have bots that will catch such a thing.(In reply to comment #5) Since we don't even have EWS for it, it's definitely allowed to break. > And it broke the Apple Windows build too: This was certainly sad. I've said this about a million times, but will say it again: "Automation is helpful" seems to be the motto of the project, but *imperfect* automation is harmful. If we're going to rely on EWS and CQ, then CQ can't land something that an EWS would catch. Anyways, this was fixed earlier today in http://trac.webkit.org/changeset/199885 (In reply to comment #7) > (In reply to comment #4) > > (In reply to comment #2) > > > Comment on attachment 276981 [details] > > > Patch v1 > > > > > > Clearing flags on attachment: 276981 > > > > > > Committed r199853: <http://trac.webkit.org/changeset/199853> > > > > It broke the Apple Mac cmake build: > > Once keeping that build working is a directive of the project and not just > the side project of one engineer, we'll have bots that will catch such a > thing.(In reply to comment #5) > > Since we don't even have EWS for it, it's definitely allowed to break. I didn't say the opposite of it. Of course, you are allowed to break it whenever you want and you don't have to fix it if you don't want. I just noticed the build break to let you and Alex know that the Apple Mac cmake is broken due to this change. > > And it broke the Apple Windows build too: > > This was certainly sad. I've said this about a million times, but will say > it again: > "Automation is helpful" seems to be the motto of the project, but > *imperfect* automation is harmful. > > If we're going to rely on EWS and CQ, then CQ can't land something that an > EWS would catch. If you check the build.webkit.org waterfall, you can see that the automation was perfect here, but an other Apple employee break the Windows build before your patch, that's why the EWS wasn't able to build the trunk at all that time. > Anyways, this was fixed earlier today in > http://trac.webkit.org/changeset/199885 Thanks for the fix. (In reply to comment #8) > (In reply to comment #7) > > (In reply to comment #4) > > > And it broke the Apple Windows build too: > > > > This was certainly sad. I've said this about a million times, but will say > > it again: > > "Automation is helpful" seems to be the motto of the project, but > > *imperfect* automation is harmful. > > > > If we're going to rely on EWS and CQ, then CQ can't land something that an > > EWS would catch. > > If you check the build.webkit.org waterfall, you can see that the automation > was perfect here, but an other Apple employee break the Windows build before > your patch, that's why the EWS wasn't able to build the trunk at all that > time. The "Perfect Automation" argument is that: 1 - The automated tools have to do 100% of what we need them to do, correctly. 2 - Nobody can land anything manually with using the tools If somebody landed something with CQ that broke the Windows build ahead of time, that's a breakdown in automation because #1 was violated. If somebody landed something *manually* to break the Windows build ahead of time, that's a breakdown in automation because #2 was violated. </soapbox> |