[Snackbar] Replace popup window with views

[Snackbar] Replace popup window with views

Popup windows are managed in a window that is separate to the activity.
Historically this fact caused many snackbar related crashes. Also it is
a heavy burden to support popup window based snackbars in multi window
mode.

This CL converts the view layer of snackbars from popup window to
Android View. After this change, the snackbar design will be a pure MVC
structure, with SnackbarCollection(M), SnackbarView(V), and
SnackbarManager(C).

BUG=570778
Committed: https://crrev.com/9e85a210f2ea147f2524845bb5c7fdbecc0670f5
Cr-Commit-Position: refs/heads/master@{#381495}

[Ian Wen, 2016-03-02 00:21:22]

ianwen@chromium.org changed reviewers:
+ newt@chromium.org, twellington@chromium.org

[Ian Wen, 2016-03-02 00:21:23]

newt@: PTAL everything.
twellington@: PTAL the changes in dimens.xml and SnackbarView.java. (I changed
the way we calculate margins/width on tablet.)

[newt (away), 2016-03-02 02:18:48]

Thanks, Ian. I think this is a good improvement!

https://codereview.chromium.org/1759483002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkActivity.java
(right):

https://codereview.chromium.org/1759483002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkActivity.java:35:
mSnackbarManager = new SnackbarManager(findViewById(android.R.id.content));
Does findViewById(android.R.id.content) return the right result, even though
you're calling setContentView() below?

https://codereview.chromium.org/1759483002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarManager.java
(right):

https://codereview.chromium.org/1759483002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarManager.java:77:
* @param rootView The main view of the embedding activity.
For clarity: "The main view (e.g. android.R.id.content) of ..."

https://codereview.chromium.org/1759483002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarManager.java:79:
public SnackbarManager(View rootView) {
This should take a ViewGroup, not a View.

https://codereview.chromium.org/1759483002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarManager.java:146:
public boolean isShowing() {
All uses of this method seem a bit suspect... Something to keep in mind, and
maybe fix.

https://codereview.chromium.org/1759483002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java
(right):

https://codereview.chromium.org/1759483002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:44:
private OnGlobalLayoutListener mLayoutListener = new OnGlobalLayoutListener() {
Do we still need a global layout listener, or could we simply put the
adjustViewPosition() logic inside of onMeasure()?

https://codereview.chromium.org/1759483002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:59:
mIsTablet = DeviceFormFactor.isTablet(parent.getContext());
nit: create a local variable "context = parent.getContext()"

https://codereview.chromium.org/1759483002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:60:
mParent = (ViewGroup) parent;
Don't cast here. The casting should happen earlier, e.g. when you call
findViewById(android.R.id.content)

https://codereview.chromium.org/1759483002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:61:
mParent.setClipChildren(false);
Calling setClipChildren(false) on the content view could hurt performance or
affect other views. What other views are direct children of
android.R.id.content? Is this change definitely OK?

https://codereview.chromium.org/1759483002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:65:
mParent.addView(mView);
Instead of adding the view manually, you can just remove "false" from the
inflate() method above.

https://codereview.chromium.org/1759483002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:69:
mAnimationDuration = mView.getResources()
Starting the animation inside the constructor is a bit surprising. I might move
the code into a separate method. At the very least, I'd move the animation code
to the bottom of the constructor so that it happens after the view has been
fully initialized.

https://codereview.chromium.org/1759483002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:71:
Animation fadeIn = AnimationUtils.loadAnimation(mParent.getContext(),
R.anim.snackbar_in);
What happened to the customized animation we used to have when the keyboard is
showing?

Also, let's use AnimatorInflater (which returns an AnimatorSet), instead of the
ancient AnimationUtils.

https://codereview.chromium.org/1759483002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:86:
mView.animate().alpha(0f).setDuration(mAnimationDuration)
for readability, I'd put each method call on a separate line:

  mView.animate()
          .alpha(0f)
          .setDuration(mAnimationDuration)
          ...

https://codereview.chromium.org/1759483002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:95:
private void adjustViewPosition() {
note: for methods called from inner classes, Android recommends removing
"private". If the method is marked private, then the compile introduces an
additional "forwarding method" that just calls the private method.

http://developer.android.com/training/articles/perf-tips.html#PackageInner

https://codereview.chromium.org/1759483002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:105:
if (lp.bottomMargin != keyboardHeight + margin) {
What if the width changed, but not the height?

https://codereview.chromium.org/1759483002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:109:
if (width + 2 * margin > mParent.getWidth()) {
How about using Math.min()?:

   lp.width = Math.min(width, mParent.getWidth() - 2 * margin);

But, really, I'd prefer to have snackbars adapt automatically to layout changes
based on the available space. If we're on tablet but in narrow multi-window
mode, then the snackbar should have 0 margin, like on phones.

To do this, I think you want to override onMeasure().

[Ian Wen, 2016-03-08 05:31:30]

PTAL :)

https://chromiumcodereview.appspot.com/1759483002/diff/20001/chrome/android/j...
File
chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkActivity.java
(right):

https://chromiumcodereview.appspot.com/1759483002/diff/20001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/bookmarks/BookmarkActivity.java:35:
mSnackbarManager = new SnackbarManager(findViewById(android.R.id.content));
On 2016/03/02 02:18:48, newt wrote:
> Does findViewById(android.R.id.content) return the right result, even though
> you're calling setContentView() below?

Yes. id.content is the parent framelayout that holds the view that is set by
setContentView().

https://chromiumcodereview.appspot.com/1759483002/diff/20001/chrome/android/j...
File
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarManager.java
(right):

https://chromiumcodereview.appspot.com/1759483002/diff/20001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarManager.java:77:
* @param rootView The main view of the embedding activity.
On 2016/03/02 02:18:48, newt wrote:
> For clarity: "The main view (e.g. android.R.id.content) of ..."

Done.

https://chromiumcodereview.appspot.com/1759483002/diff/20001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarManager.java:79:
public SnackbarManager(View rootView) {
On 2016/03/02 02:18:48, newt wrote:
> This should take a ViewGroup, not a View.

Done.

https://chromiumcodereview.appspot.com/1759483002/diff/20001/chrome/android/j...
File
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java
(right):

https://chromiumcodereview.appspot.com/1759483002/diff/20001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:44:
private OnGlobalLayoutListener mLayoutListener = new OnGlobalLayoutListener() {
On 2016/03/02 02:18:48, newt wrote:
> Do we still need a global layout listener, or could we simply put the
> adjustViewPosition() logic inside of onMeasure()?

I believe we still need it. OnGlobalLayoutListener is only used for calculating
the size of the visual keyboard. If no_resize flag is set to an activity, the
keyboard will not re-layout the activity but only overlaps it. This means we are
not notified in onMeasure when the keyboard is shown.

https://chromiumcodereview.appspot.com/1759483002/diff/20001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:59:
mIsTablet = DeviceFormFactor.isTablet(parent.getContext());
On 2016/03/02 02:18:48, newt wrote:
> nit: create a local variable "context = parent.getContext()"

Done.

https://chromiumcodereview.appspot.com/1759483002/diff/20001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:60:
mParent = (ViewGroup) parent;
On 2016/03/02 02:18:48, newt wrote:
> Don't cast here. The casting should happen earlier, e.g. when you call
> findViewById(android.R.id.content)

Done.

https://chromiumcodereview.appspot.com/1759483002/diff/20001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:61:
mParent.setClipChildren(false);
Actually this line is not necessary.

https://chromiumcodereview.appspot.com/1759483002/diff/20001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:61:
mParent.setClipChildren(false);
On 2016/03/02 02:18:48, newt wrote:
> Calling setClipChildren(false) on the content view could hurt performance or
> affect other views. What other views are direct children of
> android.R.id.content? Is this change definitely OK?

No it's not okay and it's not necessary. It is a remainder of my previous
experiment, which is no longer needed. Good catch!

https://chromiumcodereview.appspot.com/1759483002/diff/20001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:65:
mParent.addView(mView);
On 2016/03/02 02:18:48, newt wrote:
> Instead of adding the view manually, you can just remove "false" from the
> inflate() method above.

Weird enough. It messes up with the surfaceview to let the inflater add the
view. The snackbar view will show up as a broken bitmap, even without the
animation.

https://chromiumcodereview.appspot.com/1759483002/diff/20001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:69:
mAnimationDuration = mView.getResources()
On 2016/03/02 02:18:48, newt wrote:
> Starting the animation inside the constructor is a bit surprising. I might
move
> the code into a separate method. At the very least, I'd move the animation
code
> to the bottom of the constructor so that it happens after the view has been
> fully initialized.

Originally there was a show() method, which is only called after the
constructor, so I merged them. Now I added the show() method back :)

https://chromiumcodereview.appspot.com/1759483002/diff/20001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:71:
Animation fadeIn = AnimationUtils.loadAnimation(mParent.getContext(),
R.anim.snackbar_in);
On 2016/03/02 02:18:48, newt wrote:
> What happened to the customized animation we used to have when the keyboard is
> showing?
> 
> Also, let's use AnimatorInflater (which returns an AnimatorSet), instead of
the
> ancient AnimationUtils.

Discussed offline. Using android.view.animation.Animation prevents surface view
clipping issue. Added comments.

https://chromiumcodereview.appspot.com/1759483002/diff/20001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:86:
mView.animate().alpha(0f).setDuration(mAnimationDuration)
On 2016/03/02 02:18:48, newt wrote:
> for readability, I'd put each method call on a separate line:
> 
>   mView.animate()
>           .alpha(0f)
>           .setDuration(mAnimationDuration)
>           ...

Done.

https://chromiumcodereview.appspot.com/1759483002/diff/20001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:95:
private void adjustViewPosition() {
On 2016/03/02 02:18:48, newt wrote:
> note: for methods called from inner classes, Android recommends removing
> "private". If the method is marked private, then the compile introduces an
> additional "forwarding method" that just calls the private method.
> 
> http://developer.android.com/training/articles/perf-tips.html#PackageInner

First time knowing this. Done!

https://chromiumcodereview.appspot.com/1759483002/diff/20001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:105:
if (lp.bottomMargin != keyboardHeight + margin) {
On 2016/03/02 02:18:48, newt wrote:
> What if the width changed, but not the height?

Good catch. Done.

https://chromiumcodereview.appspot.com/1759483002/diff/20001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:109:
if (width + 2 * margin > mParent.getWidth()) {
On 2016/03/02 02:18:48, newt wrote:
> How about using Math.min()?:
> 
>    lp.width = Math.min(width, mParent.getWidth() - 2 * margin);
> 
> But, really, I'd prefer to have snackbars adapt automatically to layout
changes
> based on the available space. If we're on tablet but in narrow multi-window
> mode, then the snackbar should have 0 margin, like on phones.
> 
> To do this, I think you want to override onMeasure().

Tried to move all code into onMeasure(), but later I realized that on screen
orientation change, onMeasure is actually called before value of
getVisibleDisplayFrame() updates. This means the keyboardHeight will be
calculated incorrectly.

Thus I fall back to use onGlobalLayoutListener, and I added another variable to
track the previous frame size. And all calculation will happen only after that
frame size has changed.

[newt (away), 2016-03-08 17:38:16]

https://chromiumcodereview.appspot.com/1759483002/diff/20001/chrome/android/j...
File
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java
(right):

https://chromiumcodereview.appspot.com/1759483002/diff/20001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:69:
mAnimationDuration = mView.getResources()
On 2016/03/08 05:31:29, Ian Wen wrote:
> On 2016/03/02 02:18:48, newt wrote:
> > Starting the animation inside the constructor is a bit surprising. I might
> move
> > the code into a separate method. At the very least, I'd move the animation
> code
> > to the bottom of the constructor so that it happens after the view has been
> > fully initialized.
> 
> Originally there was a show() method, which is only called after the
> constructor, so I merged them. Now I added the show() method back :)

Thanks. This is a clearer API now :)

https://chromiumcodereview.appspot.com/1759483002/diff/20001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:95:
private void adjustViewPosition() {
On 2016/03/08 05:31:30, Ian Wen wrote:
> On 2016/03/02 02:18:48, newt wrote:
> > note: for methods called from inner classes, Android recommends removing
> > "private". If the method is marked private, then the compile introduces an
> > additional "forwarding method" that just calls the private method.
> > 
> > http://developer.android.com/training/articles/perf-tips.html#PackageInner
> 
> First time knowing this. Done!

Good :)  Read through that page -- the other tips there are useful as well

https://chromiumcodereview.appspot.com/1759483002/diff/40001/chrome/android/j...
File
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java
(right):

https://chromiumcodereview.appspot.com/1759483002/diff/40001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:49:
adjustViewPosition();
Could you add a comment explaining why we need to call adjustViewPosition() in
onGlobalLayout() rather than in onMeasure(), which would be more typical?

https://chromiumcodereview.appspot.com/1759483002/diff/40001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:101:
mParent.removeView(mView);
Does this always get called, even if, say, the activity is hidden during the
animation?

https://chromiumcodereview.appspot.com/1759483002/diff/40001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:108:
// Only update the visible frame has changed, otherwise there will be a layout
loop.
s/the/if the/

https://chromiumcodereview.appspot.com/1759483002/diff/40001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:120:
if (mIsTablet) {
It'd be cleaner if we didn't check "mIsTablet" at all, but rather just compared
the available width (mParent.getWidth()) to 600dp. If the available width is <
600dp, then use no margins and full width; otherwise, use 24dp margins and a
fixed width.

By that token, it'd make sense to revert the changes to both dimens.xml files.

[Ian Wen, 2016-03-09 00:20:39]

https://codereview.chromium.org/1759483002/diff/40001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java
(right):

https://codereview.chromium.org/1759483002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:49:
adjustViewPosition();
On 2016/03/08 17:38:16, newt wrote:
> Could you add a comment explaining why we need to call adjustViewPosition() in
> onGlobalLayout() rather than in onMeasure(), which would be more typical?

Done.

https://codereview.chromium.org/1759483002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:101:
mParent.removeView(mView);
On 2016/03/08 17:38:16, newt wrote:
> Does this always get called, even if, say, the activity is hidden during the
> animation?

Yes. If the activity is stopped, the runnable is called right away.

https://codereview.chromium.org/1759483002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:108:
// Only update the visible frame has changed, otherwise there will be a layout
loop.
On 2016/03/08 17:38:16, newt wrote:
> s/the/if the/

Done.

https://codereview.chromium.org/1759483002/diff/40001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/snackbar/SnackbarView.java:120:
if (mIsTablet) {
On 2016/03/08 17:38:16, newt wrote:
> It'd be cleaner if we didn't check "mIsTablet" at all, but rather just
compared
> the available width (mParent.getWidth()) to 600dp. If the available width is <
> 600dp, then use no margins and full width; otherwise, use 24dp margins and a
> fixed width.
> 
> By that token, it'd make sense to revert the changes to both dimens.xml files.

Discussed offline. mIsTablet is still needed because we do not want the snackbar
to be float on phone.

[newt (away), 2016-03-09 00:54:21]

lgtm. looks good!

[Ian Wen, 2016-03-09 00:55:03]

The CQ bit was checked by ianwen@chromium.org

[commit-bot: I haz the power, 2016-03-09 00:55:46]

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

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

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

[commit-bot: I haz the power, 2016-03-09 01:10:24]

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

[Ian Wen, 2016-03-09 01:34:01]

The CQ bit was checked by ianwen@chromium.org

[Ian Wen, 2016-03-09 01:34:01]

The patchset sent to the CQ was uploaded after l-g-t-m from newt@chromium.org
Link to the patchset: https://codereview.chromium.org/1759483002/#ps80001
(title: "rebase")

[commit-bot: I haz the power, 2016-03-09 01:34:27]

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

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

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

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

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

[Ian Wen, 2016-03-09 07:43:00]

The CQ bit was checked by ianwen@chromium.org

[commit-bot: I haz the power, 2016-03-09 07:43:14]

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

[commit-bot: I haz the power, 2016-03-09 09:40:19]

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

[commit-bot: I haz the power, 2016-03-09 09:40:20]

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

[Theresa, 2016-03-11 03:33:33]

lgtm

Not sure why the DownloadTest#testDuplicateHttpPostDownload_Overwrite failed..
doesn't seem at all related to your change so I'd just try to send this through
CQ again.

[Ian Wen, 2016-03-15 23:56:04]

The CQ bit was checked by ianwen@chromium.org

[Ian Wen, 2016-03-15 23:56:04]

The patchset sent to the CQ was uploaded after l-g-t-m from
twellington@chromium.org, newt@chromium.org
Link to the patchset: https://codereview.chromium.org/1759483002/#ps100001
(title: "disable snackbar for download test")

[commit-bot: I haz the power, 2016-03-15 23:56:36]

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

[commit-bot: I haz the power, 2016-03-15 23:59:38]

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

[commit-bot: I haz the power, 2016-03-15 23:59:39]

Try jobs failed on following builders:
  linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios_dbg_simulator_gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_gn...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Ian Wen, 2016-03-16 00:44:14]

The CQ bit was checked by ianwen@chromium.org

[Ian Wen, 2016-03-16 00:44:15]

The patchset sent to the CQ was uploaded after l-g-t-m from
twellington@chromium.org, newt@chromium.org
Link to the patchset: https://codereview.chromium.org/1759483002/#ps120001
(title: "rebase")

[commit-bot: I haz the power, 2016-03-16 00:44:35]

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

[commit-bot: I haz the power, 2016-03-16 00:56:20]

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

[commit-bot: I haz the power, 2016-03-16 00:56:21]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Ian Wen, 2016-03-16 17:56:54]

The CQ bit was checked by ianwen@chromium.org

[commit-bot: I haz the power, 2016-03-16 17:57:14]

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

[commit-bot: I haz the power, 2016-03-16 18:05:13]

Committed patchset #7 (id:120001)

[commit-bot: I haz the power, 2016-03-16 18:07:01]

Description was changed from

==========
[Snackbar] Replace popup window with views

Popup windows are managed in a window that is separate to the activity.
Historically this fact caused many snackbar related crashes. Also it is
a heavy burden to support popup window based snackbars in multi window
mode.

This CL converts the view layer of snackbars from popup window to
Android View. After this change, the snackbar design will be a pure MVC
structure, with SnackbarCollection(M), SnackbarView(V), and
SnackbarManager(C).

BUG=570778
==========

to

==========
[Snackbar] Replace popup window with views

Popup windows are managed in a window that is separate to the activity.
Historically this fact caused many snackbar related crashes. Also it is
a heavy burden to support popup window based snackbars in multi window
mode.

This CL converts the view layer of snackbars from popup window to
Android View. After this change, the snackbar design will be a pure MVC
structure, with SnackbarCollection(M), SnackbarView(V), and
SnackbarManager(C).

BUG=570778

Committed: https://crrev.com/9e85a210f2ea147f2524845bb5c7fdbecc0670f5
Cr-Commit-Position: refs/heads/master@{#381495}
==========

[commit-bot: I haz the power, 2016-03-16 18:07:03]

Patchset 7 (id:??) landed as
https://crrev.com/9e85a210f2ea147f2524845bb5c7fdbecc0670f5
Cr-Commit-Position: refs/heads/master@{#381495}