pfn / passifox

Extensions to allow Chrome and Firefox (4.0+) to auto form-fill passwords from KeePass (requires KeePassHttp)
GNU General Public License v3.0
905 stars 185 forks source link

chromeIPass is slow (adds hundreds of milliseconds to any page's load time) #550

Open mehrdadn opened 8 years ago

mehrdadn commented 8 years ago

It took me a long time to track down the slowness of my browser to chromeIPass, because I couldn't believe it might be the cause. Here's how I finally pinned it down:

  1. Start a clean browser (no extensions, or only lightweight extensions)
  2. Open the Chrome debugger's "Timeline" tab on any tab (even google.com)
  3. Refresh the page (F5)
  4. Notice that chromeIPass is slowing the page down even though nothing interesting is happening
  5. Repeat with chromeIPass disabled, and notice the page loads faster

chromeIPass version: 2.7.2 KeePassHttp version: not sure, but recent Google Chrome version: 51.0.2704.103 Win x64 (but I'm pretty sure it'd happens on all versions)

pfn commented 8 years ago

I would appreciate a pr that would help address this.

On Mon, Sep 12, 2016, 6:09 PM mehrdadn notifications@github.com wrote:

  1. Start a browUse the Chrome debugger's "Timeline" tab to see

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/pfn/passifox/issues/550, or mute the thread https://github.com/notifications/unsubscribe-auth/AAfQxRoYaV9M4eV59cuMvt3JrVmL6Z-rks5qpffYgaJpZM4J7MRZ .

mehrdadn commented 8 years ago

(I'll put that on my to-do list, but it'll take me quite a while to find the time and figure it out since I'm completely unfamiliar with the source code, so if someone gets the chance in the meantime that would be great.)

mehrdadn commented 8 years ago

OK I found one culprit. It's the "all_frames": true line in manifest.json. The problem is that every single frame load causes jQuery to be loaded, which takes around 60 ms on my machine. For a page like Gmail, this adds > 400 ms to the page load time.

Any chance it could be changed to false? Or, even better, load jQuery only where it is necessary?

pfn commented 8 years ago

Actually, I would like to drop jquery altogether, but it's used relatively pervasively (the toolbar icon dropdown utilizes jquery)

Sent from my tablet

On Sep 12, 2016 10:10 PM, "mehrdadn" notifications@github.com wrote:

OK I found one culprit. It's the "all_frames": true line in manifest.json. The problem is that every single frame load causes jQuery to be loaded, which takes around 60 ms on my machine. For a page like Gmail, this adds > 400 ms to the page load time.

Any chance it could be changed to false? Or, even better, load jQuery only where it is necessary?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/pfn/passifox/issues/550#issuecomment-246575716, or mute the thread https://github.com/notifications/unsubscribe-auth/AAfQxb6DiNNAmzSwHWIc-4RA8x9YOS0aks5qpjBFgaJpZM4J7MRZ .

mehrdadn commented 8 years ago

Right, but in the absence of that, could you just change the flag to false? Or at least provide the option to do so on the settings page?

pfn commented 8 years ago

The problem is, if it's not loaded, frames that contain login forms will not be processed.

On Tue, Sep 13, 2016, 5:17 PM mehrdadn notifications@github.com wrote:

Right, but in the absence of that, could you just change the flag to false? Or at least provide the option to do so on the settings page?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/pfn/passifox/issues/550#issuecomment-246867472, or mute the thread https://github.com/notifications/unsubscribe-auth/AAfQxW89xnfCw-BGSUUoOTkZYcAAnryEks5qpz0ZgaJpZM4J7MRZ .

mehrdadn commented 8 years ago

But that's why I said at least provide it as an advanced option? It's not an everyday thing for all users, so those of us who never come across it won't mind.

pfn commented 8 years ago

Manifest options cannot be changed at runtime

On Tue, Sep 13, 2016, 5:46 PM mehrdadn notifications@github.com wrote:

But that's why I said at least provide it as an advanced option? It's not an everyday thing for all users, so those of us who never come across it won't mind.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/pfn/passifox/issues/550#issuecomment-246872228, or mute the thread https://github.com/notifications/unsubscribe-auth/AAfQxXL_FYJrvNGwswai3isNzjk2aelQks5qp0PigaJpZM4J7MRZ .

mehrdadn commented 8 years ago

Oh, right. Can jQuery loading be made dynamic instead? So that if the option is set, then jQuery is only loaded for the top level?

pfn commented 8 years ago

That would be something someone would make a PR for

On Tue, Sep 13, 2016, 6:39 PM mehrdadn notifications@github.com wrote:

Oh, right. Can jQuery loading be made dynamic instead? So that if the option is set, then jQuery is only loaded for the top level?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/pfn/passifox/issues/550#issuecomment-246880274, or mute the thread https://github.com/notifications/unsubscribe-auth/AAfQxc7PYHLQA8fnNMGtAZSriQFUNq8Yks5qp1BhgaJpZM4J7MRZ .

Hexstream commented 7 years ago

Honestly loading chromeIPass on every single resource ever is just crazy, I'm also wasting 200+ms on every page load, even for completely unrelated resources like when I open an image directly.

This kind of stupid shit is partly why I dumped LastPass, though KeePassXC/ChromeIPass is certainly much better and I'll probably stick with it into the foreseeable future.

Please fix this. This should be considered a high-priority issue. (And don't give me the "fix it yourself" crap. ;P)

mehrdadn commented 7 years ago

Interestingly I actually ended up "fixing" this myself in the code, the only trouble is it's totally breaking things I don't care about, but I can't really merge it back because of that... (Edit: This is no longer the case.)

ForsakenHarmony commented 6 years ago

@pfn would you consider adding more contributors given that you seem to be busy and this extension is kinda stuck?