Re: [chromium-dev] Re: opening local files with chrome from command line, relative paths

[Peter Kasting]

On Mon, Jan 18, 2010 at 4:54 PM, Dirk Pranke  wrote:

> So, you then get the following algorithm:
>
> 1) if there is a ':' in the URI, you split the URI into scheme and
> scheme-specific part.
>
> 2) If there is a scheme:
>
>  2.1) If the scheme is a recognized/supported one, dispatch the URL
> as you would normally.
>
>  2.2) If scheme matches [a-zA-Z] and you are on Windows, treat as an
> absolute local file URL
>
>  2.3) Else, treat this as a syntax error in the URI
>
> 3) If there is no scheme:
>
>  3.1) If the URI starts with a "/", treat it as a full path relative
> to the current context (e.g., current scheme, host and port. If your
> current context is a local filesystem, then treat it as a file://
> scheme
>
>  3.2) If the URI starts with a "\", you're on Windows, and the
> context is a local file system point, repeat as above, prepending with
> the current drive
>
>  3.3) If the URI doesn't start with a "/" or a "\", then, optionally,
> check to see if the URI resolves to a valid hostname. This catches the
> "chrome.exe www.google.com" use case
>
>  3.4) If the URI doesn't resolve to a valid hostname, then interpret
> it as a relative URL


I'd pretty strongly like to not specify steps like these.  They duplicate
code we already have for interpreting user input, except with less fidelity.
 We have quite sophisticated heuristics for how to figure out the correct
scheme, parse drive letters, UNC paths, etc.  We don't need to write another
set.

I also don't really care about trying to fix up "www.google.com"; if it
falls out of the existing code, fine, but I wouldn't bother spending time on
it.  I'm definitely opposed to doing anything like "try DNS resolution on X
and fall back to Y" since it makes the results unpredictable based on your
network.  What if the user specifies a filename that's also an intranet
host?  What if the user says "www.google.com" but the network is currently
down -- does the browser show a "couldn't open local file www.google.com"
error?  etc.

It's enough to simply say "run the input through fixup, supplying the CWD as
the relative file path base".  We have code that can basically take it from
there.

PK
-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] WebKitApi (test_shell) and DevTools, JS debugging

[Peter Kasting]

On Mon, Jan 18, 2010 at 9:16 AM, vridosh  wrote:

> As far as I
> understand, that's why Debugger tab was not available in test_shell in
> early Chromium builds.


Probably the real reason is because test_shell isn't meant to be a reference
implementation of everything and we never bothered to spend any time on it.

So, the main question is - is it possible at all to make DevTools JS
> debugger in test_shell to work correctly?
>

AFAIK test_shell is obsolete and is going to die when we implement
DumpRenderTree atop the WebKit API.  Darin or Dimitri or someone can correct
me if I'm wrong.

I would definitely not depend on or look at test_shell for any serious
project.

PK
-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Re: Two Students Looking to work on Chromium for a Semester-Long Class Project (and beyond)

[Peter Kasting]

On Sat, Jan 16, 2010 at 2:25 AM, Alex Gartrell wrote:

> I've decided to try to tackle crbug.com/20005 (thanks Peter for the
> list).  I'll dive into it more tomorrow.  I don't know if there's an
> 'assignee' role or something on the bug tracker, but I'm currently
> working under the assumption that I should just submit a patch when I
> finish it.
>

Since it's your first time working in the codebase, feel free to grab people
as needed before you get to the patch submission stage.  The best person on
this bug is w...@chromium.org.  He's friendly :)

PK
-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] [LTTF] Finders Pool Drained, Under 300 Failures on Win XP Release

[Peter Kasting]

Dimitri, the LTTF, and anyone else involved, you are awesome, and I consider
your work to have the largest "importance / (satisfaction + recognition)"
value on the team.  This is a thankless task.  You have given me hope that
someday we could conceivably reach 0 failures.

PK
-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Re: Two Students Looking to work on Chromium for a Semester-Long Class Project (and beyond)

[Peter Kasting]

On Fri, Jan 15, 2010 at 12:35 PM, Alex Gartrell wrote:

> Pam's right in that we're looking for a 'Chromium mentor'.


Right.  I understood that, and might be willing to do it; I was more
concerned with what you were actually interested in working on (in terms of
size and style) to get an idea of who'd be appropriate.

We were also interested to see what suggestions you had for things to
> work on.  Right now, we're thinking about taking on a couple 'lay of
> the land' type bugs and then hopefully doing a more non-trivial patch
> later.  Of course, we're still trying to figure stuff out with our
> professor right now, but we're very interested in hearing suggestions.


I once made a list of some of the smaller, but not completely pointless,
bugs I was aware of that I thought someone coming up to speed might be able
to handle.  This list doesn't have anything from the last four months, and
is heavily biased towards the areas I have worked on (== a lot of address
bar stuff especially), but maybe there is something in here that catches
your eye.  Some of these are more difficult than others, but I would be
happy to handhold you through solving any of them.  They would probably
prepare you for tackling bigger projects.

crbug.com/4005
crbug.com/4095
crbug.com/6177
crbug.com/6770
*crbug.com/6872*
crbug.com/6888
crbug.com/7438
crbug.com/7982
crbug.com/9044
crbug.com/9694
crbug.com/9885
crbug.com/12305
crbug.com/13279
crbug.com/13703
crbug.com/14748
crbug.com/16746
crbug.com/18107
crbug.com/18587
crbug.com/20005
crbug.com/20250
crbug.com/22853
crbug.com/22982

PK
-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Ajax maximum response size

[Peter Kasting]

Ever think of using Web Sockets?

PK
-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Two Students Looking to work on Chromium for a Semester-Long Class Project (and beyond)

[Peter Kasting]

On Thu, Jan 14, 2010 at 9:54 PM, Alex Gartrell wrote:

> - We both submitted patches that were accepted to Mozilla as part of
> the prerequisite course to this one (So we know about dvcs, the patch
> submission process, etc.)
>

One thing to note is that Chromium uses neither a distributed VC system nor
Bugzilla, so your development process will look a little different than with
Mozilla patches.  That said, hopefully it shouldn't be hard to accommodate.


> We're pretty open minded as far as the actual project is concerned.
> One thing that was suggested to me in the irc by rubenbb was working
> with SPDY.  I'm kind of a networks geek (doing some research stuff in
> that arena), so that'd be cool.  But really, anything cool would be
> good.


Are you looking to work together on the project or separately?  And are you
looking for a single, larger task, or a lot of smaller issues?  I have a
number of smaller bugs I could throw at you but I'd only want to do that if
that's your interest :)

Your first task should be to get the code checked out, built, and perhaps
running in a debugger.  Start at http://dev.chromium.org/Home and look for
your OS of choice in the second bullet point; that should get you going.
 The majority of Chromium developers work in Windows with MSVC, but there
are healthy numbers of Mac and Linux folks too.  If you have problems, come
hang out on irc.freenode.net #chromium; that's the best place to get quick
answers.

PK
-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Re: opening local files with chrome from command line, relative paths

[Peter Kasting]

On Thu, Jan 14, 2010 at 2:05 PM, Peter Kasting  wrote:

> I don't think Victor's objections have merit.
>

(For public benefit)

Partly because you can't put ':' in a filename on Windows, which is the OS
where local files aren't resolved.   On the Mac opening local files already
works (according to Scott) and I'd be surprised if Linux was different.  In
other words, I'm not asking for new surface area from this purported attack
possibility.

PK
-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Re: opening local files with chrome from command line, relative paths

[Peter Kasting]

On Thu, Jan 14, 2010 at 1:55 PM, Scott Hess  wrote:

> If you try to open a
> relative path and it doesn't work, you go "Oh, right, relative path".
>

No, actually, what I say every time is "What the heck, why did it try to
open this as a hostname?" and then I laboriously navigate through twenty
folders of hierarchy on my disk inside the browser, and it sucks.


> The bone of contention in the thread is what should be done when you
> didn't mean to open a relative path.


I know.  You do what Firefox does, is my answer.  I don't think Victor's
objections have merit.

The number of users who supply filenames on the command line is small, but
of those users, I think pretty much all would prefer the Firefox method to
our current method, so I see no reason to leave things as they are.  The
only thing that the small userbase affects is the priority of this
implementation, which should be "low".

PK
-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Date of M5 drop?

[Peter Kasting]

On Thu, Jan 14, 2010 at 4:48 AM, OwenCM  wrote:

> Hi, I've been scouring the groups and can't find the answer anywhere,
> what time scale are we looking at until the dev branch hits m5?
>

What do you mean by "hits m5"?  Do you mean, the major version stamp gets
changed from 4 to 5?  If so, the timing is kind of irrelevant, because that
alone doesn't mean much.  We could change it today, or next week, or
whenever, as long as (a) version 4 has already branched and (b) version 5
hasn't branched yet.

The best way to figure out what will be in some particular stable release is
to look at whatever branch the beta releases are shipped from, as that
represents the thing that's being stabilized.  On Windows, the last several
beta releases have been on the 249 branch, so that's the best representation
of version 4.

If you're looking for a branch date for version 5, we don't generally
announce those, I don't think.

PK
-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Re: opening local files with chrome from command line, relative paths

[Peter Kasting]

On Thu, Jan 14, 2010 at 9:19 AM, Scott Hess  wrote:

