The android ShareHelper code (org.chromium.chrome.browser.share.ShareHelper) is getting a bit unwieldy (I suspect due to scope creep over a number of years). I've had to make a flowchart just to get my head around the interactions between the various methods.

The flowchart is attached.

The main weirdness is the extremely different logic used for Lollipop MR1+ target picker, and the custom pre-Lollipop target picker, and the unclear role of the makeIntentAndShare and shareIntent methods:

- When sharing directly to a target, makeIntentAndShare launches that target without a picker.
- Below LMR1, a picker is shown, then the result is passed to makeIntentAndShare, which launches that target.
- In LMR1 and above, makeIntentAndShare itself shows the picker.

So depending on the Android version, the picker fits into the code flow at very different points. MIAS has two different modes depending on whether a target has already been chosen, and one of those modes is illegal (asserts) on pre-Lollipop. I think it could be refactored so that the code flow looks very similar between the two versions (moving the pre-LMR1 code down into shareIntent so it works much like the post-LMR1 flow).

Another thing is that ShareHelper itself is never instantiated, and has no member fields, but a whole bunch of static methods that pass around the same arguments everywhere. I needed to add a few more arguments and had to add them to all the methods: https://codereview.chromium.org/2203273002/. I think this would be significantly improved if ShareHelper was instantiated (internally, by top-level static methods like Share), then all of these arguments that are getting passed around would become member fields of ShareHelper, not arguments to all of these methods.

I'm happy to do this refactoring as part of my work on Web Share. I'd be interested in the Android team's thoughts on the above proposals.
 

Deleted: ShareHelper code flows.svg 186 KB

ShareHelper code flows.svg 186 KB Download