On Wed, Jul 15, 2009 at 3:29 PM, Erik Kay<erik...@chromium.org> wrote:
> One naming thought for you.  I wonder if the methods should be named
> "loadScript" and "loadCSS"?  I know that this is effectively the same thing
> with JS, but "execute" sounds like it's just running something temporarily
> (maybe in the page's context) and forgetting about it, while load is loading
> it into the page and leaving it there for later reference.  I don't have a
> strong opinion about this, it's just what popped into my head.
> When you say no attempt is made to dedupe these, again it sounds like the
> only downside is that the code has been run twice.  Even for transient code,
> the newly created context would stick around until the next GC,  If the code
> did anything more substantial, it would also keep this context and extra
> memory alive.  Perhaps this is just solved by stronger wording in the doc.

I see what you mean. Since we're creating a new context, it is a
heavier operation than "execute" might imply. On the other hand, to me
"load" sounds like it is something that doesn't have any side-effects.
It might not seem like it could be used to actually do anything, other
than load a library. In JavaScript, loading and executing are the same
thing, so it's just a matter of what will confuse less often.

I'm going to leave it executeScript() since it also accepts a simple
string of JavaScript, which is definitely not implied by "load".

I added a bunch of comments about the weight though, and also changed
the name "executeCSS" to "insertCSS" because you don't really
"execute" CSS.

On Thu, Jul 16, 2009 at 2:44 AM, disya2<dis...@gmail.com> wrote:
> Concerning the following:
>  // tabId: Optional, defaults to selected tab of focused window if
> omitted.
>
> Maybe it would be better to use current window instead of focused? The
> caller is extension script and it may have some state variables or
> something which belongs to current window while extension script in
> focused window may have different state.

On Thu, Jul 16, 2009 at 6:11 AM, Erik Kay<erik...@chromium.org> wrote:
> The problem with that is that background pages don't have a "current window"
> the way that toolstrips do.

Great catch, disya. You are right that it would be more consistent,
with, eg chrome.tabs.getSelected() to get the "current" window as
opposed to the topmost one. I've changed this.

Erik, the way that getCurrent() works is that it returns the window
the code is executing in, but if there is no such window, then it
falls back to the topmost window.

- a

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

Reply via email to