> [BTW, don't take my argument as support for allowing relative paths on
> the command-line.  It's such a low-volume use-case that I'd be
> perfectly fine requiring explicit fully-qualified URLs and be done
> with it.


:( This lack-of-feature has bitten me numerous times in the past few months.
 I support the Firefox way.

PK
-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] RFC: Extensions Incognito

[Peter Kasting]

On Wed, Jan 13, 2010 at 2:28 PM,  wrote:

> I've shared Extensions 
> Incognito
> *
> *
>

The idea of having the ability to do both read-only and read-write access to
the main profile is one that's mirrored in the low-level APIs inside
Chromium, where we have an enum that differentiates between these cases that
we can pass when trying to gain access to various data stores, which
modifies what happens.  I like this parallel and maybe we can implement the
high-level APIs in terms of those low-level ones.

On Wed, Jan 13, 2010 at 2:45 PM, Antony Sargent 
 wrote:

> Right now a convenient way to see if a website is having problems due to
> some extension's content script is to open an Incognito window. Would it
> make sense to add a way to easily disable extensions in Incognito mode,


I think the use case is important ("does one of my extensions break stuff"),
but the current method to solve it (open an incognito window) is a hack, so
I wouldn't want to tie a proper solution to Incognito mode.  It seems like
this is a good use case to keep in mind when figuring out what UI to give
users to control extensions (and plugins).  We have some, but there seems to
be general agreement that we can do more/better (e.g. a simple "disable all"
button).

I also think there's a use case for saying that X extensions should be
enabled/disabled in normal mode but not Incognito, but that may be more
granularity than we can coherently expose in UI.

PK
-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] seeking c++ expertise for a tiny tricky bit of code

[Peter Kasting]

On Wed, Jan 13, 2010 at 12:07 PM, Craig Schlenter  wrote:

> > Do we have other aliasing problems?  If so my opinion changes.
>
> There are some aliasing issues still in play. Off the top of my head:
>
> 1. unit_tests has what might be an issue in stl_tree.h or a compiler issue
>  (bug filed with gcc ppl) and it was already using
> -fno-strict-aliasing to avoid this with gcc 4.4.
> 2. skia has an unresolved issue. Note the filing date:
> http://code.google.com/p/skia/issues/detail?id=18
> 3. v8 has an aliasing issue in dtoa and somewhere else that I forget.
> 4. there are issues in third party code of course eg. icu, webkit etc.


2-4 I'm not worried about because they're effectively third-party (even if
they're done in-house), but 1 is an issue, and cases the compiler doesn't
catch are issues.  So it seems best to go ahead and turn off strict
aliasing.

I'd love to see us fix and enable it later.  Not sure what will provide the
motivation to do that.

PK
-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] seeking c++ expertise for a tiny tricky bit of code

[Peter Kasting]

On Wed, Jan 13, 2010 at 11:28 AM, Evan Martin  wrote:

> PS: I'd be willing to flip the flag just after we do the next beta
> channel push and see how many more problems we get because of it.
> But in general, if it doesn't buy us any performance and it does cause
> hard-to-track-down crashes, I don't see why we should use it.


If it causes hard-to-track-down crashes I agree that is bad.  If the
compiler can't catch errors like this at compile time and we have them at
runtime, then I agree with you that we should turn off strict aliasing for
now.

 The
> "correctness" gain doesn't buy us anything on other platforms anyway.
>

It depends what you mean.  We use GCC on Linux, Mac, and ChromeOS, and we
want to be able to run ChromeOS on ARM, where strict aliasing makes a bigger
performance impact than on x86 due to the number of available registers.  On
Windows, I don't believe MSVC allows you to enable strict aliasing rules,
but ICC would, although for some reason we've never evaluated using it.

PK
-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] seeking c++ expertise for a tiny tricky bit of code

[Peter Kasting]

On Wed, Jan 13, 2010 at 11:22 AM, Craig Schlenter <
craig.schlen...@chromium.org> wrote:

> Other than the immediate gain of "hiding" crbug.com/28749,
>

Which you have a patch (actually two, but one with r+) for, right?


> I think the biggest
> benefit is that end users relying on 4.4 builds are likely to have a more
> stable
> experience in future since it is a safe default.


I don't understand what you mean.  Whichever way we set the flag, we'll be
compiling with it.  So that will be the "safe" thing for others to compile
us with.  Throwing -fstrict-aliasing has the added advantage that it makes
_both_ settings "safe".


> The other thing it buys us is a more relaxed timetable to solve the
> aliasing
> problems if it doesn't break the tree by default.


Do we have other aliasing problems?  If so my opinion changes.

PK
-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] seeking c++ expertise for a tiny tricky bit of code

[Peter Kasting]

On Wed, Jan 13, 2010 at 10:53 AM, Craig Schlenter  wrote:

> I'm one try-server run away from possibly turning -fno-strict-aliasing on
> for
> all linux/bsd gcc: http://codereview.chromium.org/519034
>
> From a "process" standpoint, given that there is some disagreement here
> is someone going to come find me with a clue bat if I commit this?
>

I don't think anyone will be rabid :)

That said, my comment in my prior email stands: if we're basically capable
of throwing -fstrict-aliasing for first-party code now, what do we gain by
instead throwing -fno-strict-aliasing?  I would be sad to see us do this
unless it really buys us something.

PK
-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] seeking c++ expertise for a tiny tricky bit of code

[Peter Kasting]

On Wed, Jan 13, 2010 at 9:42 AM, Evan Martin  wrote:

> At the code level I think it's not too hard for us to be aliasing
> correct (people like Craig have already fixed all of the places where
> we were wrong, and we have tools like bit_cast<> in basictypes.h to
> make it not too painful), so I'm not too opposed to it.
>
> But on a practical note, the default only matters on Linux with gcc
> 4.4, which is not a build configuration we (Google) yet use
> extensively, which means that we break solely but frequently for
> third-party builds:
>  http://code.google.com/p/chromium/issues/list?can=1&q=strict+aliasing
>  (note how much of those mention "fedora" or "ubuntu")


Given your first paragraph, the comment that we're building Chrome OS with
gcc 4.4, and my own experience with aliasing-related bugs and optimizations,
I'm not sure why we shouldn't throw -fstrict-aliasing (for first-party
code).  If we're already correct, this just prevents us from adding new
failures, which is a Good Thing.  I don't know whether we will actually see
a perf win, but we certainly won't see a perf loss, and there's less chance
of weird, obscure bugs.

PK
-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] seeking c++ expertise for a tiny tricky bit of code

[Peter Kasting]

On Tue, Jan 12, 2010 at 10:37 PM, Craig Schlenter <
craig.schlen...@chromium.org> wrote:

> That makes the compiler toss an aliasing error immediately:
>
> cc1plus: warnings being treated as errors
> base/rand_util_posix.cc: In function ‘uint64 base::RandUint64()’:
> base/rand_util_posix.cc:32: error: dereferencing pointer ‘instance’
> does break strict-aliasing rules
> base/rand_util_posix.cc:47: note: initialized from here
>
> Beforehand the compiler didn't warn or error at all despite -Wall -Werror
> btw.
> but it did detect the problem when I did -Wstrict-aliasing=2
>

Sadly I don't understand the issue at all, so I can't help you.  But I do
know that in the past gcc has thrown aliasing errors about code that should
be legal.  I hope this is not happening to you in this case.

PK
-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] on not displaying the home page in a new tab or window

[Peter Kasting]

On Mon, Jan 11, 2010 at 7:20 PM, Dirk Pranke  wrote:

> Is it by design that if I click on a new tab or a new window, and I
> have my preference set to "open this page" on the home page rather
> than "use the new tab page", we still show the new tab page?
>

Yes.  Note that the option is "on startup, open" not "when opening a new
tab, open".

There seem to be extensions (e.g.,
>
> https://chrome.google.com/extensions/detail/jbnkijekempmdlleaimfelifcejbkmcd
> , thanks Nico) that override this behavior, but that's not a
> particularly discoverable solution for new users. (I'm not sure what
> the answer is to fixing that particular problem).


Using an extension is the desired solution per beng on issue 68.

PK
-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Can we use a DOM ui page for ftp:/// and file:/// directory listings?

[Peter Kasting]

On Sun, Jan 10, 2010 at 7:15 AM, Pierre-Antoine LaFayette <
pierre.lafaye...@gmail.com> wrote:

> Is it illegal for the WebKit glue layer to send synchronous messages to the
> browser requesting the icon data URI?


I don't know, but in general sync messages suck.  Why couldn't you use an
async message and just show the icon once it arrives?

PK
-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] How can I tell if a Renderer process finishes loading a page

[Peter Kasting]

On Sat, Jan 9, 2010 at 12:15 AM, hap 497  wrote:

> So how does renderer process knows the whole page is loaded completely
> and tell the browser process to stop the spinning icons and shows the
> favicon of the page?


Sounds like you should start by looking in the browser for what controls the
throbber and backtrack from there.  You want the NavigationController class.

PK
-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] svn question about line-endings

[Peter Kasting]

On Fri, Jan 8, 2010 at 10:25 AM, Alok Priyadarshi  wrote:

> Yes thats what I did to fix line-endings: svn pset svn:eol-style LF.
>
> But this CL contains other changes as well in addition to fixing
> line-endings. Should I do them in separate CLs?
>

Yes, I would do them in separate CLs, since not only is it clearer, but I
think it'll make your trybot problems go away.

You don't need review on the change that just sets the eol-style prop; just
go ahead and commit it directly (assuming the tree is open).

PK
-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] svn question about line-endings

[Peter Kasting]

On Thu, Jan 7, 2010 at 7:42 PM, alokp  wrote:

> SVN Gurus,
>
> I have a question about my CL - http://codereview.chromium.org/527016/show
> .
> Mac and Linux trybots are failing at the patch stage due to line
> endings. The files in my cl were originally submitted with wrong line-
> endings. My CL fixes the line-endings and now patch is failing on mac
> and linux. If I submit the CL, will it break "gclient sync" and hence
> build too?


What do you mean by "fixes the line endings"?

What you want to do is svn pset svn:eol-style LF on all those files.  Do
that as a standalone change.  Then after that you shouldn't have problems.

PK
-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Don't use Empty[String,WString,String16,GURL]() unless you really need to

[Peter Kasting]

On Thu, Jan 7, 2010 at 8:00 PM, Michael Nordman  wrote:

> Where as It looks like GURL::EmptyGURL() may be a tad less costly than
> GURL().
>

