Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(167)

Issue 2325893002: [blimp] Add support for having multiple tabs (Closed)

Created:
4 years, 3 months ago by nyquist
Modified:
4 years, 3 months ago
CC:
chromium-reviews, anandc+watch-blimp_chromium.org, maniscalco+watch-blimp_chromium.org, sriramsr+watch-blimp_chromium.org, gcasto+watch-blimp_chromium.org, nyquist+watch-blimp_chromium.org, marcinjb+watch-blimp_chromium.org, jessicag+watch-blimp_chromium.org, kmarshall+watch-blimp_chromium.org, lethalantidote+watch-blimp_chromium.org, dtrainor+watch-blimp_chromium.org, shaktisahu+watch-blimp_chromium.org, khushalsagar+watch-blimp_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[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 Committed: https://crrev.com/d464f5a8d6f8570da079b8a6368bc3c1b145dcf9 Cr-Commit-Position: refs/heads/master@{#417518}

Patch Set 1 #

Patch Set 2 : Add tablet and non-blimp support #

Unified diffs Side-by-side diffs Delta from patch set Stats (+133 lines, -96 lines) Patch
M blimp/client/app/android/blimp_view.cc View 2 chunks +6 lines, -1 line 0 comments Download
M blimp/client/app/linux/blimp_display_manager.cc View 2 chunks +3 lines, -1 line 0 comments Download
M blimp/client/core/compositor/blimp_compositor_manager.h View 2 chunks +3 lines, -0 lines 0 comments Download
M blimp/client/core/compositor/blimp_compositor_manager.cc View 3 chunks +9 lines, -11 lines 0 comments Download
M blimp/client/core/compositor/blimp_compositor_manager_unittest.cc View 3 chunks +8 lines, -3 lines 0 comments Download
M blimp/client/core/contents/blimp_contents_impl.cc View 1 chunk +2 lines, -2 lines 0 comments Download
M blimp/client/core/contents/blimp_contents_impl_unittest.cc View 3 chunks +5 lines, -5 lines 0 comments Download
M blimp/client/core/contents/blimp_contents_manager.h View 1 chunk +4 lines, -0 lines 0 comments Download
M blimp/client/core/contents/blimp_contents_manager.cc View 2 chunks +1 line, -8 lines 0 comments Download
M blimp/client/core/contents/blimp_contents_manager_unittest.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M blimp/client/core/contents/blimp_contents_observer_unittest.cc View 3 chunks +4 lines, -4 lines 0 comments Download
M blimp/client/core/contents/blimp_navigation_controller_impl.h View 2 chunks +5 lines, -1 line 0 comments Download
M blimp/client/core/contents/blimp_navigation_controller_impl.cc View 2 chunks +11 lines, -13 lines 0 comments Download
M blimp/client/core/contents/blimp_navigation_controller_impl_unittest.cc View 3 chunks +5 lines, -2 lines 0 comments Download
M blimp/client/core/contents/navigation_feature.cc View 1 chunk +7 lines, -1 line 0 comments Download
M blimp/client/core/contents/tab_control_feature.cc View 2 chunks +2 lines, -0 lines 0 comments Download
M blimp/engine/session/blimp_engine_session.h View 2 chunks +3 lines, -14 lines 0 comments Download
M blimp/engine/session/blimp_engine_session.cc View 7 chunks +7 lines, -24 lines 0 comments Download
M blimp/engine/session/tab.h View 3 chunks +13 lines, -1 line 0 comments Download
M blimp/engine/session/tab.cc View 2 chunks +19 lines, -1 line 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/ChromeTabbedActivity.java View 2 chunks +3 lines, -0 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManagerDocument.java View 1 1 chunk +8 lines, -2 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/compositor/overlays/strip/StripLayoutHelper.java View 1 2 chunks +2 lines, -0 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/widget/emptybackground/EmptyBackgroundViewTablet.java View 1 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 16 (11 generated)
nyquist
dtrainor: PTAL
4 years, 3 months ago (2016-09-09 01:37:05 UTC) #5
David Trainor- moved to gerrit
lgtm
4 years, 3 months ago (2016-09-09 05:44:48 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2325893002/20001
4 years, 3 months ago (2016-09-09 06:27:47 UTC) #12
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 3 months ago (2016-09-09 06:33:35 UTC) #14
commit-bot: I haz the power
4 years, 3 months ago (2016-09-09 06:35:20 UTC) #16
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/d464f5a8d6f8570da079b8a6368bc3c1b145dcf9
Cr-Commit-Position: refs/heads/master@{#417518}

Powered by Google App Engine
This is Rietveld 408576698