Don't create a LoFiBarPopupController if SnackbarManager is null

Don't create a LoFiBarPopupController if SnackbarManager is null

BUG=594381
Committed: https://crrev.com/e08672445f26f16bf7ed5f217447140cb2f3311a
Cr-Commit-Position: refs/heads/master@{#381078}

[megjablon, 2016-03-14 19:55:51]

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

[megjablon, 2016-03-14 19:55:52]

PTAL, thanks!

[gone, 2016-03-14 20:15:32]

Looks like mLoFiBarPopupController is null checked everywhere outside of this,
so lgtm.

[megjablon, 2016-03-14 20:16:09]

The CQ bit was checked by megjablon@chromium.org

[commit-bot: I haz the power, 2016-03-14 20:16:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1803823002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1803823002/1

[commit-bot: I haz the power, 2016-03-14 21:34:40]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-03-14 21:37:17]

Description was changed from

==========
Don't create a LoFiBarPopupController if SnackbarManager is null

BUG=594381
==========

to

==========
Don't create a LoFiBarPopupController if SnackbarManager is null

BUG=594381

Committed: https://crrev.com/e08672445f26f16bf7ed5f217447140cb2f3311a
Cr-Commit-Position: refs/heads/master@{#381078}
==========

[commit-bot: I haz the power, 2016-03-14 21:37:18]

Patchset 1 (id:??) landed as
https://crrev.com/e08672445f26f16bf7ed5f217447140cb2f3311a
Cr-Commit-Position: refs/heads/master@{#381078}

[Ian Wen, 2016-03-16 00:35:25]

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

[Ian Wen, 2016-03-16 00:35:25]

Q: if the tab was created early via CCT prerendering, does it mean that after
this patch lofi snackbar will never be shown for that tab? Is it fine?

Let's say GSA starts a custom tab, is it possible that this tab will be opened
in lofi mode? If so, how do we plan to notify the user?

[megjablon, 2016-03-16 00:52:38]

On 2016/03/16 00:35:25, Ian Wen wrote:
> Q: if the tab was created early via CCT prerendering, does it mean that after
> this patch lofi snackbar will never be shown for that tab? Is it fine?
> 
> Let's say GSA starts a custom tab, is it possible that this tab will be opened
> in lofi mode? If so, how do we plan to notify the user?

We're not ok with not showing the snackbar when Lo-Fi is used. Yes, if GSA
starts a custom tab, then it could get Lo-Fi mode. I'm not familiar with the
recent changes in Tab and their consequences. Is it the case that the activity
will be null when a tab is created via prerendering? Will we never have a
snackbar manager at this time for prerendered tabs?

[Ian Wen, 2016-03-16 01:10:53]

It is possible that when a tab is created, the activity is not there. For
example when the user clicks the system notification?

One way you can try is to workaround it, is to not store mSnackbarManager in
LofiBarController, and always call getSnackbarManager() from the mTab.

[gone, 2016-03-16 01:12:46]

Well, the notification triggers a Tab to be created eventually, but doesn't
create it on the spot.  Something else that could potentially happen is that the
Tab runs some javascript that creates another Tab while the Tab is being
transferred to another Activity.

[megjablon, 2016-03-16 18:45:04]

On 2016/03/16 01:12:46, dfalcantara wrote:
> Well, the notification triggers a Tab to be created eventually, but doesn't
> create it on the spot.  Something else that could potentially happen is that
the
> Tab runs some javascript that creates another Tab while the Tab is being
> transferred to another Activity.

Could a Tab be loading resources while it doesn't have an Activity? If that
doesn't happen then Ian's suggestion would work when onLoFiResponseReceived
triggers the snackbar. Could the LoFiBarPopupController live in ChromeActivity?

[gone, 2016-03-16 18:46:14]

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

[gone, 2016-03-16 18:46:14]

That's a Yusuf question :/  Does it make sense to open a thread about this
outside of this CL?

[Yusuf, 2016-03-16 18:48:08]

On 2016/03/16 18:46:14, dfalcantara wrote:
> That's a Yusuf question :/  Does it make sense to open a thread about this
> outside of this CL?

That is not possible right now. We need an activity to create a tab.
In a few weeks it may be possible.

[megjablon, 2016-03-16 18:51:10]

On 2016/03/16 18:48:08, Yusuf wrote:
> On 2016/03/16 18:46:14, dfalcantara wrote:
> > That's a Yusuf question :/  Does it make sense to open a thread about this
> > outside of this CL?
> 
> That is not possible right now. We need an activity to create a tab.
> In a few weeks it may be possible.

So currently getSnackbarManager() shouldn't return null since getActivity()
won't return null on Tab creation, correct?