Not if you ever need to initialize another GURL with it (since the compiler
can't collapse the copy).  Which is true much of the time that EmptyGURL()
can be replaced by GURL().

And the code clarity reasons still stand.  Please don't do this in the name
of hoping you'll save an instruction somewhere, it's much like premature
optimization.

PK
-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Don't use Empty[String,WString,String16,GURL]() unless you really need to

[Peter Kasting]

On Thu, Jan 7, 2010 at 3:45 PM, Aaron Boodman  wrote:

> Out of curiosity, what is wrong with using EmptyString() in those
> cases? Is there a correctness problem? Unnecessary inclusion of
> string_util.h?


There are a couple reasons.  Code clarity and consistency is a primary one;
using EmptyString() implies you _need_ EmptyString() and that the default
constructor won't work, which can be confusing.  (I am especially frustrated
with code like "std::string x(EmptyString());".)  For folks used to using
WebKit string classes, which differentiate between empty and NULL, this can
look like you're saying more about the semantics of the statement than you
actually are.  If you see code with hundreds of EmptyString()s you begin
wondering whether your code should use it too.

Another reason is that EmptyString() does a threadsafe access to a global
object, whereas the default constructor is a comparatively simple block of
inlinable code that the compiler understands and may frequently be able to
optimize better.

There is not a correctness issue, which is why uses of this can seep into
the codebase without people noticing.  It is, of course, nice to avoid
including string_util.h as you mention, though I'd consider that a minor
benefit.

The totality of the reasons is somewhat lost (to me at least) in the mists
of time; EmptyString() dates from the earliest days of the codebase, and at
the time we went round and round a number of times to determine the best
solutions for each situation.

PK
-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Don't use Empty[String,WString,String16,GURL]() unless you really need to

[Peter Kasting]

On Thu, Jan 7, 2010 at 2:50 PM, Albert Wong (王重傑)  wrote:

> Is there something wrong with returning by copy, and relying on the
> compiler to execute a return value optimization?
>

I'm not totally sure what your comment is saying.

If you are saying that everywhere in the code can return by value instead of
by const ref and the compiler will optimize it equivalently, you are wrong;
I was under the same misapprehension until yesterday.  We've verified that
even in the best case (full optimizations, all functions visible to the
compiler, simple bodies), returning a member by value instead of by const
ref takes more code.

If you are saying that the RVO exists and matters, then of course you're
correct.  When you write code like this:

std::string foo() {
  std::string a;
  // Calculations with a
  return a;
}

...The compiler uses the RVO to avoid copying |a| to the return value at
EOF, and instead just allocate |a| directly to the return slot.  This is why
we prefer return-by-value to return-by-outparam where possible: RVO makes it
just as cheap, and clearer.  But neither is as cheap as return-by-const-ref
if you've already got the referenced object sitting around, as you do on
class member accessors; it's one copy versus zero.

PK
-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Don't use Empty[String,WString,String16,GURL]() unless you really need to

[Peter Kasting]

On Thu, Jan 7, 2010 at 1:50 PM, Jeremy Orlow  wrote:

> What about renaming the function?  EmptyStringHACK() or something?


It's not a hack.  It's a perfectly legitimate thing to use, and not
something we're going to get rid of, unlike ToWStringHack().

Darin suggested we could make these return const pointers instead of const
refs, so callers would need to explicitly deref, to make things look uglier.
 I'm not a big fan of this.

If someone does misuse one of these, it won't corrupt memory, so it's not
catastrophe.  Removing all the wrong uses and adding clear comments about
the right uses, here and in the code, seems sufficient to me.

PK
-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Don't use Empty[String,WString,String16,GURL]() unless you really need to

[Peter Kasting]

On Thu, Jan 7, 2010 at 1:43 PM, Jeremy Orlow  wrote:

> You ignored the second half of my suggestion.
>

The second half of your suggestion leaks memory.  When we have easy and
elegant ways to avoid memory leaks, it behooves us to use them.

It also seems like a poor idea to me to suggest that, potentially, any
function returning a string by reference might have to have its own memory
leak, or allocation code, or static object, if it needs to be able to return
an empty object.  Even if we could do that with no ill consequences, it
would be nice to avoid it.

After my patch, the total number of calls of these functions in the entire
codebase is something like 10 instances.  They're rare enough to be
invisible to most people and unusual otherwise.

PK
-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Don't use Empty[String,WString,String16,GURL]() unless you really need to

[Peter Kasting]

On Thu, Jan 7, 2010 at 1:34 PM, Jeremy Orlow  wrote:

> (As discussed during lunch...)  Why not just do this in this case and
> remove EmptyString() altogether?
>
> const std::string& MyClass::foo() const {
>   static std::string empty = EmptyString();
>   return (everything == OK) ? member_string : empty;
> }
>

This is illegal per the Google style guide:

"Objects with static storage duration, including ... function static
variables, must be Plain Old Data (POD): only ints, chars, floats, or
pointers, or arrays/structs of POD. ... This rule completely disallows
vector (use C arrays instead), or string (use const char [])."

If you see code like this in our codebase, please fix it to not use
static/global non-POD types.

PK
-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Replicated State among tabs in Chromium

[Peter Kasting]

On Thu, Jan 7, 2010 at 1:30 PM, Fady Samuel  wrote:

> What about allowing the renderer to run asynchronously with the
> script?  Right now you're either producing the view or you're running script
> but never both concurrently, correct?  Parallelizing them should not
> introduce issues I think (assuming the renderer has the equivalent of a
> snapshot view of the DOM tree)?
>

But the chief use of JS is to manipulate the DOM.  There's little real
parallelism to extract there, and great cost to doing so.

PK
-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

[chromium-dev] Don't use Empty[String,WString,String16,GURL]() unless you really need to

[Peter Kasting]

If you have ever used any of the EmptyXXX() functions, or ever will, please
read on.

These functions (in string_util.h and gurl.h) are meant for a single,
specific use case:

const std::string& MyClass::foo() const {
  return (everything == OK) ? member_string : EmptyString();
}

Here you cannot return "string()", because it's destroyed before the
function returns, and the caller receives garbage; and you don't want to
have the function return by value, because you can access the member
variable directly and save a copy.  The utility functions give you a global
empty string that you can safely return a const reference to.

DON'T USE THESE OUTSIDE THIS CASE.  You should never use these as
initializers, arguments to functions, or return values in functions that
return by value.  Just use the default constructor; that's what it's there
for.

I have a change out for review that removes the erroneous uses of these from
our codebase and clarifies the comment on their declaration, but it's worth
calling this out directly so they don't creep back in.

PK
-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

[chromium-dev] Core Principles: A refresher (especially for newer contributors/team members)

[Peter Kasting]

A while ago, Ben Goodger (our fearless tech lead) wrote up a set of core
principles around Chromium.  If you have not read and pondered these, please
do:

http://dev.chromium.org/developers/core-principles

In particular, as we've had more contributors both inside and outside Google
over the past year, I feel there have been an increasing number of debates
about things like adding more options and prompts, or making the UI more
complex in other ways.  One of our design goals is never to present users
information and choices they don't understand or care about, and to make the
browser automatically do the right thing, so that it's "a natural extension
of your will" instead of "a piece of software" (in the words of the
document).  While this presents us with hard choices, and we can't always
accommodate every user (including some of us developers!), it's important
that we share a unified vision of what the product should be.  Ben once
mused that he'd love for us to be able to _remove_ options and prefs in each
subsequent version.

My own summary is this: trying to please everyone results in a product that
doesn't perfectly please anyone; we should be willing to be bold, be
arrogant, and create a product that is unsuitable for a few users, if that
means it is exceptional for most.  There are many good choices in the
browser space today, and it's perfectly fine if an individual finds that
Firefox, Safari, or any other browser is a better fit for him--as long as we
understand and accept the tradeoff that triggers that decision.

I hope you're passionate about making something great, and that whatever
you're working on is focused around these core principles.

PK
-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Replicated State among tabs in Chromium

[Peter Kasting]

On Mon, Jan 4, 2010 at 6:55 PM, Fady Samuel  wrote:

> So a script cannot execute concurrently with the traversal of the DOM tree?
> Could this be a performance bottleneck?


Pretty much nothing in the renderer can execute concurrently with other
things in the renderer.  There have been academic papers published about
trying to parallelize parts of web rendering, and some though experiments
from various smart Mozilla and WebKit folks, but from what I've seen it's
not promising.  The web wasn't really designed with thread- or process-level
parallelism on the part of the UA in mind.  (Witness, for example, the
horror of sync XHR, or how difficult it is to make alert()s not be
renderer-modal.)

In particular, it's fairly well-defined that script sees a coherent state as
it executes, so unless you can solve the halting problem, there are pretty
severe limits on how much you could parallelize script execution with other
stuff.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Replicated State among tabs in Chromium

[Peter Kasting]

On Mon, Jan 4, 2010 at 6:32 PM, Fady Samuel  wrote:

> I know I'm asking you a lot of questions here.
>

And you keep removing chromium-dev.  Why?  I'm not the knowledgeable person
about much of this stuff, I'm just trying to be helpful.

Alex mentioned the Webkit DOM tree, indicating that making the nodes of the
> DOM tree immutable in this fashion would be interesting.
>

Why?  AFAIK the DOM tree isn't shared across processes, and the renderer is
effectively single-threaded as far as this is concerned.

How does the current traversal of the DOM tree work? Does it require
> locking? Do you create a copy of the tree for rendering purposes?
>
> I assume scripting enables dynamic updates of the DOM tree to happen all
> the time? I haven't looked into this much myself yet. How is the
> synchronization handled there currently?
>

As I said, I believe the DOM tree isn't shared across processes, so there is
no synchronization that I know of.

I am way out of my depth in this area so you really should not ask me
specifically.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Replicated State among tabs in Chromium

[Peter Kasting]

(re-adding chromium-dev)

On Mon, Jan 4, 2010 at 5:58 PM, Fady Samuel  wrote:

> Was this done to reduce memory overhead?


No, it was done because the newly opened pages and the original pages can
script each other.  From what I understand, making sites in different
processes able to script each other is somewhere between "very hard" and
"impossible".  Darin Fisher, Charlie Reis, Adam Barth and others know much
more about this.

Doesn't this introduce a fault tolerance issue?


In that it increases the number of tabs affected by renderer crashes?  Yes,
but we don't really have a choice.  That's how the web works.

