[chromium] don't use WebString for WebPreferences
Created attachment 180311 [details] Patch
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Comment on attachment 180311 [details] Patch Are you sure this is a good idea? I think we should basically forbid using any code from svn.webkit.org in Chromium's browser process. It is too easy to introduce dependencies on WebString. How will you maintain the requirement that we not use WebString? Can we solve the core issue some other way?
(In reply to comment #3) > (From update of attachment 180311 [details]) > Are you sure this is a good idea? I think we should basically forbid using any code from svn.webkit.org in Chromium's browser process. **code** != enums and POD types :)
(In reply to comment #3) > (From update of attachment 180311 [details]) > Are you sure this is a good idea? I think we should basically forbid using any code from svn.webkit.org in Chromium's browser process. It is too easy to introduce dependencies on WebString. How will you maintain the requirement that we not use WebString? The reason this only breaks in single process mode is that in single process mode, we don't initialize WebKit in the browser process during early startup. Remember, there's in_process_webkit/... As soon as somebody adds a WebString on a codepath before the WebKit initialization, chrome crashes in debug mode, so it's rather easy to catch :) Anyway, this change turns WebPreferences into POD, so it would be safe to be used. > > Can we solve the core issue some other way? The core issue is that the browser process specifies the websettings before the renderer is started. I could also copy the default settings to content/shell and add some DCHECK()s to the renderer that the default settings match the ones from WebPreferences.
(In reply to comment #5) > (In reply to comment #3) > > (From update of attachment 180311 [details] [details]) > > Are you sure this is a good idea? I think we should basically forbid using any code from svn.webkit.org in Chromium's browser process. It is too easy to introduce dependencies on WebString. How will you maintain the requirement that we not use WebString? > > The reason this only breaks in single process mode is that in single process mode, we don't initialize WebKit in the browser process during early startup. Remember, there's in_process_webkit/... > > As soon as somebody adds a WebString on a codepath before the WebKit initialization, chrome crashes in debug mode, so it's rather easy to catch :) > > Anyway, this change turns WebPreferences into POD, so it would be safe to be used. It's not really POD if it has std::string :) My point is that this'll introduce land mines in WebKit, where you will have to magically know that WebString is forbidden. That doesn't scale, and it creates more entropy / maintenance burden. > > Can we solve the core issue some other way? > > The core issue is that the browser process specifies the websettings before the renderer is started. > > I could also copy the default settings to content/shell and add some DCHECK()s to the renderer that the default settings match the ones from WebPreferences. How about if we just expose the default settings as constants / literals?