Add --expose-internals-for-testing flag

Add --expose-internals-for-testing flag

This can be handy for interactive design and debugging complex layout tests that
need access to window.internals.

BUG=None

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

[Rick Byers, 2013-06-26 14:49:19]

Hey jam,
Is this simple additional flag OK with you?  I had been just keeping this hack
locally for development, but another engineer recently told me it would have
saved him a bunch of time if he had known there was an easy way to run
content_shell interactively with window.internals exposed.

Thanks,
   Rick

[jam, 2013-06-26 16:46:16]

lgtm

https://codereview.chromium.org/17895003/diff/1/content/public/common/content...
File content/public/common/content_switches.h (right):

https://codereview.chromium.org/17895003/diff/1/content/public/common/content...
content/public/common/content_switches.h:43: extern const char
kExposeInternalsForTesting[];
nit: can you try to sort this (i realize the file isn't really sorted around the
disable/enable parts), but looks like around line 155 is better

https://codereview.chromium.org/17895003/diff/1/content/renderer/render_view_...
File content/renderer/render_view_impl.cc (right):

https://codereview.chromium.org/17895003/diff/1/content/renderer/render_view_...
content/renderer/render_view_impl.cc:3839:
switches::kExposeInternalsForTesting))
nit: indent 4 spaces here. also i think in general when the if condition has two
lines brace brackets are used?

[Rick Byers, 2013-06-26 17:32:04]

Thanks!

https://codereview.chromium.org/17895003/diff/1/content/public/common/content...
File content/public/common/content_switches.h (right):

https://codereview.chromium.org/17895003/diff/1/content/public/common/content...
content/public/common/content_switches.h:43: extern const char
kExposeInternalsForTesting[];
On 2013/06/26 16:46:16, jam wrote:
> nit: can you try to sort this (i realize the file isn't really sorted around
the
> disable/enable parts), but looks like around line 155 is better

Done.

It looks like there are two separate sort-of-sorted lists here (I was aiming for
inserting between k*ExperimentalWebGL and k*Flash3d), but 155 is indeed better.

https://codereview.chromium.org/17895003/diff/1/content/renderer/render_view_...
File content/renderer/render_view_impl.cc (right):

https://codereview.chromium.org/17895003/diff/1/content/renderer/render_view_...
content/renderer/render_view_impl.cc:3839:
switches::kExposeInternalsForTesting))
On 2013/06/26 16:46:16, jam wrote:
> nit: indent 4 spaces here. 

It is 4 spaces already, right?

> also i think in general when the if condition has two
> lines brace brackets are used?

Yeah I tried to find this in the style guide but failed.  Anyway done.

[jam, 2013-06-26 17:38:11]

https://codereview.chromium.org/17895003/diff/1/content/renderer/render_view_...
File content/renderer/render_view_impl.cc (right):

https://codereview.chromium.org/17895003/diff/1/content/renderer/render_view_...
content/renderer/render_view_impl.cc:3839:
switches::kExposeInternalsForTesting))
On 2013/06/26 17:32:05, Rick Byers wrote:
> On 2013/06/26 16:46:16, jam wrote:
> > nit: indent 4 spaces here. 
> 
> It is 4 spaces already, right?

i mean more indentation, so that it's four spaces after "CommandLine" begins
(double checked with a readability reviewer :) )

> 
> > also i think in general when the if condition has two
> > lines brace brackets are used?
> 
> Yeah I tried to find this in the style guide but failed.  Anyway done.

[Rick Byers, 2013-06-26 20:39:25]

It turns out that for non-component builds this WebKitTestingSupport I'm calling
into is only linked with content_shell (I'd been using it in both chrome and
content_shell but with component builds where it was always available).  I've
reworked the CL to be a content_shell-specific flag - that's where most of the
benefit is anyway.  PTAL. 

On 2013/06/26 17:38:11, jam wrote:
>
https://codereview.chromium.org/17895003/diff/1/content/renderer/render_view_...
> File content/renderer/render_view_impl.cc (right):
> 
>
https://codereview.chromium.org/17895003/diff/1/content/renderer/render_view_...
> content/renderer/render_view_impl.cc:3839:
> switches::kExposeInternalsForTesting))
> On 2013/06/26 17:32:05, Rick Byers wrote:
> > On 2013/06/26 16:46:16, jam wrote:
> > > nit: indent 4 spaces here. 
> > 
> > It is 4 spaces already, right?
> 
> i mean more indentation, so that it's four spaces after "CommandLine" begins
> (double checked with a readability reviewer :) )

Ah, sorry - done.
 
> > 
> > > also i think in general when the if condition has two
> > > lines brace brackets are used?
> > 
> > Yeah I tried to find this in the style guide but failed.  Anyway done.

[Rick Byers, 2013-07-01 12:17:35]

On 2013/06/26 20:39:25, Rick Byers wrote:
> It turns out that for non-component builds this WebKitTestingSupport I'm
calling
> into is only linked with content_shell (I'd been using it in both chrome and
> content_shell but with component builds where it was always available).  I've
> reworked the CL to be a content_shell-specific flag - that's where most of the
> benefit is anyway.  PTAL. 

Ping

> 
> On 2013/06/26 17:38:11, jam wrote:
> >
>
https://codereview.chromium.org/17895003/diff/1/content/renderer/render_view_...
> > File content/renderer/render_view_impl.cc (right):
> > 
> >
>
https://codereview.chromium.org/17895003/diff/1/content/renderer/render_view_...
> > content/renderer/render_view_impl.cc:3839:
> > switches::kExposeInternalsForTesting))
> > On 2013/06/26 17:32:05, Rick Byers wrote:
> > > On 2013/06/26 16:46:16, jam wrote:
> > > > nit: indent 4 spaces here. 
> > > 
> > > It is 4 spaces already, right?
> > 
> > i mean more indentation, so that it's four spaces after "CommandLine" begins
> > (double checked with a readability reviewer :) )
> 
> Ah, sorry - done.
>  
> > > 
> > > > also i think in general when the if condition has two
> > > > lines brace brackets are used?
> > > 
> > > Yeah I tried to find this in the style guide but failed.  Anyway done.

[jam, 2013-07-02 16:43:26]

lgtm

[commit-bot: I haz the power, 2013-07-02 16:54:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rbyers@chromium.org/17895003/22001

[Rick Byers, 2013-07-02 16:55:14]

On 2013/07/02 16:43:26, jam wrote:
> lgtm

Thanks!  Sorry I didn't notice the link issue before asking you to review.

[commit-bot: I haz the power, 2013-07-02 18:54:27]

Change committed as 209740

[eseidel, 2013-11-05 22:03:55]

Stumbled across this reading beverloo's weekly updates.  Very interesting.

[Rick Byers, 2013-11-05 22:27:42]

Thanks.  "interesting" as in "valuable" or as in "scary" <grin>?

I've occasionally browsed to some test site with the flag set and wondered
how much risk of attack there was if the site wanted to be malicious ;-)


On Tue, Nov 5, 2013 at 5:03 PM, <eseidel@chromium.org> wrote:

> Stumbled across this reading beverloo's weekly updates.  Very interesting.
>
> https://chromiumcodereview.appspot.com/17895003/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.