I believe MS' Gazelle project purposefully breaks scriptability here in
order to maintain more robust separation.  This pretty much breaks the web
and wouldn't be shippable as a consumer browser.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Replicated State among tabs in Chromium

[Peter Kasting]

On Mon, Jan 4, 2010 at 5:48 PM, Fady Samuel  wrote:

> So as Peter said a lot of this cache state will not be used in multiple
> tabs because many of these tabs will be different webpages. That's not to
> say that one doesn't open multiple pages from a single site however, in
> which case the image cache might have a lot replication overhead.
>

However, opening multiple pages from a single site will usually also lead to
those pages being in the same process (depending on how they were opened).

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Replicated State among tabs in Chromium

[Peter Kasting]

On Mon, Jan 4, 2010 at 5:17 PM, Fady Samuel  wrote:

> I'm also interested in looking into state that is already shared but makes
> excessive use of locking and may be hindering performance.
>

I'm not aware of anything that really falls into this category.  We share
the visited link state but use a carefully-designed system to avoid
excessive locking.  Renderers can share bitmaps with the browser via shared
memory, but there isn't contention here that I'm aware of.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Replicated State among tabs in Chromium

[Peter Kasting]

On Mon, Jan 4, 2010 at 4:58 PM, Darin Fisher  wrote:

> There are some caches in webkit (the resource cache in particular, but
> there are others) that would be nice to share between processes.


If you look into this, note that there are major tricky issues here around
synchronization if you start trying to share caches between processes
without hammering perf.

Also, the gain from sharing these will not necessarily be large, for two
reasons:
* We already set a fairly small global size limit for the sum of all
resource caches (and the other caches are pretty trivial)
* Renderers in separate processes are very likely to be on separate sites
and have mostly disjoint cacheable sets

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] chromium crash when load flash plugin

[Peter Kasting]

On Mon, Jan 4, 2010 at 3:35 PM, if-ifone  wrote:

> Hi:
>  I build a Release chromium from latest trunk code.my Chromium always
> crash when page
> contain flash elements.
>
> on ubuntu 9.10
> intel 32
> build command
>  make BUILDTYPE=Release
>  and make chrome  BUILDTYPE=Release
>

Please file bugs on crbug.com.

In addition, if you build a binary yourself, it's pretty much your
responsibility to first debug the issue to determine it's something that
other people should care about/help with before reporting such bugs.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] adding and saving preferences

[Peter Kasting]

On Sat, Jan 2, 2010 at 2:07 PM, Andy Ames  wrote:

> I have added a preference to pref_names.h/cc.
>
> In chrome/browser/browser.cc, I added the regisration of the user
> preference to ths static Browser::RegisterUserPrefs(PrefService*
> prefs) function:
>
> prefs->RegisterDictionaryPrefs(prefs::kPrinterSettings);
>

Generally prefs are used in specific classes (e.g. printer-specific code),
and you should register them from that code instead of using Browser as a
general-purpose dumping ground.

I then set a child string pref of the dictionary using:
>
> browser_->profile()->GetPrefs()->SetString(prefs::kPrinterSettings +
> L".child");
>

I don't think this would work.  Are there comments in the Value header about
how to use the classes there?  I think you need to GetMutableDictionary() on
your pref and then add a string to that dictionary.  AFAIK this will then
get saved automatically.  See e.g. the code for remembering zoom levels per
hostname, which uses strings in a dictionary.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] suppressing print dialog in kiosk mode

[Peter Kasting]

On Thu, Dec 31, 2009 at 2:32 AM, Andy Ames  wrote:

> I am considering using a separate switch such as --supress-print-
> dialog, since kiosk mode and suppressing the print dialog are
> orthogonal features.
>

They don't seem orthogonal, since in kiosk mode it doesn't seem like you
would ever want to show a user a print dialog (or any other kind of system
configuration dialog).

2) The command-line switches for the printer and its print settings
> are prefixed with --print-settings. The printer name is specified with
> --print-settings-printer.


Ugh.  Prefer descriptive individual names over groupwise patterns.

It does seem a little odd to break from the norm where command-line
> switches are static and well-documented in the switches source files.
> I had to add a function to CommandLine in order to enumerate the
> switches and look for ones with a particular prefix (--print-
> settings). Thoughts?
>

Why not instead make these things that can be configured in the preferences,
which (being JSON) can be more descriptive without affecting the
command-line parsing code?

4) WISH: Instead of putting this massive list of switches on the
> command-line, using an about:config seems ideal. Has anyone started
> this?
>

See my comment above.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] [WebGL] Recommending --no-sandbox

[Peter Kasting]

On Wed, Dec 30, 2009 at 3:56 PM, Jeremy Orlow  wrote:

> 2) Info bar.  This seems like one of the more popular options at the
>>> moment.
>>>
>>
>> This is a bad idea, we shouldn't do it.  It's not as annoying as a modal
>> dialog, it has problems with clashing with other infobars on start.
>>  Basically it's inferior to a modal dialog in every way.
>>
>
> FYI: The ui-leads (in an off-list thread) seem to like Evan's initial patch
> that goes this route.
>

I would really appreciate being on this thread.  I haven't seen this at all
and I don't see why any mails relating to this need to be private.

What is the rationale for being less annoying than a modal dialog,
particularly in light of Glen's comments above (which I fully agree with)?
 Evan's screenshot is not in-your-face enough.

Glen's idea of making the dialog actually confirm turning the flag off,
instead of just being a notification, is also worth implementing.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] [WebGL] Recommending --no-sandbox

[Peter Kasting]

On Wed, Dec 30, 2009 at 3:22 PM, Jeremy Orlow  wrote:

> I just got back from vacation and would like to take action on this.  I
> read through the thread, but I don't see any sort of consensus on what to
> do.  Here are the options as I see them:
>
> 1) Modal dialog box.  Bad for debugging, will probably just be clicked
> through by users, and not very good for users who leave the browser open for
> long stretches of time.  I'd say it's not a good solution.
>

I thought it was clear that this was the consensus best idea (see Darin's,
Glen's, my posts for example).  I don't see how it's "bad for debugging"
(Viet-Trung's objection makes absolutely no sense to me), and we don't care
about the edge case of users who both use --no-sandbox and never restart
(this works well enough even for restarting once every several weeks, which
takes care of practically everyone).  Clicking through is enough of an
annoyance to serve our purposes, and this is trivially simple to add
(simpler by far than all other options including an infobar).

2) Info bar.  This seems like one of the more popular options at the moment.
>

This is a bad idea, we shouldn't do it.  It's not as annoying as a modal
dialog, it has problems with clashing with other infobars on start.
 Basically it's inferior to a modal dialog in every way.

3) Add some other persistent UI like the incognito spy guy, an annoying
> theme (that overrides whatever you have selected), or something else.  This
> seems like a pretty good option to me, but there hasn't been too much
> discussion around it.  What type of UI would be the best?  Is this a good
> option?
>

No, we shouldn't do this.  Way too much effort and code (think about making
this work on three OSes and with custom themes), we just want something
trivial.

4) Add some warning to the new tab page.  Once again, no one's really
> thought about this seriously.  Any thoughts?
>

Again, inferior to the other options.  Doesn't work well for users who don't
start with the NTP (or ones who never see it -- I don't, I don't use ctrl-t
or the new tab button, I use middle-click and alt-enter).

If you're planning to implement something, please implement the dialog.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] linux printing support and PrintingContext::AskUserForSettings

[Peter Kasting]

On Wed, Dec 23, 2009 at 10:39 AM, Andy Ames  wrote:

> Does anyone have suggestions on where to put the suppress print dialog
> setting? In Firefox, this is done in about:config. Since it's typically a
> developer feature, I want to hide it from the casual user.
>

Unless mhm never landed it, we already have a Kiosk mode command line
switch; it would probably make sense to add it to that.

Also, since security is important, I need to make sure a website cannot
> install a malicious script that can change the value of this setting and
> start sending print jobs.
>

Our settings aren't accessible from JS.  If you add the behavior to a
command-line switch, the only way to "change the value" would be for the
site to be able to kill your process and launch a new one with different
switches.  If the site can do that it can do pretty much anything, so I
don't think you have any situation-specific worries here, just the general
don't-allow-remote-code-local-execution.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Command Line switches persisted in Preferences

[Peter Kasting]

On Fri, Dec 25, 2009 at 12:21 AM, PhistucK  wrote:

> My multi profile case, for example, is to separate the Chromium Google
> account from my regular Google account.
>

Chromium and Google Chrome use different profile paths by default.  You
don't need to manually differentiate them.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Command Line switches persisted in Preferences

[Peter Kasting]

On Thu, Dec 24, 2009 at 7:25 PM, Mohamed Mansour
wrote:

> The options that many people tend to use that I have seen in the forums are
> (or group of people that is):
>  --incognito (they always want to browse invincible)
>

We definitely don't want to encourage this one, because it breaks so many
parts of Chrome's UI, most notably the Omnibox (which will have no inline
autocompletion, no search or URL history, and no suggest or navsuggest).  My
claim is that the majority of people who use this don't understand the true
impact.

 --bookmark-menu (many use this because they want a cleaner UI with no
> bookmark bar)
>

This is a case where the option ought to disappear and we ought to decide
what to do with the implementation.  We should never have major UI
components that are under command-line switches long-term.  EIther they
should graduate to full inclusion in some form or we should decide they
failed.

It would be nice if the UI leads got together with sky and decided on a
permanent course of action for this one.

 --user-dir (some people want to change the location instead of the default)
>

I'd like to understand better the discrete needs of these users.  For
example, do they want to run multiple different Chrome profiles?  Are they
trying to make Chrome portable, or back up its settings on a network?  Are
they looking for a system-wide install?  etc.  If we understand these, maybe
there is something we can do to help.  For example, the multi-profile case
is something we've looked at various times in the past.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Command Line switches persisted in Preferences

[Peter Kasting]

On Thu, Dec 24, 2009 at 3:29 PM, Evan Martin  wrote:

