Surface Embed: Support pointer lock [chromium/src : main]
Cammie Smith Barnes (Gerrit)
New activity on the change![]( "Open in Gerrit")
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
gwsq (Gerrit)
Message from gwsq![]( "Open in Gerrit")
Reviewer source(s):
morl...@chromium.org is from context(googleclient/chrome/chromium_gwsq/components/surface_embed/config.gwsq)
Related details
- Maks Orlovich
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Maks Orlovich (Gerrit)
Maks Orlovich added 4 comments![]( "Open in Gerrit")
return static_cast<RenderWidgetHostView*>(GetRootRenderWidgetHostView());...I think you don't actually need this cast, and probably not even this level?
RenderWidgetHostViewBase is a /subclass/ of RenderWidgetHostView.
Also is Root vs. TopLevel supposed to be meaningful?
delegate_->RequestPointerLock(this, user_gesture, last_unlocked_by_target);This didn't actually change delegate_ across the loop? And I think we would want the delegate for the Browser object, and not the webui_browser here?
if (!delegate_ || (pointer_lock_widget_ != render_widget_host)) {And this didn't even bother having a loop.
void WebContentsImpl::SetPointerLockWidgetInParentChain(So high level: does a webpage grabbing a lock limit the top-level in webui_browser it won't in a views browser?
Related details
- Cammie Smith Barnes
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Cammie Smith Barnes (Gerrit)
Cammie Smith Barnes added 1 comment![]( "Open in Gerrit")
return static_cast<RenderWidgetHostView*>(GetRootRenderWidgetHostView());...I think you don't actually need this cast, and probably not even this level?
RenderWidgetHostViewBase is a /subclass/ of RenderWidgetHostView.Also is Root vs. TopLevel supposed to be meaningful?
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Cammie Smith Barnes (Gerrit)
Cammie Smith Barnes voted and added 3 comments![]( "Open in Gerrit")
Votes added by Cammie Smith Barnes
| Commit-Queue | +1 |
3 comments
I see that PS 20 has some test failures that I will now fix...
delegate_->RequestPointerLock(this, user_gesture, last_unlocked_by_target);This didn't actually change delegate_ across the loop? And I think we would want the delegate for the Browser object, and not the webui_browser here?
Does it work if we delegate to the Browser from WebUiBrowserWebContentsDelegate itself? See PS 20 for how I interpreted this. Also I don't quite follow why the delegate_ needs to change across the loop since it doesn't in the existing code...
In any case, I just manually tested pointer lock here with this CL patched in and it's working: https://mdn.github.io/dom-examples/pointer-lock/
void WebContentsImpl::SetPointerLockWidgetInParentChain(So high level: does a webpage grabbing a lock limit the top-level in webui_browser it won't in a views browser?
I am not sure what you mean by "limit"? I suppose it would if our top-chrome UI added a pointer lock button, but since that isn't needed/won't happen, I am not sure it makes a difference.
I manually tested and the behavior for this CL (as of PS 20) seems the same as the behavior in stable Chrome/views browser.
Related details
- Cammie Smith Barnes
- Maks Orlovich
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Maks Orlovich (Gerrit)
Maks Orlovich added 2 comments![]( "Open in Gerrit")
delegate_->RequestPointerLock(this, user_gesture, last_unlocked_by_target);Cammie Smith BarnesThis didn't actually change delegate_ across the loop? And I think we would want the delegate for the Browser object, and not the webui_browser here?
Does it work if we delegate to the Browser from WebUiBrowserWebContentsDelegate itself? See PS 20 for how I interpreted this. Also I don't quite follow why the delegate_ needs to change across the loop since it doesn't in the existing code...
In any case, I just manually tested pointer lock here with this CL patched in and it's working: https://mdn.github.io/dom-examples/pointer-lock/
I think what I meant is that the original implementation just asks delegate_, even if it's nested in an outer WebContents while you pull it from the root via GetRootWebContentsDelegate, which surprised me.
void WebContentsImpl::SetPointerLockWidgetInParentChain(Cammie Smith BarnesSo high level: does a webpage grabbing a lock limit the top-level in webui_browser it won't in a views browser?
I am not sure what you mean by "limit"? I suppose it would if our top-chrome UI added a pointer lock button, but since that isn't needed/won't happen, I am not sure it makes a difference.
I manually tested and the behavior for this CL (as of PS 20) seems the same as the behavior in stable Chrome/views browser.
If a webpage grabs a pointer lock, and you're in webui_browser (not a Views browser), does it mess with interacting with, say, the close tab button?
Related details
- Cammie Smith Barnes
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Cammie Smith Barnes (Gerrit)
Cammie Smith Barnes added 4 comments![]( "Open in Gerrit")
I see that PS 20 has some test failures that I will now fix...
Still working on the flaky tests...
delegate_->RequestPointerLock(this, user_gesture, last_unlocked_by_target);Cammie Smith BarnesThis didn't actually change delegate_ across the loop? And I think we would want the delegate for the Browser object, and not the webui_browser here?
Maks OrlovichDoes it work if we delegate to the Browser from WebUiBrowserWebContentsDelegate itself? See PS 20 for how I interpreted this. Also I don't quite follow why the delegate_ needs to change across the loop since it doesn't in the existing code...
In any case, I just manually tested pointer lock here with this CL patched in and it's working: https://mdn.github.io/dom-examples/pointer-lock/
I think what I meant is that the original implementation just asks delegate_, even if it's nested in an outer WebContents while you pull it from the root via GetRootWebContentsDelegate, which surprised me.
After doing some research with Gemini:
Hi Maks,
Thanks for the clarification. You're right that the original implementation just used delegate_ . However, that was a limitation that made pointer lock unsupported for nested WebContents (other than GuestView).
There are three ways to nest WebContents where the content layer has awareness of the hierarchy, and our patch (as of PS 23) handles each of them (as well as the non-nested case) correctly:
1. Classic GuestView: The guest WebContents has a specialized delegate ( WebViewGuest ) that implements RequestPointerLock to perform extension-specific permission checks. Bypassing it would be incorrect.
• Our approach: GetBrowserPluginGuest() is non-null, so we use delegate_.get() , preserving the original behavior and custom permission checks.
2. GuestContents: Uses standard nesting ( GetOuterWebContents() ), but the guest WebContents does not have a delegate set.
• Our approach: GetBrowserPluginGuest() is null, so we use GetRootWebContentsDelegate() . This traverses up the nesting chain via GetOuterWebContents() to the root delegate (the Browser window via WebUIBrowserWebContentsDelegate ). In the original code, this would have failed (rejected with kWrongDocument ) because delegate_ was null.
3. SurfaceEmbed: Uses SurfaceEmbedConnector , and also does not have a delegate set on the inner WebContents .
• Our approach: GetBrowserPluginGuest() is null, so we use GetRootWebContentsDelegate() . This traverses up via GetSurfaceEmbedConnector() to find the root delegate.
4. Non-nested WebContents:
• Our approach: GetRootWebContentsDelegate() simply returns delegate_ (since there are no outer WebContents or connectors), matching the original behavior.
So, using GetRootWebContentsDelegate() is necessary to support pointer lock for embedded WebContents that don't have their own specialized delegates, while the special case for BrowserPluginGuest ensures we don't break GuestView.
if (!delegate_ || (pointer_lock_widget_ != render_widget_host)) {And this didn't even bother having a loop.
Acknowledged, but we're not using a loop either...
void WebContentsImpl::SetPointerLockWidgetInParentChain(Cammie Smith BarnesSo high level: does a webpage grabbing a lock limit the top-level in webui_browser it won't in a views browser?
Maks OrlovichI am not sure what you mean by "limit"? I suppose it would if our top-chrome UI added a pointer lock button, but since that isn't needed/won't happen, I am not sure it makes a difference.
I manually tested and the behavior for this CL (as of PS 20) seems the same as the behavior in stable Chrome/views browser.
If a webpage grabs a pointer lock, and you're in webui_browser (not a Views browser), does it mess with interacting with, say, the close tab button?
Yes, it stops one from being able to close tabs unless ESC is pressed to quit the pointer lock (or unless a keyboard shortcut is used to close the tab). But that is also true in the Views browser.
Related details
- Maks Orlovich
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Maks Orlovich (Gerrit)
Maks Orlovich added 2 comments![]( "Open in Gerrit")
delegate_->RequestPointerLock(this, user_gesture, last_unlocked_by_target);Cammie Smith BarnesThis didn't actually change delegate_ across the loop? And I think we would want the delegate for the Browser object, and not the webui_browser here?
Maks OrlovichDoes it work if we delegate to the Browser from WebUiBrowserWebContentsDelegate itself? See PS 20 for how I interpreted this. Also I don't quite follow why the delegate_ needs to change across the loop since it doesn't in the existing code...
In any case, I just manually tested pointer lock here with this CL patched in and it's working: https://mdn.github.io/dom-examples/pointer-lock/
Cammie Smith BarnesI think what I meant is that the original implementation just asks delegate_, even if it's nested in an outer WebContents while you pull it from the root via GetRootWebContentsDelegate, which surprised me.
After doing some research with Gemini:
Hi Maks,
Thanks for the clarification. You're right that the original implementation just used delegate_ . However, that was a limitation that made pointer lock unsupported for nested WebContents (other than GuestView).
There are three ways to nest WebContents where the content layer has awareness of the hierarchy, and our patch (as of PS 23) handles each of them (as well as the non-nested case) correctly:
1. Classic GuestView: The guest WebContents has a specialized delegate ( WebViewGuest ) that implements RequestPointerLock to perform extension-specific permission checks. Bypassing it would be incorrect.
• Our approach: GetBrowserPluginGuest() is non-null, so we use delegate_.get() , preserving the original behavior and custom permission checks.
2. GuestContents: Uses standard nesting ( GetOuterWebContents() ), but the guest WebContents does not have a delegate set.
• Our approach: GetBrowserPluginGuest() is null, so we use GetRootWebContentsDelegate() . This traverses up the nesting chain via GetOuterWebContents() to the root delegate (the Browser window via WebUIBrowserWebContentsDelegate ). In the original code, this would have failed (rejected with kWrongDocument ) because delegate_ was null.
3. SurfaceEmbed: Uses SurfaceEmbedConnector , and also does not have a delegate set on the inner WebContents .
• Our approach: GetBrowserPluginGuest() is null, so we use GetRootWebContentsDelegate() . This traverses up via GetSurfaceEmbedConnector() to find the root delegate.
4. Non-nested WebContents:
• Our approach: GetRootWebContentsDelegate() simply returns delegate_ (since there are no outer WebContents or connectors), matching the original behavior.
So, using GetRootWebContentsDelegate() is necessary to support pointer lock for embedded WebContents that don't have their own specialized delegates, while the special case for BrowserPluginGuest ensures we don't break GuestView.
Interesting about GuestView, but what if the guest view is some levels of nested embedding away?
I also don't think (2)/(3) is true. Stuff won't work very well w/o WebContentsDelegate, and I would expect the guest webcontents in webui_browser to have the WebContentsDelegate to be set to Browser, 'cause that's basically what happens when tabs are created.
void WebContentsImpl::SetPointerLockWidgetInParentChain(Cammie Smith BarnesSo high level: does a webpage grabbing a lock limit the top-level in webui_browser it won't in a views browser?
Maks OrlovichI am not sure what you mean by "limit"? I suppose it would if our top-chrome UI added a pointer lock button, but since that isn't needed/won't happen, I am not sure it makes a difference.
I manually tested and the behavior for this CL (as of PS 20) seems the same as the behavior in stable Chrome/views browser.
Cammie Smith BarnesIf a webpage grabs a pointer lock, and you're in webui_browser (not a Views browser), does it mess with interacting with, say, the close tab button?
Yes, it stops one from being able to close tabs unless ESC is pressed to quit the pointer lock (or unless a keyboard shortcut is used to close the tab). But that is also true in the Views browser.
- Cammie Smith Barnes
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Cammie Smith Barnes (Gerrit)
Cammie Smith Barnes added 1 comment![]( "Open in Gerrit")
Changed to using GetFirstWebContentsDelegate(). Does this look better?
Related details
- Maks Orlovich
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Maks Orlovich (Gerrit)
Maks Orlovich voted and added 5 comments![]( "Open in Gerrit")
Votes added by Maks Orlovich
| Code-Review | +1 |
5 comments
LOG(INFO) << "Pointer lock result: " << result.ExtractString();EXPECT_EQ will print it anyway if it fails?
if (result.ExtractString() == "success") {ASSERT_EQ instead of EXPECT_EQ above won't continue if it failed...
return GetRenderManager()->GetRenderWidgetHostView();Hmm, changing this may be a bit against the security goal of trying to make every use of Surface Embed explicit for purpose (not sure we're still sticking to that.)
Yeah, that makes sense (though I am not sure why null delegates would be more relevant here than before)
if (!delegate_ || (pointer_lock_widget_ != render_widget_host)) {Cammie Smith BarnesAnd this didn't even bother having a loop.
Acknowledged, but we're not using a loop either...
- Cammie Smith Barnes
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Cammie Smith Barnes (Gerrit)
Cammie Smith Barnes added 1 comment![]( "Open in Gerrit")
return GetRenderManager()->GetRenderWidgetHostView();Hmm, changing this may be a bit against the security goal of trying to make every use of Surface Embed explicit for purpose (not sure we're still sticking to that.)
What if I altered WebContentsImpl::HasPointerLock() and WebContentsImpl::GetPointerLockWidget() to use new counterparts in SurfaceEmbedConnectorImpl with the same method names in the case where there is a connector? and then leave this method unchanged? Would that fix this issue by making SurfaceEmbed use explicit enough?
Related details
- Maks Orlovich
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Maks Orlovich (Gerrit)
Maks Orlovich voted and added 1 comment![]( "Open in Gerrit")
Votes added by Maks Orlovich
| Code-Review | +1 |
1 comment
return GetRenderManager()->GetRenderWidgetHostView();Cammie Smith BarnesHmm, changing this may be a bit against the security goal of trying to make every use of Surface Embed explicit for purpose (not sure we're still sticking to that.)
What if I altered WebContentsImpl::HasPointerLock() and WebContentsImpl::GetPointerLockWidget() to use new counterparts in SurfaceEmbedConnectorImpl with the same method names in the case where there is a connector? and then leave this method unchanged? Would that fix this issue by making SurfaceEmbed use explicit enough?
- Cammie Smith Barnes
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Cammie Smith Barnes (Gerrit)
Cammie Smith Barnes added 4 comments![]( "Open in Gerrit")
LOG(INFO) << "Pointer lock result: " << result.ExtractString();EXPECT_EQ will print it anyway if it fails?
Done
if (result.ExtractString() == "success") {ASSERT_EQ instead of EXPECT_EQ above won't continue if it failed...
Done
return GetRenderManager()->GetRenderWidgetHostView();Cammie Smith BarnesHmm, changing this may be a bit against the security goal of trying to make every use of Surface Embed explicit for purpose (not sure we're still sticking to that.)
Maks OrlovichWhat if I altered WebContentsImpl::HasPointerLock() and WebContentsImpl::GetPointerLockWidget() to use new counterparts in SurfaceEmbedConnectorImpl with the same method names in the case where there is a connector? and then leave this method unchanged? Would that fix this issue by making SurfaceEmbed use explicit enough?
It would.
Done
delegate_->RequestPointerLock(this, user_gesture, last_unlocked_by_target);Acknowledged, marking as resolved. Is that OK?
Related details
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Maks Orlovich (Gerrit)
Maks Orlovich voted Code-Review+1![]( "Open in Gerrit")
- Cammie Smith Barnes
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Cammie Smith Barnes (Gerrit)
Cammie Smith Barnes added 2 comments![]( "Open in Gerrit")
Cammie Smith BarnesI see that PS 20 has some test failures that I will now fix...
Still working on the flaky tests...
- Cammie Smith Barnes
- Nasko Oskov
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nasko Oskov (Gerrit)
Nasko Oskov added 5 comments![]( "Open in Gerrit")
• HasFocus() : We implement it to check the actual focus state of the root view. If the root view has focus, the child view is considered focused, allowing CanBePointerLocked() to return true.nit: break long lines
if (pointer_lock_widget_) {Do we need to do this cleanup along the WebContents embedding chain now that we support setting pointer lock state in the hierarchy?
current = current->GetOuterWebContents()) {This loop doesn't account for surface embeds, does it? Should it be doing `SetPointerLockWidgetInParentChain(nullptr)`?
if (WebContentsImpl* outer = GetOuterWebContents()) {In other methods, surface embeds are explicitly checked. Why is that not the case here? If it is intentional, maybe worth a comment to explain to people reading the code.
if (delegate_) {Is it actually possible to have WebContents without `delegate_` outside of tests? If so, I think it is useful to have a comment here explaining what are those cases.
Related details
- Cammie Smith Barnes
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Cammie Smith Barnes (Gerrit)
Cammie Smith Barnes added 1 comment![]( "Open in Gerrit")
• HasFocus() : We implement it to check the actual focus state of the root view. If the root view has focus, the child view is considered focused, allowing CanBePointerLocked() to return true.Cammie Smith Barnesnit: break long lines
Done
Related details
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Cammie Smith Barnes (Gerrit)
Cammie Smith Barnes added 1 comment![]( "Open in Gerrit")
In other methods, surface embeds are explicitly checked. Why is that not the case here? If it is intentional, maybe worth a comment to explain to people reading the code.
Done
Related details
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Cammie Smith Barnes (Gerrit)
Cammie Smith Barnes added 1 comment![]( "Open in Gerrit")
current = current->GetOuterWebContents()) {Cammie Smith BarnesThis loop doesn't account for surface embeds, does it? Should it be doing `SetPointerLockWidgetInParentChain(nullptr)`?
Done
Related details
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Cammie Smith Barnes (Gerrit)
Cammie Smith Barnes added 1 comment![]( "Open in Gerrit")
Do we need to do this cleanup along the WebContents embedding chain now that we support setting pointer lock state in the hierarchy?
Done
Cammie Smith Barnes (Gerrit)
Cammie Smith Barnes added 1 comment![]( "Open in Gerrit")
Is it actually possible to have WebContents without `delegate_` outside of tests? If so, I think it is useful to have a comment here explaining what are those cases.
Yes, it is indeed possible to have a WebContents without a delegate_ outside of tests. This occurs primarily during transition phases in the lifecycle of a WebContents :
1. Initialization: Right after a WebContents is created (e.g., via WebContents::Create ), it initially has no delegate. The creator (like Browser or TabAndroid ) typically sets itself as the delegate
shortly after.
2. Tear down/Destruction: In web_contents_impl.cc, SetDelegate(nullptr) is called to detach the delegate before the WebContents is fully destroyed.
3. Ownership Transfers: During certain navigation flows, like prerendering in a new tab (prerender_new_tab_handle.cc), the WebContents delegate is set to nullptr before being returned to the caller,
which will then
attach it to a new delegate (e.g., a browser window).
4. Pending Windows: When a new window is created with the opener not suppressed, the newly created WebContents is stored in pending_contents_ and may not have a delegate set until it is actually shown
or attached.
Related details
- Nasko Oskov
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nasko Oskov (Gerrit)
Nasko Oskov voted and added 1 comment![]( "Open in Gerrit")
- Cammie Smith Barnes
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Cammie Smith Barnes (Gerrit)
Cammie Smith Barnes added 1 comment![]( "Open in Gerrit")
- Fred Shih
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Fred Shih (Gerrit)
Fred Shih voted Code-Review+1![]( "Open in Gerrit")
- Cammie Smith Barnes
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Cammie Smith Barnes (Gerrit)
Cammie Smith Barnes voted Commit-Queue+2![]( "Open in Gerrit")
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Cammie Smith Barnes (Gerrit)
Cammie Smith Barnes added 8 comments![]( "Open in Gerrit")
#include "ui/views/test/widget_activation_waiter.h"need to remove
#include "base/logging.h"need to remove
#include "base/logging.h"need to remove
#include "base/logging.h"need to remove
#include "base/logging.h"need to remove
#include "base/logging.h"need to remove
#include "base/logging.h"need to remove
Related details
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Cammie Smith Barnes (Gerrit)
Cammie Smith Barnes added 9 comments![]( "Open in Gerrit")
#include "ui/views/test/widget_activation_waiter.h"Cammie Smith Barnesneed to remove
Done
#include "base/logging.h"Cammie Smith Barnesneed to remove
Done
CrossProcessFrameConnector::GetRootRenderWidgetHostView() const {the const should not have been introduced here in this CL unless it was absolutely necessary. is it necessary? or is it a drive-by fix independent of the CL. if the latter, please revert it.
#include "base/logging.h"Cammie Smith Barnesneed to remove
Done
#include "base/logging.h"Cammie Smith Barnesneed to remove
Done
#include "base/logging.h"Cammie Smith Barnesneed to remove
Done
#include "base/logging.h"Cammie Smith Barnesneed to remove
Done
#include "base/logging.h"Cammie Smith Barnesneed to remove
Done
Related details
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Cammie Smith Barnes (Gerrit)
Cammie Smith Barnes voted and added 1 comment![]( "Open in Gerrit")
Votes added by Cammie Smith Barnes
| Commit-Queue | +0 |
1 comment
CrossProcessFrameConnector::GetRootRenderWidgetHostView() const {the const should not have been introduced here in this CL unless it was absolutely necessary. is it necessary? or is it a drive-by fix independent of the CL. if the latter, please revert it.
Done
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |