search

brackets-archive / bracketsIssues

Archive of issues in brackets.
0 stars 0 forks source link

[CLOSED] Client-side disabling of the cache #2590

Open core-ai-bot opened 3 years ago

core-ai-bot commented 3 years ago

Issue by DennisKehrig Thursday Jan 31, 2013 at 03:32 GMT Originally opened as https://github.com/adobe/brackets/pull/2732

Contribution to #1744

DennisKehrig included the following code: https://github.com/adobe/brackets/pull/2732/commits

core-ai-bot commented 3 years ago

Comment by peterflynn Thursday Jan 31, 2013 at 19:01 GMT

Three quick thoughts:

core-ai-bot commented 3 years ago

Comment by gruehle Thursday Jan 31, 2013 at 19:35 GMT

Peter hit on the main points I was going to bring up.

The shell API can be appshell.app.getRemoteDebuggingPort(), since we only need the port number, not the full URL.

Rather than disabling the cache at app startup, did you try clearing the cache before reload (Network.clearBrowserCache)? This may be less invasive, and possibly more reliable.

BTW, your code works perfectly except one case: launch Brackets, open DevTools (with caching disabled), close DevTools and re-launch Brackets. In that case the re-launch doesn't clear/ignore the cache. Doing a subsequent reload fixes things.

core-ai-bot commented 3 years ago

Comment by DennisKehrig Thursday Jan 31, 2013 at 21:32 GMT

"Should we have a -shell API like getOwnRemoteDebuggingUrl()?" That would be good!

_"OTOH, the tail end of app startup will still be delayed while we talk to the socket... so maybe that should happen after APPREADY instead of before?" That's how I had it first, but it seemed a little unclean - some extensions might want to do similar tricks and then the two code paths would get in the way of one another. Though of course we could spice this up a bit and allow extensions to register for an event when the connection to Brackets via remote debugging is established, so they can inject their own code. And maybe trigger another event when all that is done. The last one would certainly be enough to permit doing this after triggering APP_READY. We can also connect to the socket much earlier, and only send the command to disable at the last moment - to make things more parallel.

"Do we understand why this works? The manual caching workaround only remains in effect while the dev tools window is open -- as soon as you close it, caching turns back on." This really seems to be part of the remote debugging tools merely cleaning up after themselves, i.e. turning the cache back on explicitly. We don't, so the setting persists. I tried to verify this by debugging the debugger, but the debugger debugger closes when the debugger closes (must... not... mention... Xzibit).

"In fact, we should make sure it's not too persistent -- if the change is remembered across CEF launches, then the 2nd bullet above is wrong and all "normal"/non-dev launches past the first one would also have no caching..." It's only for the current session and forgotten after quitting Brackets. The Dev Tools somehow remember the settings for a specific target (probably via the debugging URL) and re-apply those settings when re-connecting (that's why you don't need to check "Disable cache" again when closing dev tools and opening them again, and that's why I thought my shell-side fix had worked).

Rather than disabling the cache at app startup, did you try clearing the cache before reload (Network.clearBrowserCache)? This may be less invasive, and possibly more reliable. I haven't. To work in as many cases as possible, this would have to happen right before reloading (so nothing that is loaded after the startup remains cached, like extensions installed via the extension manager) and I'm not sure we can intercept that event. It's handled by the shell in most cases, or not even that when reloading via dev tools... do you know whether we can intercept the reload event in the shell somehow?

"BTW, your code works perfectly except one case: launch Brackets, open DevTools (with caching disabled), close DevTools and re-launch Brackets. In that case the re-launch doesn't clear/ignore the cache. Doing a subsequent reload fixes things." Good catch! That is disappointing... but it makes sense. We could periodically request the JSON file that lists the pages that can be debugged, and if the remote debugger URL is missing at some point, we know another debugger has connected. Once the URL appears again, we can sneak in and disable the cache again. We could miss a very short dev tools session or we might not notice that it ended before the reload is triggered, but in most cases that should do the trick. Even better would be an event from the shell indicating that some debugger connected/disconnected, but I don't know whether that exists. Though Chrome does offer some debugger attached/detached events for extensions and via remote debugging (which we'll use to explain why live preview was interrupted), but I don't know how accessible they are via CEF.