> You must use switches for a pretty common pattern ("separate profile
> that goes through my work proxy") and I believe that's been wontfixed
> in the past, so I'm not entirely unsympathetic to Mohamed's
> suggestion, but I think Peter is right in principle.
>

The proxy case is definitely a case where it affects a small number of
people but is a real need for that group.

My impression is that on Mac this is less of an issue because you can set up
separate Locations in the network settings (?).  On Windows, though, you're
stuck with command-line options.  I wish I had an elegant solution for this
that wasn't Firefox' "my network settings are entirely divorced from the
rest of your system".

I would hesitantly support some sort of network settings dialog that by
default is all dimmed out with a checked checkbox at the top saying "use
system settings".  Don't know that I could convince the UI leads to go for
that, though.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Command Line switches persisted in Preferences

[Peter Kasting]

On Thu, Dec 24, 2009 at 10:28 AM, Mohamed Mansour  wrote:

> Is their any argument not allowing command line options to be persisted in
> the Preferences file?
>

Yes: It allows/encourages people to use more command-line switches.  The
switches we have are generally for exceptional cases.  If people need to use
them regularly, we need to know about it, not simply enable even more usage.
 Switches are not our preferred substitute for about:config.

Things like --no-sandbox are dangerous, past switches like
--enable-extensions are no longer meaningful, and there are a variety of
esoteric switches meant for debugging.  None of these should be "fire and
forget" cases.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Strange key conflict on Windows versions of Chrome, F1 with P, F2 with Q, F3 with R, F4 with S

[Peter Kasting]

Please don't also copy chromium-dev on bugs you file, especially when
they're for esoteric cases like "there is a problem when I hack the DLL to
try and modify the keybindings".

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Recovering from browser crashes

[Peter Kasting]

On Tue, Dec 22, 2009 at 1:59 PM, Evan Martin  wrote:

> For example close to my heart: my last stab at refactoring the way we
> query plugins ended up running into subtle issues with how the metrics
> service shuts down relative to the UI and IO loops.  These pieces need
> to talk to one another but our current system of needing to carefully
> puzzle through relative lifetimes, races, and refcounts doesn't seem
> scalable.


This is part of why jam did his big threading/lifetime change.  In theory,
this situation might be better now.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Looking for a chrome developer with skia-fu

[Peter Kasting]

On Mon, Dec 21, 2009 at 3:42 PM, James Hawkins wrote:

> Hey guys,
>
> I'm trying to triage bug 9589, which is a UMR in skia.  Who would be
> good to help triage a skia bug?


r...@google.com, bre...@chromium.org, senorbla...@chromium.org

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Setting global preferences/settings in code

[Peter Kasting]

On Sat, Dec 19, 2009 at 12:59 AM, PhistucK  wrote:

> Though, I think, currently, there is no way to inject bookmarks.
>

I believe there is, or will be, because I believe we've had this request
before.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Setting global preferences/settings in code

[Peter Kasting]

We have at least some capabilities here since our installer can set some of
these values.  I'm not sure what all the capabilities are.

Make sure you don't get confused by discussions about about:config and
similar.  Generally those are about either "add more prefs" or "add a way to
get at more prefs" (usually both), whereas your request is "some way to set
the defaults for prefs that already exist".  We generally ignore requests
for the former but the latter (what you want) seems reasonable to me.

CCing someone likely to know about this stuff.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Re: Different or same cookies storage in different tabs?

[Peter Kasting]

On Fri, Dec 18, 2009 at 8:57 AM, Sergio Tudela Romero <
sergiotudelarom...@gmail.com> wrote:

> The fact is that every time a cookie is saved the "key" to this is the
> domain where it was created (is this the problme for me). Thus I could
> never have 2 cookies in a single domain since both have the same
> "key".
>

No, cookies have names, and a cookie's key is the host + cookie name pair.
 This is how websites set many cookies at once.

In other: 1 domain with several different cookies differences with the
> "key" the process id of that tab... using a single cookies store that
> offers the profile of this render.
>

Process IDs change every time you close and reopen a tab, or close and
restart the browser.  If we have cookies linked to PIDs 1, 2, and 3, and you
close 2 and 3, then open a new tab 4 on the same site, then close the whole
browser and start anew, what are the cookies at each stage?  It's impossible
to know what to do.

1 Render - 1 profile - 1 storage of cookies - 1 cookie per domain (the
> key of the cookie is the domain).
>

No, n cookies per domain.

1 Render - 1 profile - 1 storage of cookies - N cookies for 1 domain
> (the key of the cookie is the process id of the tab).
>

If you have one renderer, you only have one PID, so you don't have n cookies
for n PIDs.

It's best to attack problems by first stating a use case, then looking for
solutions.  It's not clear what use case you're solving.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Recovering from browser crashes

[Peter Kasting]

On Fri, Dec 18, 2009 at 9:13 AM, Adam Barth  wrote:

> At a high level, imagine we had a watchdog process that kept track,
> essentially, of the tab model and the navigation controllers.  When
> the browser process crashes, we could use this information to do
> something like session restore, but instead of reloading the tabs from
> the network, we could re-attach to the tab processes that are already
> running.
>

This was the proposed solution when we came up with this idea a while back.

There are a number of problems.  The primary one is that it turns out that
the amount of state you need to keep in order to properly recover from
crashes is on the order of the amount of state contained in the browser
process.  The complexity of this watchdog is such that the watchdog itself
is subject to crashes.  Even if you don't keep sufficient state to
"properly" recover, the watchdog is quite complex, and at that point, you
aren't gaining terribly much over our current state of affairs.  After
coming to this conclusion, we dropped the idea.

In the earliest days, we had hoped that the browser side of Chrome would be
so lightweight that it could basically be made completely crash-free.  It
has turned out that that was wildly optimistic.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Different or same cookies storage in different tabs?

[Peter Kasting]

Chromium code has the concept of a "Profile".  All renderers are associated
with a profile.  There is one cookie store per profile.

By default, renderers are all given the same profile.  When you use
Incognito mode, for the purposes of cookies you can think of it like
creating a new profile which will never be saved to disk, so Incognito tabs'
cookie stores are distinct from non-Incognito tabs.

>From a UI point of view we don't allow tabs from separate profiles to
cohabit the same window.  Thus to the user it appears we have Incognito vs.
non-Incognito windows, not tabs.

There is code in place to provide some basic UI for having multiple
(non-Incognito) profiles.  It's off by default.  There are probably bugs and
the user experience isn't good.  The closest bug about providing a good UX
for this functionality is crbug.com/812 .  There is no timeframe on that
work, or necessarily even agreement that it will ever happen.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Re: [chromium-reviews] Add the cmdline "-open-in-new-window" switch

[Peter Kasting]

On Tue, Dec 15, 2009 at 4:45 PM, Evan Stade  wrote:
>
> To put it a different way, the cost of supporting these now is basically
>> indistinguishable from the cost of just adding --new-window.
>>
>
> in terms of code, I don't think that's true. The current patch is extremely
> light weight.
>

And the new patch would be equally light weight.

We're never going to remove them, so there won't be any maintenance burden.
>>  But if we implement --new-window and decide we need to support one of
>> these, we have to add a new option.
>>
>
> Are you gnashing your teeth over the fact we already have --incognito?
>

Yeah, I think it was shortsighted.

Whatever.  I don't care enough about this to argue any longer.  Do whatever
you want with the patch.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Re: [chromium-reviews] Add the cmdline "-open-in-new-window" switch

[Peter Kasting]

On Mon, Dec 14, 2009 at 4:47 PM, Evan Stade  wrote:

> we have had a lot of bug reports asking for NEW_WINDOW, but none for these
> two dispositions. What use cases do you envisage?
>

Wanting to open pages in either of these ways?  Firefox used to have options
to control this and I was sad when they removed them.  CURRENT_TAB is useful
for people who like only using one tab (there are some) and for using Chrome
as an external viewer for HTML-based content.  NEW_BACKGROUND_TAB is useful
for shoveling stuff in without disturbing whatever the user is doing.

To put it a different way, the cost of supporting these now is basically
indistinguishable from the cost of just adding --new-window.  We're never
going to remove them, so there won't be any maintenance burden.  But if we
implement --new-window and decide we need to support one of these, we have
to add a new option.  Same if we ever add any other useful disposition and
want to support it.  So this ensures we won't ever need to add more options.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Re: [chromium-reviews] Add the cmdline "-open-in-new-window" switch

[Peter Kasting]

On Mon, Dec 14, 2009 at 4:34 PM, Evan Stade  wrote:

> I think adding a --disposition= field is overkill and will be harder to
> maintain (which is the worst part about command line flags). If a new
> disposition is added to webkit, one is renamed, or one is deleted, are you
> willing to either maintain this code or have users' desktop shortcuts break?
>

Given that I'm the one that created the dispositions in the first place,
yeah, I am.

CURRENT_TAB and NEW_BACKGROUND_TAB are pretty useful.  The rest aren't, so
I'd only add those two alongside the two you mentioned.

I don't know why we have both SUPPRESS_OPEN and IGNORE_ACTION; I added the
first and Ojan added the second.  At this point it looks like we only ever
set IGNORE_ACTION, and we only ever act on SUPPRESS_OPEN, so something is
really messed up.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Re: [chromium-reviews] Add the cmdline "-open-in-new-window" switch

[Peter Kasting]

On Mon, Dec 14, 2009 at 10:33 AM, Darin Fisher  wrote:

> I think for parity with other apps, we should provide a command line switch
> to force the WindowOpenDisposition to NEW_WINDOW.  For bonus points,
> we could expose other dispositions.
>

Given mdm's explanation, I agree.  And I agree that it would be nice to
expose other dispositions.  It's not particularly more difficult to do this,
so we could just implement "--disposition=" with a few different possible
strings.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

[chromium-dev] Re: [chromium-reviews] Add the cmdline "-open-in-new-window" switch

[Peter Kasting]

On Sat, Dec 12, 2009 at 12:54 AM, Clemens Fruhwirth
wrote:

> http://codereview.chromium.org/464060 adds the small one-line feature
> to open an URL in a new window from commandline
>
> Can any review these changes?
>

Please read http://dev.chromium.org/developers/contributing-code .  In
particular, because the original bug here has never been triaged, you need
to get approval from the UI team that the idea is one we want implemented.
 Then, if that goes well, you need to pick a particular reviewer to review
your patch.  chromium-reviews is a CC list that tracks code reviews, not a
direct mailing list to ask for reviewers on.

In this particular case, the patch doesn't solve what I perceive to be the
original bug as stated ("provide an option to open all links in new
windows") because it provides a command-line option that external programs
can use to open new windows, but nothing to deal with links opened within
the browser.  Also, I'm not sure this is behavior that we want to add
command-line switches for; we try and avoid these unless they're truly
necessary.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Re: [WebGL] Recommending --no-sandbox

[Peter Kasting]

On Fri, Dec 11, 2009 at 11:01 AM, PhistucK  wrote:

> But, as I understand, some people do use it due to issues with the sandbox,
> real issues, system incompatibilities.
> That would be really unfair towards them.
>

Most issues should be fixable.  We need to hear about problems the sandbox
causes, and fix them.  If people just disable the sandbox, we won't hear
about them.

If there is some issue that is not fixable, I would prefer those users not
use Chrome than that they exist in the steady state of using it with the
sandbox off.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Re: [WebGL] Recommending --no-sandbox

[Peter Kasting]

On Fri, Dec 11, 2009 at 1:57 AM, Jeremy Orlow  wrote:

> As for the info bar/modal dialog:  I've been thinking for a bit, and I'm
> not sure this is enough.  We have plenty of data that shows users often
> leave browsers open for a very long time.  The main risk is that someone
> sets the flag, starts their browser, trys out the new cool feature, and then
> leaves the browser window open...for a long time.  The next time they start
> it they'll see the warning again, but the period of time in between (that
> they're more vulnerable) could be non-trivial.
>

I think that the combo of factors involved here makes this enough of an edge
that we can Not Worry About It.

I think an infobar at startup is not annoying enough, and as Darin says, we
often have other infobars to show then, which is problematic.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] [WebGL] Recommending --no-sandbox

[Peter Kasting]

On Thu, Dec 10, 2009 at 9:38 PM, John Abd-El-Malek  wrote:

> We disable --single-process and --in-process-plugins on release Google
> Chrome builds to avoid the support headache that it causes.  I think we
> should do the same for --no-sandbox.


There are legit reasons we have asked users to try temporarily disabling the
sandbox, more frequently than for those other flags.  I'd prefer to just
make the UI turn ugly a la Jeremy's bug.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] [WebGL] Recommending --no-sandbox

[Peter Kasting]

On Thu, Dec 10, 2009 at 8:22 PM, Darin Fisher  wrote:

> Perhaps --enable-webgl should instead implicitly disable the sandbox today
>

I think this is better than having users manually disable it.  They'll be
running without a sandbox either way, but this (a) makes the enabling
process less error-prone and (b) makes it more likely the sandbox will get
turned back on eventually.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Extensions and the Mac

[Peter Kasting]

On Thu, Dec 10, 2009 at 5:02 PM, Avi Drissman  wrote:

> Q: Can't we have the extensions gallery warn that it won't work?
> A: Sorry, we can't do that in an automated fashion. The extensions author
> should mention it. Too bad they don't.
>

But we explicitly review patches with binary components.  I can't see how it
could be impossible for us to also mark them as "Win-only".

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Thoughts on "// NOLINT"?

[Peter Kasting]

On Thu, Dec 10, 2009 at 11:20 AM, Jacob Mandelson wrote:

> If something extra in an expression is a common case, I've sometimes
> seen it done like:
>return DoWork(&foo) POSIX_ONLY(&& DoWork(&posix_specific));
> where POSIX_ONLY will expand to nothing or its argument.
> It's ugly, but compact.


The Google C++ Style Guide says to avoid macros when there is a non-macro
way to do the same thing.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Thoughts on "// NOLINT"?

[Peter Kasting]

On Thu, Dec 10, 2009 at 10:45 AM, Jonathan Dixon  wrote:

> In essence:
>
> return DoWork(&foo)
> #if defined(OS_POSIX)
> && DoWork(&posix_specific)
> #endif
> ;  // <-- Lint complains about this guy
>

I'd prefer this:

#if defined(OS_POSIX)
  return DoWork(&foo) && DoWork(&posix_specific);
#else
  return DoWork(&foo);
#endif

The same number of lines, but much easier to read.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Thoughts on "// NOLINT"?

[Peter Kasting]

On Wed, Dec 9, 2009 at 3:48 PM, John Abd-El-Malek  wrote:

> Lately I've been seeing more and more // NOLINT added to the code.  It's
> great that people are running lint to make sure that they're following the
> guidelines, but I personally find adding comments or gibberish to our code
> for tools that are supposed to make the code quality better happy/more
> consistent a bit ironic.  I also find it distracting when reading the code.
>  Am I the only one?


You are not.  We should prefer linter errors to // NOLINT comments, because
we should prefer to optimize code readability at the expense of reviewers
making more judgment calls, and these comments are definitely visual noise
that detracts from readability.

I support an immediate s/\/\/\ NOLINT//g.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Re: Visualizing painting

[Peter Kasting]

On Wed, Dec 9, 2009 at 5:30 AM, MAD  wrote:

> Thanks pkasting@ for trying this with
> http://src.chromium.org/viewvc/chrome?view=rev&revision=34108
>
> But as the page cyclers dashboard seem to confirm, this is still a
> issue... :-(
> http://build.chromium.org/buildbot/perf/xp-release-dual-core/moz/report.html?history=150


Yeah, sorry.  I leave this in your hands.

I have just checked in a change which makes the disabled code build on
linux_views, once you wish to try again, so you won't have the build bustage
problem I did.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Height of Personal Stuff tab in Options window

[Peter Kasting]

On Tue, Dec 8, 2009 at 4:59 PM, Evan Stade  wrote:
>
> this makes the assumption that there is some "best" setting for each WM,
> which is false. What's best for me on metacity is not what's best for you on
> metacity.
>

Unfortunately, if you really believe that, then for one of us the default is
wrong, and statistically the aggrieved party is unlikely to find,
understand, and change the option.  The option doesn't actually solve the
problem the majority of the time.

Therefore, in most cases we should just pick what we think is best, and let
people who don't like it either adapt or leave, because that reduces
cognitive friction, support costs, and code complexity.  Unfortunately in
this case we're talking about supporting both codepaths, which nixes the
maintenance pluses.  We still get fewer user choices though, which would be
enough for me.

there have been many, many, many bugs where a user requests some random
> functionality provided by their particular WM, and our only answer (save
> re-implementing the functionality) is to tell them to disable the custom
> frame.
>

This is why I said the default should be "window manager frame for all WMs
we have not explicitly whitelisted".  We can only get away with the custom
frame if we know it will work well.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Height of Personal Stuff tab in Options window

[Peter Kasting]

On Tue, Dec 8, 2009 at 4:55 PM, Ben Goodger (Google) wrote:

> BTW I think the "Use Gtk Theme" button should be replaced by a special
> theme that triggers this mode, much like the "default" theme we have
> in the theme gallery. This would make the selection of this mode vs.
> others feel more natural wrt the others.


That sounds like a great idea to me.

Going further, I would love if the default theme(s) available for each
platform were visible in the gallery.  Then we don't need a "default"
button/set of buttons, and we can condense down to a simple "choose theme"
link/button.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Height of Personal Stuff tab in Options window

[Peter Kasting]

On Tue, Dec 8, 2009 at 4:25 PM, Evan Stade  wrote:

> On Tue, Dec 8, 2009 at 4:18 PM, Peter Kasting  wrote:
>
>> * We have crazy word wrapping.  The bookmark sync text could fit on one
>> line.  Why does it wrap?  etc. elsewhere
>>
>
> yes, we can save two lines in bookmark sync by removing the blank line and
> disallowing line wrap on the first label (although I'd guess that might
> force the dialog to be too wide in some locales).
>

Why do we need to disallow wrapping?  Why can't we just wrap at the right
margin, instead of somewhere in the middle of the dialog?  We aren't just
manually splitting this into two arbitrary strings, are we?

* What is with the blank line below that text?
>>
>
> read original post
>

My bad, missed that

* "Show Saved Passwords" button should be vertically level with "offer to
>> save passwords" radio button, horizontally on right side of dialog (a la
>> Firefox)
>>
>
> this suggestion conflicts with the gnome hig
>

I'm not convinced Chrome in general complies with the Gnome HIG.  In general
I'd be OK with a violation like this.

* Appearance section is a mess.  Why are there buttons for GTK/Classic theme
>> when it looks like what's desired is a radio button pair?
>>
>
> to match windows
>

But Windows just has a "reset" button.  It's not obvious that's how these
are functioning.  This looks more like a 3-radio group: "GTK" "Classic"
"" with a "get themes" button by the third one.

 Why are there these other options?  We should decide, based on what the
>> user's windowmanager best supports, which combo of settings will work best
>> and just do it.  We don't give Windows Aero users a button called "use
>> classic theme" or Mac users a button to use the system-style (down-hanging,
>> square-edged) tabs.
>>
>
> not plausible. Windows and Mac have the advantage of only a single window
> manager, or very few WMs if you count different editions of the same WM.
> Linux has tons of WM and each provides a different set of functionality to
> the user, mostly through the window frame. We can either force all users to
> give up all their WM functionality (no go) or give up on the custom frame
> for linux altogether (no go).
>

