New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 644774 link

Starred by 2 users

Issue metadata

Status: WontFix
Owner:
Closed: Dec 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

Use correct theme color for Blimp

Project Member Reported by xingliu@chromium.org, Sep 7 2016

Issue description

Currently in BlimpContentsImpl, we use a hard coded theme color BLIMP_THEME_COLOR for Blimp 0.6.

Use the correct color in the future.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 9 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d464f5a8d6f8570da079b8a6368bc3c1b145dcf9

commit d464f5a8d6f8570da079b8a6368bc3c1b145dcf9
Author: nyquist <nyquist@chromium.org>
Date: Fri Sep 09 06:33:17 2016

[blimp] Add support for having multiple tabs

There is still only a single Blimp tab, and the logic is for now very
simple: If there is already a blimp tab available, a new tab will be
WebContents based. If there are no blimp tabs, a blimp-based tab will
be created.

Also, when tabs are closed, it takes a while before the tab is really
destroyed, in case the user wants to undo the action. This feature
stays, but if the user selects "New Tab" or clicks the new tab button,
all the pending closures are first committed, ensuring that the tab
to be created might become a blimp tab, even if the current blimp tab
was pending closure.

Before this CL, the value 0 was always used fro the tab ID. This meant
that there would be a mixup between tabs that are closed and new tabs
that are created. This CL changes this so that each BlimpContentsImpl
gets its own unique ID, created client-side, and transferred to the
engine to be used for all communication regarding that tab, by an
update to the TabControlFeature to now include the ID.

The NavigationFeature is also updated to support receiving messages
after BlimpContentsImpl has been destroyed, which may happen, and
it now just logs those errors.

To ensure that this is visible in the tab switcher as well, a
workaround is added to ensure that the theme color is always shown,
even when on the new tab page. This is important in case where there
are many new tab pages, and one can not see which one is the blimp
tab, since the theme color is only updated when the tab navigates
away from the NTP.

To support the multiple IDs on the engine side, the Tab class is now
the implementor of the
EngineRenderWidgetFeature::RenderWidgetMessageDelegate instead of the
BlimpEngineSession. IME and compositor protos use the ID of the current
tab for their messages.

BUG= 644467 ,  644774 

