WebViewImpl is both today, but we should give them separate objects and lifetimes. WebWidget should only exist when using compositing. It should be tied to lifetime of the main frame, almost certainly (and one per frame). At this point WebViewFrameWidget could go away, I beleive, replaced with the split-out WebWidget impl.
Subframes can continue to use WebFrameWidgetImpl, but not clear if that could also be used for the main frame as the impl of WebWidget. Right now, no, but these two things (view and main widget) are tangled up. Maybe if we moved all the view specific things to WebView appropriately it could.
WebViewImpl is both today, but we should give them separate objects and lifetimes. WebWidget should only exist when using compositing. It should be tied to lifetime of the main frame, almost certainly (and one per frame). At this point WebViewFrameWidget could go away, I beleive, replaced with the split-out WebWidget impl.
Subframes can continue to use WebFrameWidgetImpl, but not clear if that could also be used for the main frame as the impl of WebWidget. Right now, no, but these two things (view and main widget) are tangled up. Maybe if we moved all the view specific things to WebView appropriately it could.
re WebViewTest.SetBaseBackgroundColorBeforeMainFrame:
- BackgroundColor() getter should be a view thing
- WidgetMsg_SetBackgroundOpaque should be a ViewMsg
- RenderWidget::OnSetBackgroundOpaque but it only acts for main frames and seems like it wants to persist across main frames like its more like a page prop
re focus vs active
- WidgetMsg_SetActive should be a ViewMsg
- We have SetFocus going to input handler posting to main thread RenderWidget
- We also have page level SetActive
- WebView::SetIsActive vs WebWidget::SetFocused
- WebWidget::SetFocused only sets active on the page, not inactive ??? But always sets SetFocused().
- Can we make a single signal/state instead of 2 ? Is one per-widget and one per-page ??
- Tests do `web_view_->GetPage()->GetFocusController().SetActive(true);` instead of going through the WebView or WebWidget. Does other code do this too ?? oh god. WebPagePopupImpl does. WebFrameWidgetImpl does.
- WebView also has SetInitialFocus() which acts on the main frame. Should possibly be part of WebWidget, though also interacts with Page.
- child local roots don't know about focus correctly, which probably means main frame widgets won't either if we separate them from the view enough. we likely fix both at once. https://bugs.chromium.org/p/chromium/issues/detail?id=689777
re Close
- Close() is on WebWidget, but also closes the WebView
- They should probably each have their own Close once they split.
re Resizing
- Resize() is a WebWidget method, but
- ResizeWithBrowserControls is a WebView method
- Should these both be on the same API? The first is implemented by calling the second.
- WebFrameWidgetImpl Resize code has some "MainFrame" stuff in its names that seems to be incorrect, as it actually just acts on its frame which can be (always is atm) a subframe.
re popups
- GetPagePopup() is part of WebWidget but is only ever called for the main frame, and as per its name, is related to the Page not the Widget.
- Should move GetPagePopup() to WebView.
- As an extra treat WebViewImpl overrides this but changes the return type of GetPagePopup to WebPagePopupImpl - an internal blink type.
re visual viewport
- ResizeVisualViewport() is part of WebWidget but only modifies values on Page, should be WebView?
WebViewImpl is both today, but we should give them separate objects and lifetimes. WebWidget should only exist when using compositing. It should be tied to lifetime of the main frame, almost certainly (and one per frame). At this point WebViewFrameWidget could go away, I beleive, replaced with the split-out WebWidget impl.
Subframes can continue to use WebFrameWidgetImpl, but not clear if that could also be used for the main frame as the impl of WebWidget. Right now, no, but these two things (view and main widget) are tangled up. Maybe if we moved all the view specific things to WebView appropriately it could.
re WebViewTest.SetBaseBackgroundColorBeforeMainFrame:
- BackgroundColor() getter should be a view thing
- WidgetMsg_SetBackgroundOpaque should be a ViewMsg
- RenderWidget::OnSetBackgroundOpaque but it only acts for main frames and seems like it wants to persist across main frames like its more like a page prop
re focus vs active
- WidgetMsg_SetActive should be a ViewMsg
- We have SetFocus going to input handler posting to main thread RenderWidget
- We also have page level SetActive
- WebView::SetIsActive vs WebWidget::SetFocused
- WebWidget::SetFocused only sets active on the page, not inactive ??? But always sets SetFocused().
- Can we make a single signal/state instead of 2 ? Is one per-widget and one per-page ??
- Tests do `web_view_->GetPage()->GetFocusController().SetActive(true);` instead of going through the WebView or WebWidget. Does other code do this too ?? oh god. WebPagePopupImpl does. WebFrameWidgetImpl does.
- WebView also has SetInitialFocus() which acts on the main frame. Should possibly be part of WebWidget, though also interacts with Page.
- child local roots don't know about focus correctly, which probably means main frame widgets won't either if we separate them from the view enough. we likely fix both at once. https://bugs.chromium.org/p/chromium/issues/detail?id=689777
re Close
- Close() is on WebWidget, but also closes the WebView
- They should probably each have their own Close once they split.
re Resizing
- Resize() is a WebWidget method, but
- ResizeWithBrowserControls is a WebView method
- Should these both be on the same API? The first is implemented by calling the second.
- WebFrameWidgetImpl Resize code has some "MainFrame" stuff in its names that seems to be incorrect, as it actually just acts on its frame which can be (always is atm) a subframe.
* re popups
* - GetPagePopup() is part of WebWidget but is only ever called for the main frame, and as per its name, is related to the Page not the Widget.
* - Should move GetPagePopup() to WebView.
re focus con't: https://bugs.chromium.org/p/chromium/issues/detail?id=689777
* - As an extra treat WebViewImpl overrides this but changes the return type of GetPagePopup to WebPagePopupImpl - an internal blink type.
re visual viewport
- ResizeVisualViewport() is part of WebWidget but only modifies values on Page, should be WebView?
* = done
WebViewImpl is both today, but we should give them separate objects and lifetimes. WebWidget should only exist when using compositing. It should be tied to lifetime of the main frame, almost certainly (and one per frame). At this point WebViewFrameWidget could go away, I beleive, replaced with the split-out WebWidget impl.
Subframes can continue to use WebFrameWidgetImpl, but not clear if that could also be used for the main frame as the impl of WebWidget. Right now, no, but these two things (view and main widget) are tangled up. Maybe if we moved all the view specific things to WebView appropriately it could.
re WebViewTest.SetBaseBackgroundColorBeforeMainFrame:
- BackgroundColor() getter should be a view thing
- WidgetMsg_SetBackgroundOpaque should be a ViewMsg
- RenderWidget::OnSetBackgroundOpaque but it only acts for main frames and seems like it wants to persist across main frames like its more like a page prop
re focus vs active
- WidgetMsg_SetActive should be a ViewMsg
- We have SetFocus going to input handler posting to main thread RenderWidget
- We also have page level SetActive
- WebView::SetIsActive vs WebWidget::SetFocused
- WebWidget::SetFocused only sets active on the page, not inactive ??? But always sets SetFocused().
- Can we make a single signal/state instead of 2 ? Is one per-widget and one per-page ??
- Tests do `web_view_->GetPage()->GetFocusController().SetActive(true);` instead of going through the WebView or WebWidget. Does other code do this too ?? oh god. WebPagePopupImpl does. WebFrameWidgetImpl does.
- WebView also has SetInitialFocus() which acts on the main frame. Should possibly be part of WebWidget, though also interacts with Page.
- child local roots don't know about focus correctly, which probably means main frame widgets won't either if we separate them from the view enough. we likely fix both at once. https://bugs.chromium.org/p/chromium/issues/detail?id=689777
re Close
- Close() is on WebWidget, but also closes the WebView
- They should probably each have their own Close once they split.
re Resizing
- Resize() is a WebWidget method, but
- ResizeWithBrowserControls is a WebView method
- Should these both be on the same API? The first is implemented by calling the second.
- WebFrameWidgetImpl Resize code has some "MainFrame" stuff in its names that seems to be incorrect, as it actually just acts on its frame which can be (always is atm) a subframe.
* re popups
* - GetPagePopup() is part of WebWidget but is only ever called for the main frame, and as per its name, is related to the Page not the Widget.
* - Should move GetPagePopup() to WebView.
re focus con't: https://bugs.chromium.org/p/chromium/issues/detail?id=689777
* - As an extra treat WebViewImpl overrides this but changes the return type of GetPagePopup to WebPagePopupImpl - an internal blink type.
re visual viewport
- ResizeVisualViewport() is part of WebWidget but only modifies values on Page, should be WebView?
* = done
WebViewImpl is both today, but we should give them separate objects and lifetimes. WebWidget should only exist when using compositing. It should be tied to lifetime of the main frame, almost certainly (and one per frame). At this point WebViewFrameWidget could go away, I beleive, replaced with the split-out WebWidget impl.
Subframes can continue to use WebFrameWidgetImpl, but not clear if that could also be used for the main frame as the impl of WebWidget. Right now, no, but these two things (view and main widget) are tangled up. Maybe if we moved all the view specific things to WebView appropriately it could.
re WebViewTest.SetBaseBackgroundColorBeforeMainFrame:
- BackgroundColor() getter should be a view thing
- WidgetMsg_SetBackgroundOpaque should be a ViewMsg
- RenderWidget::OnSetBackgroundOpaque but it only acts for main frames and seems like it wants to persist across main frames like its more like a page prop
re focus vs active
- WidgetMsg_SetActive should be a ViewMsg
- We have SetFocus going to input handler posting to main thread RenderWidget
- We also have page level SetActive
- WebView::SetIsActive vs WebWidget::SetFocused
- WebWidget::SetFocused only sets active on the page, not inactive ??? But always sets SetFocused().
- Can we make a single signal/state instead of 2 ? Is one per-widget and one per-page ??
- Tests do `web_view_->GetPage()->GetFocusController().SetActive(true);` instead of going through the WebView or WebWidget. Does other code do this too ?? oh god. WebPagePopupImpl does. WebFrameWidgetImpl does.
- WebView also has SetInitialFocus() which acts on the main frame. Should possibly be part of WebWidget, though also interacts with Page.
- child local roots don't know about focus correctly, which probably means main frame widgets won't either if we separate them from the view enough. we likely fix both at once. https://bugs.chromium.org/p/chromium/issues/detail?id=689777
re Close
- Close() is on WebWidget, but also closes the WebView
- They should probably each have their own Close once they split.
re Resizing
- Resize() is a WebWidget method, but
- ResizeWithBrowserControls is a WebView method
- Should these both be on the same API? The first is implemented by calling the second.
- WebFrameWidgetImpl Resize code has some "MainFrame" stuff in its names that seems to be incorrect, as it actually just acts on its frame which can be (always is atm) a subframe.
* re popups
* - GetPagePopup() is part of WebWidget but is only ever called for the main frame, and as per its name, is related to the Page not the Widget.
* - Should move GetPagePopup() to WebView.
* - As an extra treat WebViewImpl overrides this but changes the return type of GetPagePopup to WebPagePopupImpl - an internal blink type.
re visual viewport
- ResizeVisualViewport() is part of WebWidget but only modifies values on Page, should be WebView?
* = done
WebViewImpl is both today, but we should give them separate objects and lifetimes. WebWidget should only exist when using compositing. It should be tied to lifetime of the main frame, almost certainly (and one per frame). At this point WebViewFrameWidget could go away, I beleive, replaced with the split-out WebWidget impl.
Subframes can continue to use WebFrameWidgetImpl, but not clear if that could also be used for the main frame as the impl of WebWidget. Right now, no, but these two things (view and main widget) are tangled up. Maybe if we moved all the view specific things to WebView appropriately it could.
re WebViewTest.SetBaseBackgroundColorBeforeMainFrame:
- BackgroundColor() getter should be a view thing
- WidgetMsg_SetBackgroundOpaque should be a ViewMsg
- RenderWidget::OnSetBackgroundOpaque but it only acts for main frames and seems like it wants to persist across main frames like its more like a page prop
re focus vs active
- WidgetMsg_SetActive should be a ViewMsg
- We have SetFocus going to input handler posting to main thread RenderWidget
- We also have page level SetActive
- WebView::SetIsActive vs WebWidget::SetFocused
- WebWidget::SetFocused only sets active on the page, not inactive ??? But always sets SetFocused().
- Can we make a single signal/state instead of 2 ? Is one per-widget and one per-page ??
- Tests do `web_view_->GetPage()->GetFocusController().SetActive(true);` instead of going through the WebView or WebWidget. Does other code do this too ?? oh god. WebPagePopupImpl does. WebFrameWidgetImpl does.
- WebView also has SetInitialFocus() which acts on the main frame. Should possibly be part of WebWidget, though also interacts with Page.
- child local roots don't know about focus correctly, which probably means main frame widgets won't either if we separate them from the view enough. we likely fix both at once. https://bugs.chromium.org/p/chromium/issues/detail?id=689777
re Close
- Close() is on WebWidget, but also closes the WebView
- They should probably each have their own Close once they split.
re Resizing
- Resize() is a WebWidget method, but
- ResizeWithBrowserControls is a WebView method
- Should these both be on the same API? The first is implemented by calling the second.
- WebFrameWidgetImpl Resize code has some "MainFrame" stuff in its names that seems to be incorrect, as it actually just acts on its frame which can be (always is atm) a subframe.
* re popups
* - GetPagePopup() is part of WebWidget but is only ever called for the main frame, and as per its name, is related to the Page not the Widget.
* - Should move GetPagePopup() to WebView.
* - As an extra treat WebViewImpl overrides this but changes the return type of GetPagePopup to WebPagePopupImpl - an internal blink type.
re visual viewport
- ResizeVisualViewport() is part of WebWidget but only modifies values on Page, should be WebView?
re cursor visibility
- WebViewImpl::SetCursorVisibilityState() changes page state only but is a WebWidget method.
- It is called from a RenderWidget IPC
* = done
re WebViewTest.SetBaseBackgroundColorBeforeMainFrame:
- BackgroundColor() getter should be a view thing
- WidgetMsg_SetBackgroundOpaque should be a ViewMsg
- RenderWidget::OnSetBackgroundOpaque but it only acts for main frames and seems like it wants to persist across main frames like its more like a page prop
re focus vs active
- WidgetMsg_SetActive should be a ViewMsg
- We have SetFocus going to input handler posting to main thread RenderWidget
- We also have page level SetActive
- WebView::SetIsActive vs WebWidget::SetFocused
- WebWidget::SetFocused only sets active on the page, not inactive ??? But always sets SetFocused().
- Can we make a single signal/state instead of 2 ? Is one per-widget and one per-page ??
- Tests do `web_view_->GetPage()->GetFocusController().SetActive(true);` instead of going through the WebView or WebWidget. Does other code do this too ?? oh god. WebPagePopupImpl does. WebFrameWidgetImpl does.
re focus con't
- WebView also has SetInitialFocus() which acts on the main frame. Should possibly be part of WebWidget, though also interacts with Page.
re Close
- Close() is on WebWidget, but also closes the WebView
- They should probably each have their own Close once they split.
re Resizing
- Resize() is a WebWidget method, but
- ResizeWithBrowserControls is a WebView method
- Should these both be on the same API? The first is implemented by calling the second.
- WebFrameWidgetImpl Resize code has some "MainFrame" stuff in its names that seems to be incorrect, as it actually just acts on its frame which can be (always is atm) a subframe.
re popups
- GetPagePopup() is part of WebWidget but is only ever called for the main frame, and as per its name, is related to the Page not the Widget.
- Should move GetPagePopup() to WebView.
- As an extra treat WebViewImpl overrides this but changes the return type of GetPagePopup to WebPagePopupImpl - an internal blink type.
re visual viewport
- ResizeVisualViewport() is part of WebWidget but only modifies values on Page, should be WebView?
Comment 1 by danakj@chromium.org
, Dec 5