I'm not convinced.  It seems easy to enumerate the dozen most common WMs
(KWM, blackbox, fluxbox, etc.) and decide what's best for each of them, and
that will cover most cases.  For the rest, we use the window manager frame
because we don't know what they might do.

The whole issue with removing options is that you have to be willing to make
choices on the user's behalf and presume you know what's best.  It's easy to
punt things to options.  That's how we get products like Seamonkey.  Chrome
should be the opposite.  We should be arrogant.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Height of Personal Stuff tab in Options window

[Peter Kasting]

On Tue, Dec 8, 2009 at 4:11 PM, Evan Stade  wrote:

> proposals on which of these options (again, see original attachment) to
> "rip out" are welcome and within the scope of this thread.
>

Off the top of my head:
* We have crazy word wrapping.  The bookmark sync text could fit on one
line.  Why does it wrap?  etc. elsewhere
* What is with the blank line below that text?
* "Show Saved Passwords" button should be vertically level with "offer to
save passwords" radio button, horizontally on right side of dialog (a la
Firefox)
* Save passwords/save form values could perhaps be combined into one section
heading
* Does the explanatory text under "browsing data" add much?  Maybe rip it
out and make the button text longer if we need clarity ("Import data from
another browser...")
* Appearance section is a mess.  Why are there buttons for GTK/Classic theme
when it looks like what's desired is a radio button pair?  Why are there
these other options?  We should decide, based on what the user's
windowmanager best supports, which combo of settings will work best and just
do it.  We don't give Windows Aero users a button called "use classic theme"
or Mac users a button to use the system-style (down-hanging, square-edged)
tabs.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Height of Personal Stuff tab in Options window

[Peter Kasting]

On Tue, Dec 8, 2009 at 4:03 PM, Ian Fette  wrote:

> Putting on my individual contributor hat here, I have to say that Ben's
> solution would seem very non-intuitive to me. I'm not aware of any app that
> works that way, and I would probably think that the dialog was just cut off
> (as is currently the case on my netbook), and would not expect to be able to
> somehow auto-scroll it by touching the edge. It seems to me that you would
> want to somehow visually indicate that there is more of this dialog to be
> seen, preferably show how much dialog is left to be seen and allow someone
> to move through that, at which point we're talking scroll bars...


Also throwing my hat into the ring to say that I don't think Ben's proposal
is great.

I think Ben's goal is to fight against options sprawl.  Being hardnosed
about not adding scrollbars or tabs is a way to put pressure on to decrease
options menu size, rather than increase.  I applaud this principle.

The thing is, if we're already offscreen, and we add a magic solution like
"autoscroll at screen edge" or any other similar solution, we effectively
have a scrollbar -- just a hard to use one.  It's the worst of both worlds.

I would love to see us continually rip out more and more options.  If we are
unable to do that, I think having a scrollbar is better than having a magic
scroll function.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Visualizing painting

[Peter Kasting]

On Mon, Dec 7, 2009 at 8:15 PM, Darin Fisher  wrote:

> Chrome now supports a handy command line flag that'll show you the regions
> of the
> page that are being re-painted.  This can be helpful if you are tracking
> down repaint
> issues.
>
> $ chrome --show-paint-rects
>
> It's interesting to see what causes large re-paints ;-)
>

I would love to see someone enable the window resize gripper on Windows
(which has never been enabled due to repaint perf issues) and start using
this flag to figure out what's up.

Added incentive: maybe Darin's repaint coalescing work has eliminated the
performance problems of this code and we can just turn it on.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Re: Plugin Manager UI Proposal

[Peter Kasting]

Sigh, resending now that I have re-added my address to Groups after it got
auto-removed :(

On Mon, Dec 7, 2009 at 12:25 PM, Peter Kasting  wrote:

> On Mon, Dec 7, 2009 at 11:45 AM, Panayiotis Mavrommatis <
> panayio...@google.com> wrote:
>
>> I'm not sure if my email did reach chromium-dev,
>
>
> It didn't, so I was confused at first :)
>
> > >- Modify about:plugins for this purpose. That page is a simple
>>
> > >javascript page that iterates over navigator.plugins. Similar to
>> other
>> > >about: pages it doesn't have access to chrome internals. We feel
>> > >chrome://plugins follows the model of chrome://extensions better,
>> in that
>> > >this page cannot be inspected and allows the user to modify state
>> of the
>> > >browser.
>>
>
> To users the distinction is meaningless.  I suspect (but am not sure) that
> we can write an internal handler that serves about:plugins in whatever way
> we want (e.g. as a DOM UI page with two-way communication with the browser).
>
> > >- Add the plugins to chrome://extensions  -- we can do that too,
>> we'll
>> > >let Chromium devs chime in on what's the preferred option here.
>>
>
> We should definitely put the two together in whatever UI we end up with.
>  Users don't perceive these differently (just try explaining to a
> non-developer how when they say "plugins" they really mean "extensions"),
> and the information to see and actions to take are pretty much identical
> with an extension versus a plugin.
>
> > >- Modify mostly PluginService (chrome/browser/plugin_service.cc)
>> > >- Will store state in the sqlite database, per user.
>>
>
> I'm not familiar with this code.  Does it already use a sqlite database?
>  If not it might make sense to just shove this in the Preferences file.
>  This is where we list other similar data, and it avoids the overhead of
> having another file to open and read on startup.
>
> > >- A plugin is identified by its path in the filesystem. Different
>> paths
>> > >are considered different plugins.
>>
>
> Does this imply that different versions of a plugin are different plugins?
>  I ask because it would be annoying to find the Flash auto-reenabled itself
> after it auto-updated.  Perhaps "same name OR same path"?
>
> PK
>

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Profiles + SharedWorkers

[Peter Kasting]

On Mon, Dec 7, 2009 at 11:37 AM, Drew Wilson  wrote:

> I currently am special-casing incognito windows, so incognito windows don't
> share workers with non-incognito workers, but I don't do anything to deal
> with profiles in general (so if you were running with separate profiles,
> those profiles would see one another's workers).
>

That's bad.  You should not special-case Incognito.  You should make these
profile-scoped.

I'm trying to figure out the best way to address this (and whether this is
> something that needs to be addressed in the near term, given that we don't
> officially support profiles - it sounds like chromeos may make some use of
> them?).
>

We do officially support profiles: they're how Incognito works.  And we go
to great lengths elsewhere to make them work right in general.  We don't
have UI for widespread general use of lots of profiles, but the
functionality works well.

If you look at the implementation of profiles you will find that most things
that act "global" are really hung off the profile.

ResourceMessageFilter has a reference to a Profile object - are these
> Profile objects global (for example, in the typical situation of a single
> profile + incognito, are there only two Profile objects in the Browser
> process)?
>

Yes.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] NaCl related build error

