Create a valid RendererPreferences for notification windows.

Create a valid RendererPreferences for notification windows.

Essentially the same fix as http://crrev.com/44709.

BUG=140177
TEST=Select text in a notification and verify it to be highlighted.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=150236

[kinaba, 2012-08-02 05:51:54]

erg, can you take a look?
This should be the same fix as your old patch r44709, which seems to be missed
out in r109468.

stevenjb: I'd like an owner's stamp :).

[stevenjb, 2012-08-02 17:17:34]

owner rubber-stamp lgtm, but this should definitely be reviewed by someone who
understands the change :)

http://codereview.chromium.org/10825150/diff/1/chrome/browser/notifications/b...
File chrome/browser/notifications/balloon_host.cc (right):

http://codereview.chromium.org/10825150/diff/1/chrome/browser/notifications/b...
chrome/browser/notifications/balloon_host.cc:142:
web_contents_->GetMutableRendererPrefs(), balloon_->profile());
Is this modifying web_contents_->GetMutableRendererPrefs()? If so, shouldn't the
arguments be in the reverse order? I realize that might be out of scope for this
CL, but it's not clear from reading this what it is doing.

[Elliot Glaysher, 2012-08-02 17:23:46]

stampity lgtm.

[kinaba, 2012-08-03 01:39:23]

Oops, both are lgtm stamps.
Do you guys have any idea who will be the best person for reviewing this?

http://codereview.chromium.org/10825150/diff/1/chrome/browser/notifications/b...
File chrome/browser/notifications/balloon_host.cc (right):

http://codereview.chromium.org/10825150/diff/1/chrome/browser/notifications/b...
chrome/browser/notifications/balloon_host.cc:142:
web_contents_->GetMutableRendererPrefs(), balloon_->profile());
On 2012/08/02 17:17:34, stevenjb (chromium) wrote:
> Is this modifying web_contents_->GetMutableRendererPrefs()?

Yes, this is how the function is defined in 2 or 3 years ago...
I understand your concern, but not sure if it is worth changing.
For this CL, I'll add some comments here to clarify it.

[stevenjb, 2012-08-03 01:53:40]

Hmm, it's really not at all clear from 'blame' who 'owns' any of this. I'm not
familiar at all with either renderer_preferences_util::UpdateFromSystemSettings
or SyncRendererPrefs(). Maybe ask ben@, sky@, or jam@ who might be a good
reviewer?

[kinaba, 2012-08-03 04:01:57]

Adding jam@,
could you review the change (or could you suggest who is knowledgable in this
area)? It looks once fixed in r44709 but missed out in r109468 (migration to
TabContents from RenderViewHost).

[jam, 2012-08-06 19:11:15]

lgtm

[commit-bot: I haz the power, 2012-08-06 23:40:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kinaba@chromium.org/10825150/1

[commit-bot: I haz the power, 2012-08-07 01:45:32]

Try job failure for 10825150-1 (previous was lost) (retry) on win_rel for step
"runhooks".
It's a second try, previously, step "interactive_ui_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...