ShareHelper: Call onCancel when dismissed, in Kit Kat and earlier.

ShareHelper: Call onCancel when dismissed, in Kit Kat and earlier.

Previously, with the pre-Lollipop picker, onCancel was never called. Now
it is called immediately upon dismissing the dialog. In post-Lollipop,
there is still an ongoing issue where onCancel is not called immediately
upon closing the picker (it waits until the next share action); see
crbug.com/636274.

BUG=646247, 636274
Committed: https://crrev.com/2a9911b41a53319b9b60a3624c8df34171315a53
Cr-Commit-Position: refs/heads/master@{#421151}

[Matt Giuca, 2016-09-13 05:09:12]

mgiuca@chromium.org changed reviewers:
+ dtrainor@chromium.org

[Matt Giuca, 2016-09-13 05:09:12]

Hi David,

If you can think of a better or more natural way to have a mutable local
variable available inside the closure (than a singleton array), let me know. I
thought about creating a MutableBoolean or MutableValue class but it seemed like
overkill.

[David Trainor- moved to gerrit, 2016-09-14 17:12:26]

On 2016/09/13 05:09:12, Matt Giuca wrote:
> Hi David,
> 
> If you can think of a better or more natural way to have a mutable local
> variable available inside the closure (than a singleton array), let me know. I
> thought about creating a MutableBoolean or MutableValue class but it seemed
like
> overkill.

We've used AtomicBoolean a lot, which does roughly what you're suggesting :). 
wdyt?

[Matt Giuca, 2016-09-15 01:05:20]

On 2016/09/14 17:12:26, David Trainor wrote:
> On 2016/09/13 05:09:12, Matt Giuca wrote:
> > Hi David,
> > 
> > If you can think of a better or more natural way to have a mutable local
> > variable available inside the closure (than a singleton array), let me know.
I
> > thought about creating a MutableBoolean or MutableValue class but it seemed
> like
> > overkill.
> 
> We've used AtomicBoolean a lot, which does roughly what you're suggesting :). 
> wdyt?

Hmm. That seems like it would work but I'm not sure what the ramifications
are... I mean the AtomicBoolean documentation says things like "cannot be used
as a replacement for a Boolean", and "Atomic classes are not general purpose
replacements for java.lang.Integer and related classes". I can do it if you
want, but it seems like a weird fit.

[Matt Giuca, 2016-09-19 01:20:03]

Hi, friendly ping?

[Matt Giuca, 2016-09-21 03:05:10]

The CQ bit was checked by mgiuca@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-21 03:05:33]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-21 03:31:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-21 03:31:37]

Dry run: This issue passed the CQ dry run.

[David Trainor- moved to gerrit, 2016-09-23 20:24:17]

yeah that's fair.  This is roughly the same thing anyway :).

lgtm

[Matt Giuca, 2016-09-26 00:12:27]

The CQ bit was checked by mgiuca@chromium.org

[commit-bot: I haz the power, 2016-09-26 00:12:39]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-26 02:45:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-26 02:45:48]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Matt Giuca, 2016-09-26 03:06:38]

The CQ bit was checked by mgiuca@chromium.org

[commit-bot: I haz the power, 2016-09-26 03:06:44]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-26 05:35:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-26 05:35:39]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Matt Giuca, 2016-09-27 08:22:52]

The CQ bit was checked by mgiuca@chromium.org

[Matt Giuca, 2016-09-27 08:22:52]

The patchset sent to the CQ was uploaded after l-g-t-m from
dtrainor@chromium.org
Link to the patchset: https://codereview.chromium.org/2338683002/#ps60001
(title: "Rebase.")

[commit-bot: I haz the power, 2016-09-27 08:23:08]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-27 08:58:09]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-09-27 09:00:29]

Description was changed from

==========
ShareHelper: Call onCancel when dismissed, in Kit Kat and earlier.

Previously, with the pre-Lollipop picker, onCancel was never called. Now
it is called immediately upon dismissing the dialog. In post-Lollipop,
there is still an ongoing issue where onCancel is not called immediately
upon closing the picker (it waits until the next share action); see
crbug.com/636274.

BUG=646247,636274
==========

to

==========
ShareHelper: Call onCancel when dismissed, in Kit Kat and earlier.

Previously, with the pre-Lollipop picker, onCancel was never called. Now
it is called immediately upon dismissing the dialog. In post-Lollipop,
there is still an ongoing issue where onCancel is not called immediately
upon closing the picker (it waits until the next share action); see
crbug.com/636274.

BUG=646247,636274

Committed: https://crrev.com/2a9911b41a53319b9b60a3624c8df34171315a53
Cr-Commit-Position: refs/heads/master@{#421151}
==========

[commit-bot: I haz the power, 2016-09-27 09:00:30]

Patchset 4 (id:??) landed as
https://crrev.com/2a9911b41a53319b9b60a3624c8df34171315a53
Cr-Commit-Position: refs/heads/master@{#421151}