[Peter Kasting]

On Mon, Nov 30, 2009 at 10:01 PM, Antony Sargent wrote:

> I just updated to revision 33425, and a clean build (manually deleted Debug
> directory before opening .sln file) gives the following error in
> service_runtime_x86. I'm running Visual Studio 2008 on Vista x64. Anyone
> else seeing this, or know what might be the problem?


NaCl has had problems building with Cygwin for almost two weeks.  I believe
Brad Nelson just fixed things, so try wiping out your Debug/ dir,
re-syncing, and rebuilding.  (I haven't yet tested this myself.)

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] [MEMORY] CreateDIBSection: memory usage for bitmaps in Windows

[Peter Kasting]

On Fri, Nov 27, 2009 at 5:43 PM, Kenneth Russell  wrote:

> While investigating
> http://code.google.com/p/chromium/issues/detail?id=21921 I observed
> that on Windows that when the Chrome window is resized, a Skia canvas
> the size of the entire window is allocated and discarded in order to
> paint the window background. I don't think this happens during normal
> repainting operations but is probably one cause of allocation
> thrashing of these objects. If it would be helpful I can try to
> reconstruct where this happens but others on this list probably know
> where it is off the top of their heads.
>

+CC brettw, darin

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] PrefsService shared with OffTheRecordProfile

[Peter Kasting]

On Wed, Nov 25, 2009 at 11:38 AM, John Gregg  wrote:

> if you whitelist an origin for popups while in incognito mode, that origin
> is whitelisted permanently even when you go back to normal mode.
>

On further reflection I'm convinced persisting these changes is wrong.  I
have now posted a patch to fix this at http://codereview.chromium.org/434109.

I have similarly updated my change to add per-host remembering of zoom
levels ( http://codereview.chromium.org/437077 ) to also avoid persisting
changes while off the record.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] PrefsService shared with OffTheRecordProfile

[Peter Kasting]

On Wed, Nov 25, 2009 at 12:49 PM, John Gregg  wrote:

> On Wed, Nov 25, 2009 at 12:26 PM, Peter Kasting wrote:
>
>> This is the sort of thing for which Profile::ServiceAccessType was
>> invented.  Ideally, things like recording whitelisted popup hosts should
>> request IMPLICIT_ACCESS, which should result in no read happening.
>>
>
> From reading the code, I'm not sure that's consistent.  Adding a
> whitelisted host for popups is a result of a user action (the user choosing
> "always allow" from the blocked popup container), so according to the code
> comments, that would be an explicit access.  It's kind of a gray area: it's
> like adding a bookmark in terms of user-initiation, but it also affects web
> browsing.
>

Hmm.  On the one hand, we persist things like created bookmarks and
downloaded files.  On the other hand, we don't preserve changes due to other
"user actions" like clicking a link.  It's not obvious to me what the best
example is.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] PrefsService shared with OffTheRecordProfile

[Peter Kasting]

On Wed, Nov 25, 2009 at 12:14 PM, Adam Barth  wrote:

> On Wed, Nov 25, 2009 at 11:38 AM, John Gregg  wrote:
> > if you whitelist an
> > origin for popups while in incognito mode, that origin is whitelisted
> > permanently even when you go back to normal mode.  And in my case, it
> > behaves likewise for notifications, since those permissions are stored in
> > the PrefsService.  Is that how we want it to work?
>
> Nope.  That sounds like a bug.  We should never write URLs or hosts
> the user visits while in incognito to disk.


This is the sort of thing for which Profile::ServiceAccessType was invented.
 Ideally, things like recording whitelisted popup hosts should request
IMPLICIT_ACCESS, which should result in no read happening.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Core Text

[Peter Kasting]

On Mon, Nov 23, 2009 at 2:51 PM, Dirk Pranke  wrote:

> As an aside, have we looked at using DirectWrite() on Windows?


crbug.com/25541

("No")

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] UI Jank Task Force Status Update

[Peter Kasting]

On Mon, Nov 23, 2009 at 2:58 PM, Glenn Wilson  wrote:

