Make Linux pass users' font settings through to renderer.

Make Linux pass users' font settings through to renderer.

The renderer doesn't give the settings to Skia yet -- that'll
go in after agl's work on hinting and subpixel rendering in Skia.

[brettw, 2009-07-20 20:10:32]

http://codereview.chromium.org/155787/diff/1/2
File chrome/browser/browser.cc (right):

http://codereview.chromium.org/155787/diff/1/2#newcode1907
Line 1907: tab_contents->delegate()->IsNormalWindow();
It seems like it might be possible to not add IsNormalWindow to the TCDelegate.
It looks like you know you're creating an application so you can set it here to
false.

http://codereview.chromium.org/155787/diff/1/5
File chrome/browser/renderer_host/render_view_host.h (right):

http://codereview.chromium.org/155787/diff/1/5#newcode106
Line 106: // Send the renderer process its preferences from the delegate.
Can you clarify "from the delegate" here? Like "the current preferences supplied
by the RVHDelegate."

http://codereview.chromium.org/155787/diff/1/6
File chrome/browser/tab_contents/tab_contents.cc (right):

http://codereview.chromium.org/155787/diff/1/6#newcode1058
Line 1058: void TabContents::InitRendererPrefs() {
Do you think this function might make sense on the TCView? Then we wouldn't need
the ifdef and the include stuff above. It seems like the current functions are
arguably view-related as well.

[use derat at chromium.org, 2009-07-20 21:12:39]

Thanks, I've applied your feedback.

Adam, can you take another look as well?  I fixed some embarrassing
issues (a segfault if GtkSettings returns NULL strings because
gnome-settings-daemon isn't running, and a memory leak caused by me
not freeing the strings). :-/

On Mon, Jul 20, 2009 at 8:10 PM, <brettw@chromium.org> wrote:
>
> http://codereview.chromium.org/155787/diff/1/2
> File chrome/browser/browser.cc (right):
>
> http://codereview.chromium.org/155787/diff/1/2#newcode1907
> Line 1907: tab_contents->delegate()->IsNormalWindow();
> It seems like it might be possible to not add IsNormalWindow to the
> TCDelegate. It looks like you know you're creating an application so you
> can set it here to false.
>
> http://codereview.chromium.org/155787/diff/1/5
> File chrome/browser/renderer_host/render_view_host.h (right):
>
> http://codereview.chromium.org/155787/diff/1/5#newcode106
> Line 106: // Send the renderer process its preferences from the
> delegate.
> Can you clarify "from the delegate" here? Like "the current preferences
> supplied by the RVHDelegate."
>
> http://codereview.chromium.org/155787/diff/1/6
> File chrome/browser/tab_contents/tab_contents.cc (right):
>
> http://codereview.chromium.org/155787/diff/1/6#newcode1058
> Line 1058: void TabContents::InitRendererPrefs() {
> Do you think this function might make sense on the TCView? Then we
> wouldn't need the ifdef and the include stuff above. It seems like the
> current functions are arguably view-related as well.
>
> http://codereview.chromium.org/155787
>

[agl, 2009-07-20 21:49:28]

On 2009/07/20 21:12:39, derat wrote:
> Adam, can you take another look as well?

LGTM. Any such crashes etc will be weeded out pretty quickly in the tree.

[brettw, 2009-07-20 22:03:07]

LGTM with name change.

http://codereview.chromium.org/155787/diff/15/16
File chrome/browser/browser.cc (right):

http://codereview.chromium.org/155787/diff/15/16#newcode1902
Line 1902: tab_contents->render_view_host()->SetRendererPrefs();
This name feels wrong to me. Can you call it SyncRendererPrefs instead?