chromiumembedded / cef

Chromium Embedded Framework (CEF). A simple framework for embedding Chromium-based browsers in other applications.
https://bitbucket.org/chromiumembedded/cef/
Other
3.32k stars 464 forks source link

Cancel pending loads before calling HandleWindowClose #113

Closed magreenblatt closed 8 years ago

magreenblatt commented 14 years ago

Original report by Anonymous.


Original issue 113 created by emerick on 2010-08-24T14:58:34.000Z:

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

magreenblatt commented 13 years ago

Original comment by Anonymous.


Comment 1. originally posted by crazy.fun.random on 2011-03-09T23:37:11.000Z:

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?

magreenblatt commented 11 years ago

Original comment by Anonymous.


Comment 2. originally posted by cristian.amarie on 2013-05-16T22:09:05.000Z:

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 (bb), 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).

magreenblatt commented 11 years ago

Original comment by Anonymous.


Comment 3. originally posted by chaos.defilerz on 2013-05-17T06:09:51.000Z:

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.

magreenblatt commented 11 years ago

Original comment by Anonymous.


Comment 4. originally posted by chaos.defilerz on 2013-05-21T10:07:14.000Z:

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 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.

magreenblatt commented 11 years ago

Original comment by Anonymous.


Comment 5. originally posted by chaos.defilerz on 2013-05-21T10:09:20.000Z:

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.

magreenblatt commented 11 years ago

Comment 6. originally posted by magreenblatt on 2013-09-05T16:49:23.000Z:

issue #767 has been merged into this issue.

magreenblatt commented 8 years ago

If this problem reproduces with current CEF branches (2526 or newer) please post and we can re-open.

magreenblatt commented 11 years ago

Original changes by Anonymous.


magreenblatt commented 8 years ago