> Investigating using a flat file for safebrowsing instead of SQLite (need to
> recreate bloom filter and watch disk I/O)


jam crunched some numbers here.  During the bloom filter recreation process
we read about 50 MB and write about 40 MB.  The actual raw size of the
hashes is roughly 4 MB.  I don't know how big the bloom filter is but it's
not terribly big.  Conclusion: by moving from sqlite to some kind of raw
file on disk, we could save a large amount of the IO.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Singleton shenanigans in base/time_win.cc causing problems (not the first time)

[Peter Kasting]

On Sun, Nov 22, 2009 at 4:28 PM, Mike Belshe  wrote:

> I think we should have a list of low-level functionality which we just
> never cleanup.
>
> For the items you listed, I think you should leak them all.  Trying to
> cleanup these items creates complicated code and ultimately won't run any
> better and possibly slower.
>

+1 for leaking a much as possible on shutdown.  We should make sure we shut
down anything that can write data to disk, and then just kill the process.
 Right now I suspect we clean up much more stuff in the browser process than
we have to.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] whitlisting compilers for -Werror

[Peter Kasting]

On Sat, Nov 21, 2009 at 12:06 PM, Evan Martin  wrote:

> This works for warnings we know about now, but not warnings that will
> occur in the future, which is the larger problem.
> I'd say we break the automated Ubuntu builds every couple of weeks
> (and get an additional report from users at about that same rate).
>

What I'm concerned about is that occasionally these warnings may be real and
useful, rather than always being stupid.  It seems like each warning should
be something we at least take a look at to make sure it's not a concern,
which is why I'm uneasy just disabling warnings.  (This goes for Jens'
compile error case too.)

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] whitlisting compilers for -Werror

[Peter Kasting]

On Sat, Nov 21, 2009 at 10:19 AM, Evan Martin  wrote:

> I have been particularly frustrated with gcc warning bugs that have
> been fixed in newer versions of gcc.  In older gccs, the following
> code produces a "variable may be used uninitialized" warning depending
> on your optimization settings.
>  int x;
>  { x = 3; }
>  printf("%d\n", x);
> The fix is to put in a redundant initialization of x, but people
> (reasonably) will remove them when working on the code.


There is another fix, which is to disable to warning within the file or
globally for GCC versions less than X.  GCC exposes a number of different
macros and switches that let you determine the version precisely either from
script or in the preprocessor, and we already have a file that holds
compiler-specific magic.

The reason this would be nice compared to the whitelist proposal is that it
wouldn't produce false negatives.  The assumption is that there are only a
small number of these kinds of cases, so enumerating them is not going to do
awful things to our code.

Random side note: In the final build fix you linked, you used a C-style
"(void)" cast.  Can we use a C++-style cast?  The style guide frowns on
C-style casts.  (This assumes we don't disable the warning via your or my
above methods.)

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Re: class has virtual method but non-virtual destructor

[Peter Kasting]

You did note that we're in the process of enforcing precisely what you want
enforced, right?

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] class has virtual method but non-virtual destructor

[Peter Kasting]

On Fri, Nov 20, 2009 at 3:06 PM, James Robinson  wrote:

> I'd also favor just going with virtual d'tors rather than protected
> non-virtual ones.  Protected virtual if you want to enforce that the object
> is never deleted via a ptr to the base class.
>

I have no opinion here so I'll let you guys sort this out independently.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] class has virtual method but non-virtual destructor

[Peter Kasting]

On Fri, Nov 20, 2009 at 3:01 PM, Jacob Mandelson wrote:

> I had the impression that at the end of the discussion you were still
> against.  Can you LG 201100 and 200106 ?


Done.  I didn't bother looking at the patch, I assume you did the right
thing and followed relevant style rules.  Let me know if you want me to do a
real review.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] class has virtual method but non-virtual destructor

[Peter Kasting]

On Fri, Nov 20, 2009 at 2:53 PM, Jacob Mandelson wrote:

> http://codereview.chromium.org/201100/show
>

Yes, that caused a large subsequent discussion at which it seemed like it
was determined that this was fine.  I was surprised to hear this issue come
up again because I'd assumed you'd already checked in your fixes.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] class has virtual method but non-virtual destructor

[Peter Kasting]

On Fri, Nov 20, 2009 at 1:31 PM, James Robinson  wrote:

> On Fri, Nov 20, 2009 at 12:59 PM, Peter Kasting wrote:
>
>> For a concrete example, take AutocompleteEditController, which is declared
>> in autocomplete_edit.h.  This is an abstract base class that names several
>> different methods.  The purpose of the class is to insulate the
>> functionality the edit needs from whatever code actually implements that
>> functionality.  However, the edit doesn't create or own pointers to its
>> controller, and never deletes its controller, so this abstract class doesn't
>> have a virtual destructor.  The pattern here is "implements interface X" as
>> opposed to the "is a specialized type of an X" pattern of parent and child
>> classes.
>>
>
> What's the benefit of omitting the virtual destructor?
>

I'm not trying to say it has massive benefits.  I'm trying to make concrete
the rather abstract statement that we have patterns right now where objects
don't specify destructors.

If you want me to argue for it, then I would probably say that it's a little
simpler and clearer without a destructor, and for someone used to reading
our code it's a tipoff that this is an instance of the "interface" class
pattern.  If I were to add a destructor, I'd declare one in private as
opposed to adding a virtual one, again just to emphasize that this is an
interface as opposed to a parent of more specialized children.  Not a very
important set of reasons.

If you feel violently, write the patch.  I won't stop you.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] class has virtual method but non-virtual destructor

[Peter Kasting]

On Fri, Nov 20, 2009 at 12:42 PM, Mark Mentovai  wrote:

> As Evan points out, there are some cases when it's not absolutely
> necessary to have a base or interface class declare a virtual
> destructor.


For a concrete example, take AutocompleteEditController, which is declared
in autocomplete_edit.h.  This is an abstract base class that names several
different methods.  The purpose of the class is to insulate the
functionality the edit needs from whatever code actually implements that
functionality.  However, the edit doesn't create or own pointers to its
controller, and never deletes its controller, so this abstract class doesn't
have a virtual destructor.  The pattern here is "implements interface X" as
opposed to the "is a specialized type of an X" pattern of parent and child
classes.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

[chromium-dev] Re: Memory purger available for testing

[Peter Kasting]

Jeremy noted yet another omission: I also clear the memory backing the
LocalStorage sqlite DBs in the browser.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] user-contributed translations

[Peter Kasting]

On Thu, Nov 19, 2009 at 2:17 PM, Evan Martin  wrote:

> Also, another option for these users is to write their own
> translations and let downstream (Chromium distributors) ship them.


I do think it would be nice to document somewhere how someone could build a
Chromium with their own translation included, for an existing or new
language.  I'd expect the usage of such steps would be low.  However, in a
few cases, serious public contributors might use this to test and refine a
translation to the point where perhaps it would be useful to present to some
of the team that's doing the translations (dunno exactly how they work).

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

[chromium-dev] Re: Memory purger available for testing

[Peter Kasting]

On Wed, Nov 18, 2009 at 1:16 PM, Peter Kasting  wrote:

> *Currently purged:
> Browser process: History backend, "Web data" backend (search keywords
> etc.), Proxy resolver JS heaps, Safe Browsing backend, TCMalloc free pages
>

Chase noted I forgot to list one other thing I'm purging in the browser
process: Backing stores.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

[chromium-dev] Memory purger available for testing

[Peter Kasting]

This morning I checked in the central bits of the MemoryPurger.  This allows
you to start Chrome with "--purge-memory-button", which will add a button to
the Task Manager called "Purge Memory".  Pressing this button will attempt
to free as much memory as possible* from the browser and renderer processes.
 My hope is that this will be useful in finding cases where we're using too
much memory; I'd like to eventually hook pieces of this to automatic
signals.

Please let me know anything interesting you find with this.

PK

*Currently purged:
Browser process: History backend, "Web data" backend (search keywords etc.),
Proxy resolver JS heaps, Safe Browsing backend, TCMalloc free pages
Renderer process: Spellchecker, WebCore object cache, WebCore font cache,
WebCore cross-origin preflight cache, sqlite database backing stores, JS
heap, TCMalloc free pages.

Let me know if there are major memory consumers I'm missing.

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Linting chrome/ in pre-submit checks

[Peter Kasting]

On Tue, Nov 17, 2009 at 11:40 AM, Evan Martin  wrote:

> Since we're talking about style, I'll note that this pattern is no
> good (and I've seen it explicitly called out somewhere before).
>
> The problem is that your assertions are not helpful.  You get
> "expected 'foo', got 'bar' on line 80" but line 80 is just the body of
> a for loop.


(a) I frequently write EXPECT_EQ(a, b) << "Test case in question is " << c;
(b) Even when this is not true it costs me almost no time to track down the
case in question in the debugger (if I need to; often the expected result is
unique)

Don't assume what is and is not helpful to me.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev

Re: [chromium-dev] Linting chrome/ in pre-submit checks

[Peter Kasting]

On Tue, Nov 17, 2009 at 11:16 AM, Elliot Glaysher (Chromium) <
e...@chromium.org> wrote:

> Currently, it only runs it at (gcl/git cl)
> upload time and only generates warnings. In the future, it should
> error at commit time, but I want to put this through a trial period so
> please pay attention to the warnings and yell and scream at me if
> there are false positives.


There are some unittests where we have super-long bits which go over 80
chars, and there are also some where the linter thinks our indenting is
strange when we do something like:

struct Foo { ... };
Foo foo_cases[] = {
  {"a",
   "b",
   "c"},
}

(It doesn't like the uneven number of spaces when lining up the struct
member initializers).

I don't know whether those will trigger warnings/errors in your tool.

- ';' shouldn't be used in empty loops. Use "{}" instead.
>

This makes me super sad.  ";" alone, indented, is so much more obvious.

PK

-- 
Chromium Developers mailing list: chromium-dev@googlegroups.com 
View archives, change email options, or unsubscribe: 
http://groups.google.com/group/chromium-dev