Set up a Shell class entry for Cobalt #4612
Conversation
kaidokert
force-pushed
the
scaffold_shell
branch
2 times, most recently
from
57861d7 to
80ba1a4
Compare
kaidokert
force-pushed
the
scaffold_shell
branch
2 times, most recently
from
a8184a2 to
09ee019
Compare
kaidokert
changed the title
| @@ -108,6 +108,14 @@ Shell* Shell::CreateShell(std::unique_ptr<WebContents> web_contents, | |||
| bool should_set_delegate) { | |||
| WebContents* raw_web_contents = web_contents.get(); | |||
| Shell* shell = new Shell(std::move(web_contents), should_set_delegate); | |||
| #if BUILDFLAG(IS_COBALT) | |||
There was a problem hiding this comment.
@andrewsavage1 i'd expect you to comment on this - it's really ugly, but the patch/diff illustrates intent clearly.
I can make this little less ugly by doing a bit more elaborate #if oldcode #else hackedcode block maybe.
I'd also hope this doesn't have to be very long lived.
There was a problem hiding this comment.
Further in the future, what do we want to happen with references to content::shell::Shell already in Chromium code? Do we want them to reference cobalt::Shell or are we okay with them not changing?
I'm not sure why we want to continue to use this class at all instead of fully copying it over into cobalt_shell—isn't that the end goal?
There was a problem hiding this comment.
The end goal is not to rely on content::shell:Shell at all. I put this in as a reasonably self-contained incremental step to allow us to extend, without having to keep modifying the code in content/shell directly.
I'm honestly not sure how many direct references there would remain to content::shell::Shell - it's sort of a "leaf" in the dependency chain. In our own copy of the code we would just have CobaltShell or such.
| @@ -0,0 +1,33 @@ | |||
| // Copyright 2013 The Chromium Authors | |||
There was a problem hiding this comment.
Is this file needed? which part is changes after it is copied from content/shell/browser?
There was a problem hiding this comment.
Technically it is not currently needed. I only copied it over because all of those 4 are grouped together in gn in a single block.
I'm expecting we may need to override our own Delegate soon however, too.
kaidokert
force-pushed
the
scaffold_shell
branch
from
5b5b965 to
3f55dc1
Compare
Sets up a Cobalt specific Shell class and necessary wiring to provide an instance of it at startup. It's currently delegating most of the functionality to
content::Shell.content::Shellitself is currently hacked, as it's not designed or intended for sub-classing. This should be cleaned up in the future and Cobalt should fully own all of the related code to set up the shell.The main immediate use is to be able to override
WebContentsObserverevents, likePrimaryMainDocumentElementAvailable.b/384979128
b/390021478