Make the ShortcutHelper toast instead of send the user home for web app banners

Make the ShortcutHelper toast instead of send the user home for web app banners

* Fix null pointer access when a shortcut is added via
  a manifest.  It was relying on the WebContents() being
  accessible immediately after stopping the observation of
  the WebContents.

* When adding a shortcut on the homescreen through web app
  banners, stop firing Intents to send the user to the
  home. Instead show a toast to indicate that a shortcut is
  being added to their homescreen.  The string was chosen by UI
  and is described on the bug.

* Hopefully fix the ShortcutHelperTests.

BUG=457424, 303486
TEST=ShortcutHelperTest

Committed: https://crrev.com/30728015d9945c0c988ffa0bbb00ffb86bcaeb72
Cr-Commit-Position: refs/heads/master@{#317079}

[gone, 2015-02-18 02:47:41]

dfalcantara@chromium.org changed reviewers:
+ tedchoc@chromium.org

[mlamouri (slow - plz ping), 2015-02-18 10:27:26]

mlamouri@chromium.org changed reviewers:
+ mlamouri@chromium.org

[mlamouri (slow - plz ping), 2015-02-18 10:27:26]

Wasn't the user redirected to the homescreen for security concerns?
Also, how would that interact with the usual Add to Homescreen (from menu) flow?
Is it using the new UI now?

[Ted C, 2015-02-18 16:02:28]

On 2015/02/18 10:27:26, Mounir Lamouri wrote:
> Wasn't the user redirected to the homescreen for security concerns?
> Also, how would that interact with the usual Add to Homescreen (from menu)
flow?
> Is it using the new UI now?

code - lgtm

[felt, 2015-02-18 22:03:45]

felt@chromium.org changed reviewers:
+ felt@chromium.org

[felt, 2015-02-18 22:03:45]

security rubber stamp lgtm

The original concern was about spoofing: we wanted the user to be able to see
precisely which icon/title was being added by the website, to prevent
adrienneporterfelt.com from pretending to be Gmail. However, the app install
banner shows both the title and the icon so the user can see that before
clicking the button in the first place. So having a toast instead seems OK.
(Mounir, if you're aware of another risk that I've overlooked please let me
know.)

https://chromiumcodereview.appspot.com/934183002/diff/1/chrome/android/java/s...
File chrome/android/java/strings/android_chrome_strings.grd (right):

https://chromiumcodereview.appspot.com/934183002/diff/1/chrome/android/java/s...
chrome/android/java/strings/android_chrome_strings.grd:973: This site was added
to your homescreen
would it make more sense to specify the origin ("foo.com was added to your
homescreen") instead of saying "This site"? (up to you: a suggestion, not a
security requirement)

[mlamouri (slow - plz ping), 2015-02-18 22:16:43]

If the security concerns are waved, it sounds good as long as the "Add To
Homescreen" menu item ends up in the same UI flow as the app banner. Otherwise,
the UI flow might look odd.

[felt, 2015-02-18 22:42:24]

On 2015/02/18 22:16:43, Mounir Lamouri wrote:
> If the security concerns are waved, it sounds good as long as the "Add To
> Homescreen" menu item ends up in the same UI flow as the app banner.
Otherwise,
> the UI flow might look odd.

Argh, we have both a thread and this CL going on at once. :'(

I initially thought that the "Add to homescreen" would also show the icon, but
apparently that is not the case right now. So my recommendation via e-mail is to
make the "add to homescreen" also show the icon as well.

[gone, 2015-02-19 00:45:21]

New patchsets have been uploaded after l-g-t-m from
tedchoc@chromium.org,felt@chromium.org

[gone, 2015-02-19 00:47:34]

Added support for either toasting or sending the user to the homescreen and
fixed a null pointer access.  PTAL at the updated changes.

[gone, 2015-02-19 00:47:55]

Oh, it also now indicates the domain+registry of the site in the toast when
adding a shortcut.

[felt, 2015-02-19 04:23:50]

I'm not entirely familiar with this code, but it looks like now there are two
versions -- it returns to homescreen if added via shortcut, does not if added
via banner. You should update the description. Assuming that's correct lgtm

[mlamouri (slow - plz ping), 2015-02-19 12:09:17]

lgtm

[gone, 2015-02-19 17:52:47]

Updated the description, thanks!

[gone, 2015-02-19 17:53:07]

The CQ bit was checked by dfalcantara@chromium.org

[commit-bot: I haz the power, 2015-02-19 17:54:11]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/934183002/20001

[commit-bot: I haz the power, 2015-02-19 18:28:05]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-02-19 18:28:38]

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