Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2617293002:
DevTools: TabbedLocations only materialize tabs when needed (Closed)
|
Created:
3 years, 11 months ago by luoe Modified:
3 years, 11 months ago CC:
apavlov+blink_chromium.org, blink-reviews, caseq+blink_chromium.org, chromium-reviews, devtools-reviews_chromium.org, kozyatinskiy+blink_chromium.org, lushnikov+blink_chromium.org, pfeldman+blink_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionDevTools: TabbedLocations only materialize tabs when needed
TabbedLocations used to materialize tab contents on tabSelected. Now,
they materialize only if the tab is selected while the TabbedPane is showing, or
when the TabbedPane is being shown.
BUG=none
Review-Url: https://codereview.chromium.org/2617293002
Cr-Commit-Position: refs/heads/master@{#442824}
Committed: https://chromium.googlesource.com/chromium/src/+/7a9e12f9d21ca00eb2d15262f317a183213ae164
Patch Set 1 #
Total comments: 6
Patch Set 2 : rework for TabbedLocations #
Total comments: 2
Patch Set 3 : DevTools: materialize container widgets only when they are shown #Messages
Total messages: 22 (10 generated)
Description was changed from ========== DevTools: create drawer only when needed BUG=none ========== to ========== DevTools: create drawer only when needed View manager creates the drawer's tabbed location and selects the last saved tab. Doing so materializes the tab contents, causing modules to load even when the drawer itself is not visible. This CL creates the drawer only when it is visible, not on startup. BUG=none ==========
luoe@chromium.org changed reviewers: + einbinder@chromium.org
Please take a look
https://codereview.chromium.org/2617293002/diff/1/third_party/WebKit/Source/d... File third_party/WebKit/Source/devtools/front_end/ui/InspectorView.js (right): https://codereview.chromium.org/2617293002/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/ui/InspectorView.js:46: this._drawerBuilt = false; What is this used for? https://codereview.chromium.org/2617293002/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/ui/InspectorView.js:176: _drawerLocation() { jsdoc @return https://codereview.chromium.org/2617293002/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/ui/InspectorView.js:180: this._drawerTabbedLocation = jsdoc @type this in constructor https://codereview.chromium.org/2617293002/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/ui/InspectorView.js:183: this._drawerTabbedPane = this._drawerTabbedLocation.tabbedPane(); var drawerTabbedPane = https://codereview.chromium.org/2617293002/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/ui/InspectorView.js:210: return this._drawerLocation().tabbedPane().isShowing(); !!this._drawerTabbedLocation && this._drawerLocation().tabbedPane().isShowing() https://codereview.chromium.org/2617293002/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/ui/InspectorView.js:214: if (!this._drawerLocation().tabbedPane().isShowing()) !!this._drawerTabbedLocation && this._drawerLocation().tabbedPane().isShowing()
This could be fixed for all the locations - don't materialize default tab until tabbed pane is shown.
Description was changed from ========== DevTools: create drawer only when needed View manager creates the drawer's tabbed location and selects the last saved tab. Doing so materializes the tab contents, causing modules to load even when the drawer itself is not visible. This CL creates the drawer only when it is visible, not on startup. BUG=none ========== to ========== DevTools: TabbedLocations only materialize tabs when needed TabbedLocations used to materialize tab contents on tabSelected. Now, they materialize only if the tab is selected while the TabbedPane is showing, or when the TabbedPane is being shown. BUG=none ==========
luoe@chromium.org changed reviewers: + dgozman@chromium.org
Thanks for the feedback, it's been reworked in the latest patch. I've introduced a new TabsShown event for the location to listen to. Please take a look.
lgtm
https://codereview.chromium.org/2617293002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/devtools/front_end/ui/View.js (right): https://codereview.chromium.org/2617293002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/ui/View.js:756: _tabsShown(event) { We can instead materialize when our wrapper (UI.ViewManager._ContainerWidget) was shown.
Done. https://codereview.chromium.org/2617293002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/devtools/front_end/ui/View.js (right): https://codereview.chromium.org/2617293002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/ui/View.js:756: _tabsShown(event) { Brilliant idea
lgtm
The CQ bit was checked by luoe@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from einbinder@chromium.org Link to the patchset: https://codereview.chromium.org/2617293002/#ps40001 (title: "DevTools: materialize container widgets only when they are shown")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by luoe@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1484110057750380,
"parent_rev": "1f8e11ba4fa17aa707f9a8a4a70807745faa3f4f", "commit_rev":
"7a9e12f9d21ca00eb2d15262f317a183213ae164"}
Message was sent while issue was closed.
Description was changed from ========== DevTools: TabbedLocations only materialize tabs when needed TabbedLocations used to materialize tab contents on tabSelected. Now, they materialize only if the tab is selected while the TabbedPane is showing, or when the TabbedPane is being shown. BUG=none ========== to ========== DevTools: TabbedLocations only materialize tabs when needed TabbedLocations used to materialize tab contents on tabSelected. Now, they materialize only if the tab is selected while the TabbedPane is showing, or when the TabbedPane is being shown. BUG=none Review-Url: https://codereview.chromium.org/2617293002 Cr-Commit-Position: refs/heads/master@{#442824} Committed: https://chromium.googlesource.com/chromium/src/+/7a9e12f9d21ca00eb2d15262f317... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/7a9e12f9d21ca00eb2d15262f317... |
