Use Display instead of DisplayMetrics to find out the Display size.

Use Display instead of DisplayMetrics to find out the Display size.

It solves issues where we were sometimes getting stale data,
especially if reading information after a DisplayListener event.

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

[mlamouri (slow - plz ping), 2014-03-27 11:29:24]

This is required for CL https://codereview.chromium.org/213123011/ to work when
we use a DisplayListener instead of a ConfigurationChange (I guess the
DisplayMetrics from Configuration is changed after the Display object.)

[aelias_OOO_until_Jul13, 2014-03-28 01:06:46]

https://codereview.chromium.org/212933012/diff/1/ui/android/java/src/org/chro...
File ui/android/java/src/org/chromium/ui/gfx/DeviceDisplayInfo.java (right):

https://codereview.chromium.org/212933012/diff/1/ui/android/java/src/org/chro...
ui/android/java/src/org/chromium/ui/gfx/DeviceDisplayInfo.java:142:
DisplayMetrics metrics = new DisplayMetrics();
This is or risks being called at high frequency, I don't think we should create
an object here.  Adding jdduke@ who looked at bit at the performance of
displayinfo.

[jdduke (slow), 2014-03-28 02:24:25]

https://codereview.chromium.org/212933012/diff/1/ui/android/java/src/org/chro...
File ui/android/java/src/org/chromium/ui/gfx/DeviceDisplayInfo.java (right):

https://codereview.chromium.org/212933012/diff/1/ui/android/java/src/org/chro...
ui/android/java/src/org/chromium/ui/gfx/DeviceDisplayInfo.java:142:
DisplayMetrics metrics = new DisplayMetrics();
On 2014/03/28 01:06:46, aelias wrote:
> This is or risks being called at high frequency, I don't think we should
create
> an object here.  Adding jdduke@ who looked at bit at the performance of
> displayinfo.

Is this change because |mAppContext.getResources().getDisplayMetrics()| may
contain stale data?  We should definitely avoid creating garbage here, it should
be sufficient to create a single |DisplayMetric| instance on this object, and
re-use that for each query.  Many of the |Display| methods incur a
synchronization cost, but that's probably fine.

[mlamouri (slow - plz ping), 2014-03-28 14:15:03]

PTAL.

https://codereview.chromium.org/212933012/diff/1/ui/android/java/src/org/chro...
File ui/android/java/src/org/chromium/ui/gfx/DeviceDisplayInfo.java (right):

https://codereview.chromium.org/212933012/diff/1/ui/android/java/src/org/chro...
ui/android/java/src/org/chromium/ui/gfx/DeviceDisplayInfo.java:142:
DisplayMetrics metrics = new DisplayMetrics();
On 2014/03/28 02:24:26, jdduke wrote:
> On 2014/03/28 01:06:46, aelias wrote:
> > This is or risks being called at high frequency, I don't think we should
> create
> > an object here.  Adding jdduke@ who looked at bit at the performance of
> > displayinfo.
> 
> Is this change because |mAppContext.getResources().getDisplayMetrics()| may
> contain stale data?  We should definitely avoid creating garbage here, it
should
> be sufficient to create a single |DisplayMetric| instance on this object, and
> re-use that for each query.  Many of the |Display| methods incur a
> synchronization cost, but that's probably fine. 

Yes, the data we have is not in sync if we use a DisplayListener to listen for
changes. I updated DisplayMetrics to a temp object in the class.

[jdduke (slow), 2014-03-28 14:48:02]

https://codereview.chromium.org/212933012/diff/20001/ui/android/java/src/org/...
File ui/android/java/src/org/chromium/ui/gfx/DeviceDisplayInfo.java (right):

https://codereview.chromium.org/212933012/diff/20001/ui/android/java/src/org/...
ui/android/java/src/org/chromium/ui/gfx/DeviceDisplayInfo.java:86: return
getDisplay().getPixelFormat();
Any reason for the change here? This call appears to be deprecated after JBMR1.

[mlamouri (slow - plz ping), 2014-03-28 14:51:49]

https://codereview.chromium.org/212933012/diff/20001/ui/android/java/src/org/...
File ui/android/java/src/org/chromium/ui/gfx/DeviceDisplayInfo.java (right):

https://codereview.chromium.org/212933012/diff/20001/ui/android/java/src/org/...
ui/android/java/src/org/chromium/ui/gfx/DeviceDisplayInfo.java:86: return
getDisplay().getPixelFormat();
On 2014/03/28 14:48:02, jdduke wrote:
> Any reason for the change here? This call appears to be deprecated after
JBMR1.

I have no strong opinion about that. Just thought it was simpler to just do that
call instead of branching for no reason (the call will produce the expected
result). Happy to remove that if you want.

[jdduke (slow), 2014-03-28 14:55:59]

https://codereview.chromium.org/212933012/diff/20001/ui/android/java/src/org/...
File ui/android/java/src/org/chromium/ui/gfx/DeviceDisplayInfo.java (right):

https://codereview.chromium.org/212933012/diff/20001/ui/android/java/src/org/...
ui/android/java/src/org/chromium/ui/gfx/DeviceDisplayInfo.java:86: return
getDisplay().getPixelFormat();
On 2014/03/28 14:51:49, Mounir Lamouri wrote:
> On 2014/03/28 14:48:02, jdduke wrote:
> > Any reason for the change here? This call appears to be deprecated after
> JBMR1.
> 
> I have no strong opinion about that. Just thought it was simpler to just do
that
> call instead of branching for no reason (the call will produce the expected
> result). Happy to remove that if you want.

Yeah let's change it back.  The branch should be optimized out.

[mlamouri (slow - plz ping), 2014-03-28 15:00:12]

PTAL

[jdduke (slow), 2014-03-28 15:37:20]

lgtm, but please update the description to say why the change was necessary
(something about otherwise getting stale data), and remove the line "The CL is
also cleaning up a bit some random stuff in the file.". Thanks.

[mlamouri (slow - plz ping), 2014-03-28 16:03:34]

The CQ bit was checked by mlamouri@chromium.org

[commit-bot: I haz the power, 2014-03-28 16:06:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/mlamouri@chromium.org/212933012/40001

[commit-bot: I haz the power, 2014-03-28 18:58:57]

Change committed as 260223