split webkit/glue/webpreferences to common and renderer

split webkit/glue/webpreferences to common and renderer

TBR=jamesr@chromium.org

BUG=239107
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=203261

[scottmg, 2013-05-30 20:15:31]

renderer/webpreferences_renderer.cc is semi-sketchy (uses
common/webpreferences.h as its header, and contains ApplyWebPreferences). ctor
in common/webpreferences.cc

[jamesr, 2013-05-30 20:24:23]

https://codereview.chromium.org/16216007/diff/2001/webkit/common/webpreferenc...
File webkit/common/webpreferences.h (right):

https://codereview.chromium.org/16216007/diff/2001/webkit/common/webpreferenc...
webkit/common/webpreferences.h:50: WEBKIT_GLUE_EXPORT void
ApplyWebPreferences(const WebPreferences& prefs,
could we just move this function declaration to a header that lives in
webkit/renderer/ to avoid the sketchy? with this arrangement, code that's in the
browser process can see webkit/common/webpreferences.h and see the declaration
of this function, but it can't see the definition of the symbol so it'll be a
link error if it tries to use it. that seems a little subtle. the function
itself is just a free function

[scottmg, 2013-05-30 21:03:51]

https://codereview.chromium.org/16216007/diff/2001/webkit/common/webpreferenc...
File webkit/common/webpreferences.h (right):

https://codereview.chromium.org/16216007/diff/2001/webkit/common/webpreferenc...
webkit/common/webpreferences.h:50: WEBKIT_GLUE_EXPORT void
ApplyWebPreferences(const WebPreferences& prefs,
On 2013/05/30 20:24:23, jamesr wrote:
> could we just move this function declaration to a header that lives in
> webkit/renderer/ to avoid the sketchy? with this arrangement, code that's in
the
> browser process can see webkit/common/webpreferences.h and see the declaration
> of this function, but it can't see the definition of the symbol so it'll be a
> link error if it tries to use it. that seems a little subtle. the function
> itself is just a free function

Done.

[jamesr, 2013-05-30 21:17:37]

lgtm. you're gonna get some merge conflicts, though

https://codereview.chromium.org/16216007/diff/15001/chrome/renderer/DEPS
File chrome/renderer/DEPS (right):

https://codereview.chromium.org/16216007/diff/15001/chrome/renderer/DEPS#newc...
chrome/renderer/DEPS:17: "+webkit/common",
If you're adding this remove the "webkit/common/fileapi" line on the next line,
it's redundant

https://codereview.chromium.org/16216007/diff/15001/ppapi/shared_impl/DEPS
File ppapi/shared_impl/DEPS (right):

https://codereview.chromium.org/16216007/diff/15001/ppapi/shared_impl/DEPS#ne...
ppapi/shared_impl/DEPS:6: "+webkit/common",
probably better to list out webpreferences.h explicitly so we know what's
allowed

https://codereview.chromium.org/16216007/diff/15001/ppapi/shared_impl/DEPS#ne...
ppapi/shared_impl/DEPS:7: "+webkit/glue",
nuke this line

https://codereview.chromium.org/16216007/diff/15001/webkit/renderer/webprefer...
File webkit/renderer/webpreferences_renderer.cc (right):

https://codereview.chromium.org/16216007/diff/15001/webkit/renderer/webprefer...
webkit/renderer/webpreferences_renderer.cc:8: #include
"third_party/WebKit/Source/Platform/chromium/public/WebString.h"
FYI abarth recently replaced
"third_party/WebKit/Source/Platform/chromium/public" with
"third_party/WebKit/public/platform" across the repo, so this stuff will
conflict and have to be updated.

[scottmg, 2013-05-30 21:23:57]

https://codereview.chromium.org/16216007/diff/15001/chrome/renderer/DEPS
File chrome/renderer/DEPS (right):

https://codereview.chromium.org/16216007/diff/15001/chrome/renderer/DEPS#newc...
chrome/renderer/DEPS:17: "+webkit/common",
On 2013/05/30 21:17:37, jamesr wrote:
> If you're adding this remove the "webkit/common/fileapi" line on the next
line,
> it's redundant

Done.

https://codereview.chromium.org/16216007/diff/15001/ppapi/shared_impl/DEPS
File ppapi/shared_impl/DEPS (right):

https://codereview.chromium.org/16216007/diff/15001/ppapi/shared_impl/DEPS#ne...
ppapi/shared_impl/DEPS:6: "+webkit/common",
On 2013/05/30 21:17:37, jamesr wrote:
> probably better to list out webpreferences.h explicitly so we know what's
> allowed

Done.

https://codereview.chromium.org/16216007/diff/15001/ppapi/shared_impl/DEPS#ne...
ppapi/shared_impl/DEPS:7: "+webkit/glue",
On 2013/05/30 21:17:37, jamesr wrote:
> nuke this line

Done.

https://codereview.chromium.org/16216007/diff/15001/webkit/renderer/webprefer...
File webkit/renderer/webpreferences_renderer.cc (right):

https://codereview.chromium.org/16216007/diff/15001/webkit/renderer/webprefer...
webkit/renderer/webpreferences_renderer.cc:8: #include
"third_party/WebKit/Source/Platform/chromium/public/WebString.h"
On 2013/05/30 21:17:37, jamesr wrote:
> FYI abarth recently replaced
> "third_party/WebKit/Source/Platform/chromium/public" with
> "third_party/WebKit/public/platform" across the repo, so this stuff will
> conflict and have to be updated.

hmm, i just pulled. i guess it hasn't rolled yet?

[jamesr, 2013-05-30 21:26:06]

>
https://codereview.chromium.org/16216007/diff/15001/webkit/renderer/webprefer...
> File webkit/renderer/webpreferences_renderer.cc (right):
> 
>
https://codereview.chromium.org/16216007/diff/15001/webkit/renderer/webprefer...
> webkit/renderer/webpreferences_renderer.cc:8: #include
> "third_party/WebKit/Source/Platform/chromium/public/WebString.h"
> On 2013/05/30 21:17:37, jamesr wrote:
> > FYI abarth recently replaced
> > "third_party/WebKit/Source/Platform/chromium/public" with
> > "third_party/WebKit/public/platform" across the repo, so this stuff will
> > conflict and have to be updated.
> 
> hmm, i just pulled. i guess it hasn't rolled yet?

https://src.chromium.org/viewvc/chrome?revision=203020&view=revision

did this:
https://src.chromium.org/viewvc/chrome/trunk/src/webkit/glue/webpreferences.c...

so you should probably do that as well in this file

[scottmg, 2013-05-30 21:33:05]

On 2013/05/30 21:26:06, jamesr wrote:
> >
>
https://codereview.chromium.org/16216007/diff/15001/webkit/renderer/webprefer...
> > File webkit/renderer/webpreferences_renderer.cc (right):
> > 
> >
>
https://codereview.chromium.org/16216007/diff/15001/webkit/renderer/webprefer...
> > webkit/renderer/webpreferences_renderer.cc:8: #include
> > "third_party/WebKit/Source/Platform/chromium/public/WebString.h"
> > On 2013/05/30 21:17:37, jamesr wrote:
> > > FYI abarth recently replaced
> > > "third_party/WebKit/Source/Platform/chromium/public" with
> > > "third_party/WebKit/public/platform" across the repo, so this stuff will
> > > conflict and have to be updated.
> > 
> > hmm, i just pulled. i guess it hasn't rolled yet?
> 
> https://src.chromium.org/viewvc/chrome?revision=203020&view=revision
> 
> did this:
>
https://src.chromium.org/viewvc/chrome/trunk/src/webkit/glue/webpreferences.c...
> 
> so you should probably do that as well in this file

ah, thanks. was confused by the forwarding headers.

[scottmg, 2013-05-30 23:03:45]

tbr darin for header renames in chrome/ and content/

[joth, 2013-05-30 23:04:42]

a_w/ lgtm

[scottmg, 2013-05-30 23:13:20]

Committed patchset #6 manually as r203261 (presubmit successful).