This is series of supplement-ification (Bug 78440). NotificationController and its client should be pushed out from Page class using PageSupplement.
CC-ed some folks just for FYI because it looks notification is under hot discussion. Note that this change never intents any functional change. This is purely code-level factoring.
Created attachment 128395 [details] Patch
Adam, Hakaken, could you give me a stamp? This will help future supplimentalization.
Comment on attachment 128395 [details] Patch Clearing flags on attachment: 128395 Committed r108615: <http://trac.webkit.org/changeset/108615>
All reviewed patches have been landed. Closing bug.
Reopen, because it broke Qt WK2 build: /home/webkitbuildbot/slaves/release32bit-qt5/buildslave/qt-linux-32-release-qt5/build/Source/WebKit2/WebProcess/WebPage/WebPage.cpp: In constructor ‘WebKit::WebPage::WebPage(uint64_t, const WebKit::WebPageCreationParameters&)’: /home/webkitbuildbot/slaves/release32bit-qt5/buildslave/qt-linux-32-release-qt5/build/Source/WebKit2/WebProcess/WebPage/WebPage.cpp:236: error: ‘struct WebCore::Page::PageClients’ has no member named ‘notificationClient’
Why was the qt EWS bot green? :( /me will see if morrita is around and if not I'll fix it.
(In reply to comment #7) > Why was the qt EWS bot green? :( Because the Qt EWS doesn't build WebKit2 :( I hope that we can change that in the future.
Committed r108621: <http://trac.webkit.org/changeset/108621>
(In reply to comment #8) > (In reply to comment #7) > > Why was the qt EWS bot green? :( > > Because the Qt EWS doesn't build WebKit2 :( > > I hope that we can change that in the future. The Qt EWS is same as "Qt Linux Release" bot on build.webkit.org. (Qt 4.8.0, x86 architecture) It is only a decision, we can do everything. :) Now all of our bots (except Qt Linux 64-bit Release (Perf)) on build.webkit.org uses the released Qt 4.8.0 because of historically reason. When we drop WebKit2 for Qt 4.8, Qt 5 wasn't so stable, we have build problems every day, etc. If we would like to switch the EWS to Qt 5, we should guarantee that QtWebKit with Qt5 with WebKit2 almost always builds. If the EWS doesn't work for a night, because nobody fix the build, it can cause much more trouble than we don't have WebKit2 EWS and somebody break the Qt-WK2 build once in a month. The best solution would be if we setup a Qt-WK2 EWS too and leave the Qt EWS build WebKit1 with Qt 4.8.
(In reply to comment #10) > (In reply to comment #8) > > (In reply to comment #7) > > > Why was the qt EWS bot green? :( > > > > Because the Qt EWS doesn't build WebKit2 :( > > > > I hope that we can change that in the future. > > The Qt EWS is same as "Qt Linux Release" bot on build.webkit.org. > (Qt 4.8.0, x86 architecture) > > It is only a decision, we can do everything. :) Now all of our bots (except Qt Linux 64-bit Release (Perf)) on build.webkit.org uses the released Qt 4.8.0 because of historically reason. When we drop WebKit2 for Qt 4.8, Qt 5 wasn't so stable, we have build problems every day, etc. > > If we would like to switch the EWS to Qt 5, we should guarantee that QtWebKit with Qt5 with WebKit2 almost always builds. If the EWS doesn't work for a night, because nobody fix the build, it can cause much more trouble than we don't have WebKit2 EWS and somebody break the Qt-WK2 build once in a month. > > The best solution would be if we setup a Qt-WK2 EWS too and leave the Qt EWS build WebKit1 with Qt 4.8. Right, either two EWSes or one Qt-EWS that does a build against Qt 4 and Qt 5 in one shot. I suppose the former is easier? :)
This broke the Mac build: Undefined symbols for architecture x86_64: "__ZN7WebCore19provideNotificationEPNS_4PageEPNS_21NotificationPresenterE", referenced from: ld: symbol(s) not found for architecture x86_64 I'm fixing it now.
(In reply to comment #12) > This broke the Mac build: > Undefined symbols for architecture x86_64: > "__ZN7WebCore19provideNotificationEPNS_4PageEPNS_21NotificationPresenterE", referenced from: > ld: symbol(s) not found for architecture x86_64 > > I'm fixing it now. Oops. Thank you for taking care of this.