Fix crash on J,K while swiping on custom tabs title

Fix crash on J,K while swiping on custom tabs title

We use LayoutManagerChrome on Tabbed mode and override all related
swipe related code, but the parent class LayoutMangerDocument has
document mode specific behavior which requires L only APIs. Until
custom tabs launched, it wasn't possible to reach this codepath since
no J,K builds had an activity launching with LayoutManagerDocument.
This guards the call with a no op if the device is not document eligible.

BUG=525153
Committed: https://crrev.com/a1051b84638e236bddcb2b5ae195278d292cc9bc
Cr-Commit-Position: refs/heads/master@{#346783}

[Yusuf, 2015-09-01 21:35:52]

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

[Yusuf, 2015-09-01 21:36:00]

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

[commit-bot: I haz the power, 2015-09-01 21:36:43]

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

[David Trainor- moved to gerrit, 2015-09-01 21:56:38]

https://codereview.chromium.org/1313043003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManagerDocument.java
(right):

https://codereview.chromium.org/1313043003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManagerDocument.java:430:
if (!FeatureUtilities.isDocumentModeEligible(context)) return;
This should probably go into isSwipeEnabled right?  It looks like that's
*trying* to do what this is doing (probably fixes normal document mode, but not
custom tabs).

Also, can we use mHost.getContext()?

[Yusuf, 2015-09-01 22:02:18]

https://codereview.chromium.org/1313043003/diff/1/chrome/android/java/src/org...
File
chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManagerDocument.java
(right):

https://codereview.chromium.org/1313043003/diff/1/chrome/android/java/src/org...
chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManagerDocument.java:430:
if (!FeatureUtilities.isDocumentModeEligible(context)) return;
On 2015/09/01 21:56:38, David Trainor wrote:
> This should probably go into isSwipeEnabled right?  It looks like that's
> *trying* to do what this is doing (probably fixes normal document mode, but
not
> custom tabs).
> 
> Also, can we use mHost.getContext()?

Done.

[Yusuf, 2015-09-01 22:02:32]

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

[commit-bot: I haz the power, 2015-09-01 22:03:09]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1313043003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1313043003/20001

[David Trainor- moved to gerrit, 2015-09-01 22:23:51]

https://codereview.chromium.org/1313043003/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManagerDocument.java
(right):

https://codereview.chromium.org/1313043003/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManagerDocument.java:448:
|| !FeatureUtilities.isDocumentModeEligible(mHost.getContext())
Now that I think about it, I think this is supposed to work with document mode
right?  We use model.setIndex() to force the activity swap.  I'm actually
surprised this is enabled though.  I think the issue is you're in "tabbed
chrome" but in a "custom tab" right?

[Yusuf, 2015-09-01 22:29:42]

https://codereview.chromium.org/1313043003/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManagerDocument.java
(right):

https://codereview.chromium.org/1313043003/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManagerDocument.java:448:
|| !FeatureUtilities.isDocumentModeEligible(mHost.getContext())
On 2015/09/01 22:23:50, David Trainor wrote:
> Now that I think about it, I think this is supposed to work with document mode
> right?  We use model.setIndex() to force the activity swap.  I'm actually
> surprised this is enabled though.  I think the issue is you're in "tabbed
> chrome" but in a "custom tab" right?

Yes, we are in tabbed mode since it is J or K. It is not "really" enabled. The
base class is LayoutManagerDocument and all related impl assumes since you are
using this class and not overriding anything running L APIs is OK which is imo
the main problem here.

We have @TargetAPI(Lollipop) above but no formal code that blocks the use of
that API. This line fixes that. CustomTabActivity by using the simplest, parent
LayoutManager starting calling that API on J and K devices.

[commit-bot: I haz the power, 2015-09-01 22:49:32]

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

[commit-bot: I haz the power, 2015-09-01 22:49:33]

Dry run: This issue passed the CQ dry run.

[David Trainor- moved to gerrit, 2015-09-01 23:23:53]

lgtm

https://codereview.chromium.org/1313043003/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManagerDocument.java
(right):

https://codereview.chromium.org/1313043003/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManagerDocument.java:448:
|| !FeatureUtilities.isDocumentModeEligible(mHost.getContext())
On 2015/09/01 22:29:42, Yusuf wrote:
> On 2015/09/01 22:23:50, David Trainor wrote:
> > Now that I think about it, I think this is supposed to work with document
mode
> > right?  We use model.setIndex() to force the activity swap.  I'm actually
> > surprised this is enabled though.  I think the issue is you're in "tabbed
> > chrome" but in a "custom tab" right?
> 
> Yes, we are in tabbed mode since it is J or K. It is not "really" enabled. The
> base class is LayoutManagerDocument and all related impl assumes since you are
> using this class and not overriding anything running L APIs is OK which is imo
> the main problem here.
> 
> We have @TargetAPI(Lollipop) above but no formal code that blocks the use of
> that API. This line fixes that. CustomTabActivity by using the simplest,
parent
> LayoutManager starting calling that API on J and K devices. 

Ok that works for me and makes sense! :)

[Yusuf, 2015-09-01 23:24:48]

The CQ bit was checked by yusufo@chromium.org

[commit-bot: I haz the power, 2015-09-01 23:25:40]

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

[commit-bot: I haz the power, 2015-09-01 23:32:45]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-09-01 23:33:22]

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