DeviceOrientationDispatcher: no orientation updates equal to lastOrientation().

DeviceOrientationDispatcher: no orientation updates equal to lastOrientation().

The DeviceOrientationDispatcher, which implements WebDeviceOrientationClient,
must not send orientation updates that are equal to its lastOrientation().
This can happen when the client is stopped and then started again.

BUG=44654
TEST=Manual

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

[joth, 2010-08-26 11:05:20]

LGTM, with 2 small suggestions / questions

http://codereview.chromium.org/3104038/diff/1/2
File chrome/renderer/device_orientation_dispatcher.cc (right):

http://codereview.chromium.org/3104038/diff/1/2#newcode69
chrome/renderer/device_orientation_dispatcher.cc:69: return;
could we do something like
if (last_orientation_.get() && last_orientation->Equals(p.can, p.alpha, ....)
  return;

btw it seems surprising you compare the alpha values even if they can't provide
them (& beta & gamma)
maybe comment they are guaranteed to be <whatever> if no provided?

http://codereview.chromium.org/3104038/diff/1/3
File chrome/renderer/device_orientation_dispatcher.h (right):

http://codereview.chromium.org/3104038/diff/1/3#newcode9
chrome/renderer/device_orientation_dispatcher.h:9: #include
"third_party/WebKit/WebKit/chromium/public/WebDeviceOrientation.h"
namespace WebKit { class WebDeviceOrientation; } instead?

[hans, 2010-08-26 16:01:00]

http://codereview.chromium.org/3104038/diff/1/2
File chrome/renderer/device_orientation_dispatcher.cc (right):

http://codereview.chromium.org/3104038/diff/1/2#newcode69
chrome/renderer/device_orientation_dispatcher.cc:69: return;
On 2010/08/26 11:05:20, joth wrote:
> could we do something like
> if (last_orientation_.get() && last_orientation->Equals(p.can, p.alpha, ....)
>   return;
Hmm, yeah that would be nice, but require a change in the WebKit class... I'm
trying to think if there might be a nice way to not have to do that.

Moving this out to a separate function to make it a bit clearer.

> 
> btw it seems surprising you compare the alpha values even if they can't
provide
> them (& beta & gamma)
> maybe comment they are guaranteed to be <whatever> if no provided?

Fixing this.

http://codereview.chromium.org/3104038/diff/1/3
File chrome/renderer/device_orientation_dispatcher.h (right):

http://codereview.chromium.org/3104038/diff/1/3#newcode9
chrome/renderer/device_orientation_dispatcher.h:9: #include
"third_party/WebKit/WebKit/chromium/public/WebDeviceOrientation.h"
On 2010/08/26 11:05:20, joth wrote:
> namespace WebKit { class WebDeviceOrientation; } instead?

Done.

[allanwoj, 2010-08-26 16:12:18]

LGTM

On 2010/08/26 16:01:00, hans wrote:
> http://codereview.chromium.org/3104038/diff/1/2
> File chrome/renderer/device_orientation_dispatcher.cc (right):
> 
> http://codereview.chromium.org/3104038/diff/1/2#newcode69
> chrome/renderer/device_orientation_dispatcher.cc:69: return;
> On 2010/08/26 11:05:20, joth wrote:
> > could we do something like
> > if (last_orientation_.get() && last_orientation->Equals(p.can, p.alpha,
....)
> >   return;
> Hmm, yeah that would be nice, but require a change in the WebKit class... I'm
> trying to think if there might be a nice way to not have to do that.
> 
> Moving this out to a separate function to make it a bit clearer.
> 
> > 
> > btw it seems surprising you compare the alpha values even if they can't
> provide
> > them (& beta & gamma)
> > maybe comment they are guaranteed to be <whatever> if no provided?
> 
> Fixing this.
> 
> http://codereview.chromium.org/3104038/diff/1/3
> File chrome/renderer/device_orientation_dispatcher.h (right):
> 
> http://codereview.chromium.org/3104038/diff/1/3#newcode9
> chrome/renderer/device_orientation_dispatcher.h:9: #include
> "third_party/WebKit/WebKit/chromium/public/WebDeviceOrientation.h"
> On 2010/08/26 11:05:20, joth wrote:
> > namespace WebKit { class WebDeviceOrientation; } instead?
> 
> Done.

[joth, 2010-08-26 16:24:12]

LGTM too

On 26 August 2010 17:12, <allanwoj@chromium.org> wrote:

> LGTM
>
>
> On 2010/08/26 16:01:00, hans wrote:
>
>> http://codereview.chromium.org/3104038/diff/1/2
>> File chrome/renderer/device_orientation_dispatcher.cc (right):
>>
>
>  http://codereview.chromium.org/3104038/diff/1/2#newcode69
>> chrome/renderer/device_orientation_dispatcher.cc:69: return;
>> On 2010/08/26 11:05:20, joth wrote:
>> > could we do something like
>> > if (last_orientation_.get() && last_orientation->Equals(p.can, p.alpha,
>>
> ....)
>
>> >   return;
>> Hmm, yeah that would be nice, but require a change in the WebKit class...
>> I'm
>> trying to think if there might be a nice way to not have to do that.
>>
>
>  Moving this out to a separate function to make it a bit clearer.
>>
>
>  >
>> > btw it seems surprising you compare the alpha values even if they can't
>> provide
>> > them (& beta & gamma)
>> > maybe comment they are guaranteed to be <whatever> if no provided?
>>
>
>  Fixing this.
>>
>
>  http://codereview.chromium.org/3104038/diff/1/3
>> File chrome/renderer/device_orientation_dispatcher.h (right):
>>
>
>  http://codereview.chromium.org/3104038/diff/1/3#newcode9
>> chrome/renderer/device_orientation_dispatcher.h:9: #include
>> "third_party/WebKit/WebKit/chromium/public/WebDeviceOrientation.h"
>> On 2010/08/26 11:05:20, joth wrote:
>> > namespace WebKit { class WebDeviceOrientation; } instead?
>>
>
>  Done.
>>
>
>
>
> http://codereview.chromium.org/3104038/show
>