Close other tabs when creating new Blimp tab |
||||
Issue descriptionBlimp only supports one tab at this time, close out other tabs when a new tab is created. Driving this from the engine would be the right way to do this, but it would require changing the protocol to fully understand tab ids. nyquist@ to investigate.
,
Sep 8 2016
,
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
,
Sep 9 2016
Unless the CL landed in #3 is reverted, this should now be fixed.
,
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
,
Dec 9 2016
|
||||
►
Sign in to add a comment |
||||
Comment 1 by nyquist@chromium.org
, Sep 7 2016