cloudtrends / chromiumembedded

Automatically exported from code.google.com/p/chromiumembedded
1 stars 1 forks source link

Cancel pending loads before calling HandleWindowClose #113

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Load a long-ish web page (maybe NYT front page or Google News or something 
like that).
2. Close the browser before it finishes loading the page.

What is the expected output? What do you see instead?

I expect to see the load cancelled, HandleLoadEnd called, and then 
HandleBeforeWindowClose called.  Instead, the load is not cancelled and 
HandleLoadEnd is not called.

What version of the product are you using? On what operating system?

CEF r82 on 32-bit Windows 7.

Please provide any additional information below.

Forum post with details is here: 
http://www.magpcss.org/ceforum/viewtopic.php?f=6&t=147&start=0

Original issue reported on code.google.com by emerick on 24 Aug 2010 at 2:58

GoogleCodeExporter commented 9 years ago
I have made this change in my build and thus far have not run into any issues:

void CefBrowserImpl::UIT_DestroyBrowser()
{
  if (UIT_GetWebView()) 
    UIT_GetWebView()->mainFrame()->stopLoading();

Not fully knowing all the code, is there risk to this?

Original comment by crazy.fu...@gmail.com on 9 Mar 2011 at 11:37

GoogleCodeExporter commented 9 years ago
I am trying now to call 
  net::URLFetcher::CancelAll(); 
from CefProcessIOThread::Cleanup();

In /trunk/src/chrome/browser/io_thread.cc the call
  content::URLFetcher::CancelAll();
was removed in revision 117934, so I figure out this is the cause of remnants 
URLRequest objects when closing CEF.

Will get back with details as soon as I have a working version. (This applies 
to CEF1).

Original comment by cristian...@gmail.com on 16 May 2013 at 10:09

GoogleCodeExporter commented 9 years ago
URLFetcher::CancelAll() in Cleanup() is too late; request->Cancel() is async 
and is a race condition between CefIOThread::Cleanup() and URLRequestContext 
(in debug at least still repro even with CancelAll). Not sure yet which 
solution I'll try - moving up CancelAll() seems dangerous for now.

Original comment by chaos.de...@gmail.com on 17 May 2013 at 6:09

GoogleCodeExporter commented 9 years ago
Managed to solve the bug (no patch yet) by changing cef (AND chromium 
url_request :( ... ).

void CefBrowserImpl::UIT_DestroyBrowser() {
...
  if (request_context_proxy_.get()) {
    UIT_CancelPendingRequests(true); // ==> added this

    // Delete the proxy on the IO thread.
    CefThread::DeleteSoon(CefThread::IO, FROM_HERE,
        request_context_proxy_.release());
  }
...
}
...
void CefBrowserImpl::UIT_CancelPendingRequests(bool waitForTermination) {
  REQUIRE_UIT();

  std::unique_ptr<base::WaitableEvent> request_destroyed_event;
  if(waitForTermination) {
      request_destroyed_event.reset(new base::WaitableEvent(false, false));
  }

  if (request_context_proxy_.get()) {
    CefThread::PostTask(CefThread::IO, FROM_HERE,
        base::Bind(&BrowserRequestContextProxy::CancelRequests, 
        base::Unretained(request_context_proxy_.get()), 
        request_destroyed_event.get()));

    if(request_destroyed_event.get()) {
        request_destroyed_event->Wait();
    }
  }
}

In chromium\src\net\url_request\url_request_context.h:
class NET_EXPORT URLRequestContext
    : NON_EXPORTED_BASE(public base::NonThreadSafe) {
 public:
  URLRequestContext();
  virtual ~URLRequestContext();

  // cancel all requests, signaling the event if passed
  void CancelRequests(base::WaitableEvent* destroyEvent); // ==> add this
...

In chromium\src\net\url_request\url_request_context.cc:
void URLRequestContext::CancelRequests(base::WaitableEvent* destroyEvent) {
  int num_requests = url_requests_ ? url_requests_->size() : 0;
  if(num_requests != 0) {
      // we need to destroy the requests AND wait for all to go into destructor
      // if an URLRequest remains alive AND is destroyed, its __dtor will access
      // URLRequestContext which is destroyed.

      // we need to make a copy since url_requests_->erase(request) will be called from 
      // the URLRequest __dtor.
      std::set<const URLRequest*> requests = *url_requests_;
      for(std::set<const URLRequest*>::iterator itreq = requests.begin();
          itreq != requests.end();
          ++itreq) {
          URLRequest* request = const_cast<URLRequest*>(*itreq);
          if(request != 0) {
              request->Cancel();
          }
      }
  }

  if(destroyEvent) {
    destroyEvent->Signal();
  }
}

For convenience, I have added to CefBrowser also a CancelPendingRequests method 
(see files in cef directory - note there are also other changes unrelated to 
this bug). See the attached (manual job !) .zip file containing the modified 
files (don't take them for granted).
The reference revision URL is  
http://chromiumembedded.googlecode.com/svn/branches/1364/cef1.

Original comment by chaos.de...@gmail.com on 21 May 2013 at 10:07

Attachments:

GoogleCodeExporter commented 9 years ago
Follow up: I am mainly using this for shutdown issues (long unfinished download 
OR long loading time and close application). Not sure if is 100% correct, 
though - it does the job for my particular problem and I didn't tested it 
against more elaborate scenarios.

Original comment by chaos.de...@gmail.com on 21 May 2013 at 10:09

GoogleCodeExporter commented 9 years ago
Issue 767 has been merged into this issue.

Original comment by magreenb...@gmail.com on 5 Sep 2013 at 4:49

GoogleCodeExporter commented 9 years ago
CEF is transitioning from Google Code to Bitbucket project hosting. If you 
would like to continue receiving notifications on this issue please add 
yourself as a Watcher at the new location: 
https://bitbucket.org/chromiumembedded/cef/issue/113

Original comment by magreenb...@gmail.com on 14 Mar 2015 at 3:20