Review-Url: https://codereview.chromium.org/2325893002
Cr-Commit-Position: refs/heads/master@{#417518}

[modify] https://crrev.com/d464f5a8d6f8570da079b8a6368bc3c1b145dcf9/blimp/client/app/android/blimp_view.cc
[modify] https://crrev.com/d464f5a8d6f8570da079b8a6368bc3c1b145dcf9/blimp/client/app/linux/blimp_display_manager.cc
[modify] https://crrev.com/d464f5a8d6f8570da079b8a6368bc3c1b145dcf9/blimp/client/core/compositor/blimp_compositor_manager.cc
[modify] https://crrev.com/d464f5a8d6f8570da079b8a6368bc3c1b145dcf9/blimp/client/core/compositor/blimp_compositor_manager.h
[modify] https://crrev.com/d464f5a8d6f8570da079b8a6368bc3c1b145dcf9/blimp/client/core/compositor/blimp_compositor_manager_unittest.cc
[modify] https://crrev.com/d464f5a8d6f8570da079b8a6368bc3c1b145dcf9/blimp/client/core/contents/blimp_contents_impl.cc
[modify] https://crrev.com/d464f5a8d6f8570da079b8a6368bc3c1b145dcf9/blimp/client/core/contents/blimp_contents_impl_unittest.cc
[modify] https://crrev.com/d464f5a8d6f8570da079b8a6368bc3c1b145dcf9/blimp/client/core/contents/blimp_contents_manager.cc
[modify] https://crrev.com/d464f5a8d6f8570da079b8a6368bc3c1b145dcf9/blimp/client/core/contents/blimp_contents_manager.h
[modify] https://crrev.com/d464f5a8d6f8570da079b8a6368bc3c1b145dcf9/blimp/client/core/contents/blimp_contents_manager_unittest.cc
[modify] https://crrev.com/d464f5a8d6f8570da079b8a6368bc3c1b145dcf9/blimp/client/core/contents/blimp_contents_observer_unittest.cc
[modify] https://crrev.com/d464f5a8d6f8570da079b8a6368bc3c1b145dcf9/blimp/client/core/contents/blimp_navigation_controller_impl.cc
[modify] https://crrev.com/d464f5a8d6f8570da079b8a6368bc3c1b145dcf9/blimp/client/core/contents/blimp_navigation_controller_impl.h
[modify] https://crrev.com/d464f5a8d6f8570da079b8a6368bc3c1b145dcf9/blimp/client/core/contents/blimp_navigation_controller_impl_unittest.cc
[modify] https://crrev.com/d464f5a8d6f8570da079b8a6368bc3c1b145dcf9/blimp/client/core/contents/navigation_feature.cc
[modify] https://crrev.com/d464f5a8d6f8570da079b8a6368bc3c1b145dcf9/blimp/client/core/contents/tab_control_feature.cc
[modify] https://crrev.com/d464f5a8d6f8570da079b8a6368bc3c1b145dcf9/blimp/engine/session/blimp_engine_session.cc
[modify] https://crrev.com/d464f5a8d6f8570da079b8a6368bc3c1b145dcf9/blimp/engine/session/blimp_engine_session.h
[modify] https://crrev.com/d464f5a8d6f8570da079b8a6368bc3c1b145dcf9/blimp/engine/session/tab.cc
[modify] https://crrev.com/d464f5a8d6f8570da079b8a6368bc3c1b145dcf9/blimp/engine/session/tab.h
[modify] https://crrev.com/d464f5a8d6f8570da079b8a6368bc3c1b145dcf9/chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java
[modify] https://crrev.com/d464f5a8d6f8570da079b8a6368bc3c1b145dcf9/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManagerDocument.java
[modify] https://crrev.com/d464f5a8d6f8570da079b8a6368bc3c1b145dcf9/chrome/android/java/src/org/chromium/chrome/browser/compositor/overlays/strip/StripLayoutHelper.java
[modify] https://crrev.com/d464f5a8d6f8570da079b8a6368bc3c1b145dcf9/chrome/android/java/src/org/chromium/chrome/browser/widget/emptybackground/EmptyBackgroundViewTablet.java

Project Member

Comment 2 by bugdroid1@chromium.org, Sep 12 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5f96e9a9fd95d7a9928de17effb1788de5f66691

commit 5f96e9a9fd95d7a9928de17effb1788de5f66691
Author: nyquist <nyquist@chromium.org>
Date: Mon Sep 12 22:26:12 2016

[blimp] Fix theme color in tab switcher

Before the CL https://codereview.chromium.org/2325763003/ landed, it was
possible to use a theme color without alpha for the tab switcher toolbar,
because the code was using the alpha from the underlying resource instead.
However after that CL this was no longer possible and a correct theme color
is required.

The CL that added theme colors to the tab switcher for the new tab page for
blimp tabs ( https://codereview.chromium.org/2325893002/ ) landed around the
same time, and there was no test, so this was not caught in that CL, and it
continued to use a theme color without alpha.

This CL just adds the alpha channel to the theme color from Blimp, which means
that it now propagates correctly throughout the codebase.

BUG= 644467 ,  644774 

Review-Url: https://codereview.chromium.org/2335863002
Cr-Commit-Position: refs/heads/master@{#418078}

[modify] https://crrev.com/5f96e9a9fd95d7a9928de17effb1788de5f66691/blimp/client/core/contents/android/java/src/org/chromium/blimp/core/contents/BlimpContentsImpl.java

Status: WontFix (was: Untriaged)

Sign in to add a comment