core-ai-bot commented 3 years ago

Comment by gruehle Friday Feb 01, 2013 at 23:30 GMT

do you know whether we can intercept the reload event in the shell somehow?

Yes - the handleFileReload() function in DocumentCommandHandlers.js is called when reloading from Brackets. I don't know how to detect a reload from the DevTools, but if you have DevTools open, you will need to have caching disabled anyway since we won't be able to establish a remote debugging connection.

If we do stick with the current implementation of disabling cache at startup, I'm in favor of doing it after the APP_READY event is fired. Extensions that require a remote debugging connection to Brackets is an edge case. We can, as you say, always add an event later if needed.

core-ai-bot commented 3 years ago

Comment by DennisKehrig Monday Feb 04, 2013 at 11:19 GMT

Thanks, Glenn! Sounds good to me.

core-ai-bot commented 3 years ago

Comment by DennisKehrig Monday Feb 04, 2013 at 15:37 GMT

Investigated a bit further by hacking the dev tools. Peter was right in being surprised that this works - I didn't see any call to Network.setCacheDisabled(false) that the dev tools perform when the connection is closed. In fact, even when simply killing the Chrome frame with the dev tools open (and thus giving it no time to perform a cleanup), caching is enabled again afterwards. So it remains mysterious why the cache remains disabled after we disconnected.

Another clue: simply opening and closing the dev tools is not a problem. Rather, a reload has to occur before closing dev tools again. Then our code to disable the cache does not work because we can't connect. Only THEN will closing the dev tools actually turn caching back on.

core-ai-bot commented 3 years ago

Comment by DennisKehrig Monday Feb 04, 2013 at 16:24 GMT

Okay, got it. If the cache is disabled, the next reload is guaranteed to not be cached (regardless of whether it occurs while the client is still connected or not). Also, all reloads during that connection will not be cached. If there has been a reload after disabling the cache while still connected, the first reload after having disconnected will not be cached.

So we need to disable the cache after every reload. Normally the dev tools don't do that. They disable the cache once when you open them (and every time you check the checkbox).

We can hack the dev tools to change this.

core-ai-bot commented 3 years ago

Comment by gruehle Thursday Feb 07, 2013 at 00:22 GMT

I tried calling _disableCache() from the handleFileReload() function in DocumentCommandHandlers.js:

function handleFileReload(commandData) {
    return _handleWindowGoingAway(commandData, function () {
        _disableCache().always(function () {
            window.location.reload(true);
        });
    });
}

This way the cache is disabled immediately before the window is reloaded. This always worked for me, even if I opened up DevTools, disabled caching, and closed DevTools. Reloading from DevTools works as long as caching is disabled.

Can you think of any downside to disabling at reload time?

core-ai-bot commented 3 years ago

Comment by DennisKehrig Monday Feb 11, 2013 at 16:50 GMT

Sounds good, this way at least reloading with dev tools on only works when the cache is disabled there, making it easier to make the connection when it doesn't work.

core-ai-bot commented 3 years ago

Comment by DennisKehrig Monday Feb 11, 2013 at 16:52 GMT

Of course this wouldn't work if some extension triggered window.location.reload(), but since other things are associated with reloading, people should use Commands.DEBUG_REFRESH_WINDOW anyway.

core-ai-bot commented 3 years ago

Comment by DennisKehrig Monday Feb 11, 2013 at 17:08 GMT

Thank,@gruehle! I modified the patch. For some bizarre reason reloading just once without the dev tools was enough to make all subsequent reloads work, even when the unmodified dev tools were connected without the checkbox to disable the cache.

core-ai-bot commented 3 years ago

Comment by gruehle Friday Feb 15, 2013 at 21:47 GMT

Hey@DennisKehrig - this is a much simpler fix, thank you! There is just one small change that needs to be made: the WebSocket global directive for JSLint needs to be moved from brackets.js to DocumentCommandHandlers.js.

Would you mind submitting a new pull request that only has the changes to DocumentCommandHandlers.js? Thanks!

core-ai-bot commented 3 years ago

Comment by DennisKehrig Saturday Feb 16, 2013 at 17:10 GMT

Sure, Glenn! Have fun with #2894 :)