Try to open the inspector in MiniBrowser and see that it's not rendered at all.
Let me pick this up.
Created attachment 271640 [details] Patch
Comment on attachment 271640 [details] Patch Thanks for the patch! I think it would be better to do this in a single place then, so I think we can move this to WebPreferences::platformInitializeStore() since it will be common, and remove it from _WebKitSettingsPrivate().
Created attachment 271658 [details] Patch
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment on attachment 271658 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=271658&action=review > Source/WebKit2/WebProcess/WebPage/DrawingAreaImpl.cpp:75 > if (webPage.corePage()->settings().acceleratedDrawingEnabled() || webPage.corePage()->settings().forceCompositingMode()) Or else we can move whole lines in this constructor into DrawingAreaImpl::updatePreferences(). What do you think?
Comment on attachment 271658 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=271658&action=review > Source/WebKit2/WebProcess/WebPage/DrawingAreaImpl.cpp:73 > - > #if USE(COORDINATED_GRAPHICS_THREADED) > - webPage.corePage()->settings().setForceCompositingMode(true); > + webPage.corePage()->settings().setForceCompositingMode(parameters.store.getBoolValueForKey(WebPreferencesKey::forceCompositingModeKey())); > #endif I think we don't even need this anymore. The drawing area is created by WebPage in its constructor and after that it calls updatePreferences that ends up calling DrawinArea::updatePreferences that already sets this setting.
Comment on attachment 271658 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=271658&action=review >> Source/WebKit2/WebProcess/WebPage/DrawingAreaImpl.cpp:75 >> if (webPage.corePage()->settings().acceleratedDrawingEnabled() || webPage.corePage()->settings().forceCompositingMode()) > > Or else we can move whole lines in this constructor into DrawingAreaImpl::updatePreferences(). What do you think? Actually, I don't see how this can work, because this happens before settings have been set in WebPage.
Comment on attachment 271658 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=271658&action=review >> Source/WebKit2/WebProcess/WebPage/DrawingAreaImpl.cpp:73 >> #endif > > I think we don't even need this anymore. The drawing area is created by WebPage in its constructor and after that it calls updatePreferences that ends up calling DrawinArea::updatePreferences that already sets this setting. Why this line is needed here is because of following lines. If we just remove this line only over leaving below lines. We will see a crash just after launching the mini browser. So I asked how about moving all lines in the constructor into DrawingAreaImpl::updatePreferences(). =)
(In reply to comment #9) > Comment on attachment 271658 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=271658&action=review > > >> Source/WebKit2/WebProcess/WebPage/DrawingAreaImpl.cpp:73 > >> #endif > > > > I think we don't even need this anymore. The drawing area is created by WebPage in its constructor and after that it calls updatePreferences that ends up calling DrawinArea::updatePreferences that already sets this setting. > > Why this line is needed here is because of following lines. If we just > remove this line only over leaving below lines. We will see a crash just > after launching the mini browser. So I asked how about moving all lines in > the constructor into DrawingAreaImpl::updatePreferences(). =) Sounds good to me, I guess we would also need to enter AC mdoe only when there's no layer tree host yet
Comment on attachment 271658 [details] Patch Attachment 271658 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/849874 New failing tests: storage/indexeddb/odd-strings-private.html
Created attachment 271660 [details] Archive of layout-test-results from ews116 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-yosemite Platform: Mac OS X 10.10.5
Comment on attachment 271658 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=271658&action=review >>>> Source/WebKit2/WebProcess/WebPage/DrawingAreaImpl.cpp:73 >>>> #endif >>> >>> I think we don't even need this anymore. The drawing area is created by WebPage in its constructor and after that it calls updatePreferences that ends up calling DrawinArea::updatePreferences that already sets this setting. >> >> Why this line is needed here is because of following lines. If we just remove this line only over leaving below lines. We will see a crash just after launching the mini browser. So I asked how about moving all lines in the constructor into DrawingAreaImpl::updatePreferences(). =) > > Sounds good to me, I guess we would also need to enter AC mdoe only when there's no layer tree host yet I found another issue after moving the lines into DrawingAreaImpl::updatePreferences(). It messes up rendering when not using threaded compositor. While webPage.corePage()->settings().acceleratedDrawingEnabled() is false in the constructor, it is true in DrawingAreaImpl::updatePreferences. So it results in making m_alwaysUseCompositing true and calling enterAcceleratedCompositingMode(0) even when not using threaded compositor. This is because the default value of the acceleratedDrawingEnabled of WebPreference is true so that it changes settings().acceleratedDrawingEnabled() in WebPage::updatePreferences. I assume this is not intended. When not using threaded compositor, m_alwaysUseCompositing should be false, right? If so, we move the lines into DrawingAreaImpl::updatePreferences, but with a guard COORDINATED_GRAPHICS_THREADED. Or we can leave it as is. Any idea?
(In reply to comment #13) > Comment on attachment 271658 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=271658&action=review > > >>>> Source/WebKit2/WebProcess/WebPage/DrawingAreaImpl.cpp:73 > >>>> #endif > >>> > >>> I think we don't even need this anymore. The drawing area is created by WebPage in its constructor and after that it calls updatePreferences that ends up calling DrawinArea::updatePreferences that already sets this setting. > >> > >> Why this line is needed here is because of following lines. If we just remove this line only over leaving below lines. We will see a crash just after launching the mini browser. So I asked how about moving all lines in the constructor into DrawingAreaImpl::updatePreferences(). =) > > > > Sounds good to me, I guess we would also need to enter AC mdoe only when there's no layer tree host yet > > I found another issue after moving the lines into > DrawingAreaImpl::updatePreferences(). It messes up rendering when not using > threaded compositor. While > webPage.corePage()->settings().acceleratedDrawingEnabled() is false in the > constructor, it is true in DrawingAreaImpl::updatePreferences. So it results > in making m_alwaysUseCompositing true and calling > enterAcceleratedCompositingMode(0) even when not using threaded compositor. > This is because the default value of the acceleratedDrawingEnabled of > WebPreference is true so that it changes > settings().acceleratedDrawingEnabled() in WebPage::updatePreferences. I > assume this is not intended. When not using threaded compositor, > m_alwaysUseCompositing should be false, right? If so, we move the lines into > DrawingAreaImpl::updatePreferences, but with a guard > COORDINATED_GRAPHICS_THREADED. Or we can leave it as is. Any idea? It seems to me that the check has always been wrong and it actually meant if AC enabled *and* force compositing not *or*, but because it was done before settings were updated, it always used the default values. We don't want to force compositing always when AC is enabled. Maybe we can split this patch since we are fixing too many things in the end :-)
(In reply to comment #14) > (In reply to comment #13) > > Comment on attachment 271658 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=271658&action=review > > > > >>>> Source/WebKit2/WebProcess/WebPage/DrawingAreaImpl.cpp:73 > > >>>> #endif > > >>> > > >>> I think we don't even need this anymore. The drawing area is created by WebPage in its constructor and after that it calls updatePreferences that ends up calling DrawinArea::updatePreferences that already sets this setting. > > >> > > >> Why this line is needed here is because of following lines. If we just remove this line only over leaving below lines. We will see a crash just after launching the mini browser. So I asked how about moving all lines in the constructor into DrawingAreaImpl::updatePreferences(). =) > > > > > > Sounds good to me, I guess we would also need to enter AC mdoe only when there's no layer tree host yet > > > > I found another issue after moving the lines into > > DrawingAreaImpl::updatePreferences(). It messes up rendering when not using > > threaded compositor. While > > webPage.corePage()->settings().acceleratedDrawingEnabled() is false in the > > constructor, it is true in DrawingAreaImpl::updatePreferences. So it results > > in making m_alwaysUseCompositing true and calling > > enterAcceleratedCompositingMode(0) even when not using threaded compositor. > > This is because the default value of the acceleratedDrawingEnabled of > > WebPreference is true so that it changes > > settings().acceleratedDrawingEnabled() in WebPage::updatePreferences. I > > assume this is not intended. When not using threaded compositor, > > m_alwaysUseCompositing should be false, right? If so, we move the lines into > > DrawingAreaImpl::updatePreferences, but with a guard > > COORDINATED_GRAPHICS_THREADED. Or we can leave it as is. Any idea? > > It seems to me that the check has always been wrong and it actually meant if > AC enabled *and* force compositing not *or*, but because it was done before > settings were updated, it always used the default values. We don't want to > force compositing always when AC is enabled. Maybe we can split this patch > since we are fixing too many things in the end :-) You are right. Let's go step by step.
Created attachment 271662 [details] Patch
Comment on attachment 271662 [details] Patch Clearing flags on attachment: 271662 Committed r196767: <http://trac.webkit.org/changeset/196767>
All reviewed patches have been landed. Closing bug.