Fix for out-of-sync scale factor issue

3rd try:

Fix for out-of-sync scale factor issue
- Set active root window early when creating browsers on startup.
- Send screen info when device scale factor chagned.
- gfx::Screen::GetDisplayNearestWindow should return Primary when the window's root doesn't exist. I missed this condition in  https://chromiumcodereview.appspot.com/10961021

BUG=155201, 152728
TEST=manual: see bug for repro steps

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=161758

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=162691

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=163105

[oshima, 2012-10-12 01:02:03]

piman -> render_widget_host_impl.cc
sky -> everything else

[sky, 2012-10-12 14:31:40]

http://codereview.chromium.org/11066130/diff/14001/chrome/browser/ui/startup/...
File chrome/browser/ui/startup/startup_browser_creator_impl.cc (right):

http://codereview.chromium.org/11066130/diff/14001/chrome/browser/ui/startup/...
chrome/browser/ui/startup/startup_browser_creator_impl.cc:815:
ash::Shell::GetInstance()->set_active_root_window(
I don't understand why this is necessary. Why is the renderer keying off the
active RootWindow? Wouldn't this mean if a timer kicks in and tries to create a
renderer that isn't in the active rootwindow it'll get the wrong scale?

[piman, 2012-10-12 16:02:08]

LGTM when Scott's concerns are addressed.

[oshima, 2012-10-12 16:04:46]

http://codereview.chromium.org/11066130/diff/14001/chrome/browser/ui/startup/...
File chrome/browser/ui/startup/startup_browser_creator_impl.cc (right):

http://codereview.chromium.org/11066130/diff/14001/chrome/browser/ui/startup/...
chrome/browser/ui/startup/startup_browser_creator_impl.cc:815:
ash::Shell::GetInstance()->set_active_root_window(
On 2012/10/12 14:31:40, sky wrote:
> I don't understand why this is necessary. Why is the renderer keying off the
> active RootWindow?

We're not passing enough information for WebContentsViewAura::CreateView
to be able to tell in which display/scale factor it is being created. Thus,
window_->SetParent(NULL) in this method will pick active root window.

And when you create a new browser on 1x display, while an active root window
is on 2x display, it will first create renderer with 2x scale factor, and then
switch to 1x when it is added to a browser. And there is a potential race
condition when you switch scale factor right after its created, which can result
in showing 1x on the web contents. (refreshing by moving/resizing fixes
the issue, according to the report). Even without the race, it'd be waste to
render at different scale factor and then drop the result.

> Wouldn't this mean if a timer kicks in and tries to create a
> renderer that isn't in the active rootwindow it'll get the wrong scale?

It will use the correct scale but issue is the race and waste, but above race
happen only when you switch immediately after it's created. Waste may still
happen though.

RIght fix is to pass enough information to WebContentsViewAura::CreateView
so that it can determine the correct scale factor, but it will requite a lot of
changes in many places, and it's risky to merge to 23. I filed a bug
(crbug.com/155443) and will work on this for 25.

And I'll update the comment to be more clear.

[oshima, 2012-10-12 17:11:27]

Uploaded new patch. PTAL.

[piman, 2012-10-12 17:12:56]

http://codereview.chromium.org/11066130/diff/10005/content/browser/web_conten...
File content/browser/web_contents/web_contents_view_aura.cc (right):

http://codereview.chromium.org/11066130/diff/10005/content/browser/web_conten...
content/browser/web_contents/web_contents_view_aura.cc:294: LOG(ERROR) <<
"CreateView:";
Did you mean to leave this in?

[oshima, 2012-10-12 17:44:53]

http://codereview.chromium.org/11066130/diff/10005/content/browser/web_conten...
File content/browser/web_contents/web_contents_view_aura.cc (right):

http://codereview.chromium.org/11066130/diff/10005/content/browser/web_conten...
content/browser/web_contents/web_contents_view_aura.cc:294: LOG(ERROR) <<
"CreateView:";
On 2012/10/12 17:12:56, piman wrote:
> Did you mean to leave this in?

Oops, my mistake. removed.

[piman, 2012-10-12 17:47:00]

lgtm

[sky, 2012-10-12 17:50:10]

As long as this is temporary, LGTM

[commit-bot: I haz the power, 2012-10-12 18:21:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/oshima@chromium.org/11066130/4008

[commit-bot: I haz the power, 2012-10-13 09:22:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/oshima@chromium.org/11066130/21001

[commit-bot: I haz the power, 2012-10-13 11:12:50]

Change committed as 161758

[commit-bot: I haz the power, 2012-10-18 09:38:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/oshima@chromium.org/11066130/33001

[commit-bot: I haz the power, 2012-10-18 11:25:23]

Change committed as 162691

[commit-bot: I haz the power, 2012-10-19 21:38:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/oshima@chromium.org/11066130/42001

[commit-bot: I haz the power, 2012-10-20 00:28:12]

Change committed as 163105