Remove double notification on Date pickers caused by trying to notify in onClick and in onStop.

Remove double notification on Date pickers caused by trying to notify in onClick and in onStop.


BUG=159381

[Miguel Garcia, 2012-11-30 12:43:39]

It seems you two were involved on upstreaming this. 

For some reason that I can't understand there we try to notify the renderer both
onClick and onStop if the APK is 16 or bigger. In those cases (all cases?) we
send two notifications instead of one which breaks some assumptions on webkit
(since some objects are freed once the selection is made).

Can you throw some light into this? there is very little git history upstream so
it's hard to know what motivated that in the first place.

[Leandro Graciá Gil, 2012-11-30 13:35:01]

You probably want to ask Oli about this. I'm adding him to the review.

On 2012/11/30 12:43:39, Miguel Garcia wrote:
> It seems you two were involved on upstreaming this. 
> 
> For some reason that I can't understand there we try to notify the renderer
both
> onClick and onStop if the APK is 16 or bigger. In those cases (all cases?) we
> send two notifications instead of one which breaks some assumptions on webkit
> (since some objects are freed once the selection is made).
> 
> Can you throw some light into this? there is very little git history upstream
so
> it's hard to know what motivated that in the first place.

[Leandro Graciá Gil, 2012-11-30 13:35:29]

+Oli

On 2012/11/30 13:35:01, Leandro Graciá Gil wrote:
> You probably want to ask Oli about this. I'm adding him to the review.
> 
> On 2012/11/30 12:43:39, Miguel Garcia wrote:
> > It seems you two were involved on upstreaming this. 
> > 
> > For some reason that I can't understand there we try to notify the renderer
> both
> > onClick and onStop if the APK is 16 or bigger. In those cases (all cases?)
we
> > send two notifications instead of one which breaks some assumptions on
webkit
> > (since some objects are freed once the selection is made).
> > 
> > Can you throw some light into this? there is very little git history
upstream
> so
> > it's hard to know what motivated that in the first place.

[olilan, 2012-12-03 10:26:00]

Yes, this code to apply the chosen date in onStop is added for JB because in JB
the behaviour is different from ICS. In JB dismissing the dialog (e.g. by
pressing Back) applies the chosen date.

The date should not be applied twice. When this was implemented, code was added
to ImeAdapter to prevent this, using the mDialogCanceled variable. If this has
been lost, that would explain the issue.

You can see the changes originally made for this at
https://gerrit-int.chromium.org/#/c/17478

On 2012/11/30 13:35:29, Leandro Graciá Gil wrote:
> +Oli
> 
> On 2012/11/30 13:35:01, Leandro Graciá Gil wrote:
> > You probably want to ask Oli about this. I'm adding him to the review.
> > 
> > On 2012/11/30 12:43:39, Miguel Garcia wrote:
> > > It seems you two were involved on upstreaming this. 
> > > 
> > > For some reason that I can't understand there we try to notify the
renderer
> > both
> > > onClick and onStop if the APK is 16 or bigger. In those cases (all cases?)
> we
> > > send two notifications instead of one which breaks some assumptions on
> webkit
> > > (since some objects are freed once the selection is made).
> > > 
> > > Can you throw some light into this? there is very little git history
> upstream
> > so
> > > it's hard to know what motivated that in the first place.

[Miguel Garcia, 2012-12-03 19:12:05]

Got it, so I am keeping this behavior then however I think you mention only
distinguishes a cancel (by pressing the cancel button) from a cancel (by
dismissing the dialog some other way). It does not seem to prevent double
notifications which is leading me to believe that we were happily notifying
twice with the same value. That was not a problem until now but due to some
WebKit changes it is.

I am sending https://codereview.chromium.org/11434087/ to fix this.


On 2012/12/03 10:26:00, olilan wrote:
> Yes, this code to apply the chosen date in onStop is added for JB because in
JB
> the behaviour is different from ICS. In JB dismissing the dialog (e.g. by
> pressing Back) applies the chosen date.
> 
> The date should not be applied twice. When this was implemented, code was
added
> to ImeAdapter to prevent this, using the mDialogCanceled variable. If this has
> been lost, that would explain the issue.
> 
> You can see the changes originally made for this at
> https://gerrit-int.chromium.org/#/c/17478
> 
> On 2012/11/30 13:35:29, Leandro Graciá Gil wrote:
> > +Oli
> > 
> > On 2012/11/30 13:35:01, Leandro Graciá Gil wrote:
> > > You probably want to ask Oli about this. I'm adding him to the review.
> > > 
> > > On 2012/11/30 12:43:39, Miguel Garcia wrote:
> > > > It seems you two were involved on upstreaming this. 
> > > > 
> > > > For some reason that I can't understand there we try to notify the
> renderer
> > > both
> > > > onClick and onStop if the APK is 16 or bigger. In those cases (all
cases?)
> > we
> > > > send two notifications instead of one which breaks some assumptions on
> > webkit
> > > > (since some objects are freed once the selection is made).
> > > > 
> > > > Can you throw some light into this? there is very little git history
> > upstream
> > > so
> > > > it's hard to know what motivated that in the first place.