Bug 183042 – Add release logging for CacheStorage::Engine disk related functions

• Status: RESOLVED
• Resolution: FIXED
• Component: Service Workers

[youenn fablet]

Add release logging for CacheStorage::Engine disk related functions

[youenn fablet]

Created attachment 334456 [details]
Patch

[Chris Dumez]

Comment on attachment 334456 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=334456&action=review

> Source/WebKit/NetworkProcess/cache/CacheStorageEngine.cpp:285
> +                RELEASE_LOG_ERROR(CacheStorage, "CacheStorage::Engine::writeFile failed with error %d", error);

An engine has an associated sessionID. You should check that the sessionID is not one for an ephemeral session before logging.

[youenn fablet]

(In reply to Chris Dumez from comment #2)
> Comment on attachment 334456 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=334456&action=review
> 
> > Source/WebKit/NetworkProcess/cache/CacheStorageEngine.cpp:285
> > +                RELEASE_LOG_ERROR(CacheStorage, "CacheStorage::Engine::writeFile failed with error %d", error);
> 
> An engine has an associated sessionID. You should check that the sessionID
> is not one for an ephemeral session before logging.

This is unnecessary as CacheStorage::Engine should not write any file for ephemeral sessions.
We could add something like ASSERT(!sessionID.isEphemeral()) at the beginning of writeDisk/readDisk though.

[Chris Dumez]

(In reply to youenn fablet from comment #3)
> (In reply to Chris Dumez from comment #2)
> > Comment on attachment 334456 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=334456&action=review
> > 
> > > Source/WebKit/NetworkProcess/cache/CacheStorageEngine.cpp:285
> > > +                RELEASE_LOG_ERROR(CacheStorage, "CacheStorage::Engine::writeFile failed with error %d", error);
> > 
> > An engine has an associated sessionID. You should check that the sessionID
> > is not one for an ephemeral session before logging.
> 
> This is unnecessary as CacheStorage::Engine should not write any file for
> ephemeral sessions.
> We could add something like ASSERT(!sessionID.isEphemeral()) at the
> beginning of writeDisk/readDisk though.

Ok, please do that.

[Chris Dumez]

Comment on attachment 334456 [details]
Patch

r=me with the added assertions.

[youenn fablet]

(In reply to youenn fablet from comment #3)
> (In reply to Chris Dumez from comment #2)
> > Comment on attachment 334456 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=334456&action=review
> > 
> > > Source/WebKit/NetworkProcess/cache/CacheStorageEngine.cpp:285
> > > +                RELEASE_LOG_ERROR(CacheStorage, "CacheStorage::Engine::writeFile failed with error %d", error);
> > 
> > An engine has an associated sessionID. You should check that the sessionID
> > is not one for an ephemeral session before logging.
> 
> This is unnecessary as CacheStorage::Engine should not write any file for
> ephemeral sessions.
> We could add something like ASSERT(!sessionID.isEphemeral()) at the
> beginning of writeDisk/readDisk though.

Actually, Engine does not store any sessionID.
In case of ephemeral session, m_rootPath is null and we are checking whether it is null at the beginning of read/write methods.

[youenn fablet]

Chris, I'll land the patch as is given the persistency check is close to the release logging and given the fact engine is not storing its sessionID.
Let me know if that does not work for you.

[WebKit Commit Bot]

Comment on attachment 334456 [details]
Patch

Rejecting attachment 334456 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 334456, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
rdparty/autoinstalled/mechanize/_urllib2_fork.py", line 332, in _call_chain
    result = func(*args)
  File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/thirdparty/autoinstalled/mechanize/_urllib2_fork.py", line 1170, in https_open
    return self.do_open(conn_factory, req)
  File "/Volumes/Data/EWS/WebKit/Tools/Scripts/webkitpy/thirdparty/autoinstalled/mechanize/_urllib2_fork.py", line 1118, in do_open
    raise URLError(err)
urllib2.URLError: <urlopen error [Errno 60] Operation timed out>

Full output: http://webkit-queues.webkit.org/results/6624429

[WebKit Commit Bot]

Comment on attachment 334456 [details]
Patch

Clearing flags on attachment: 334456

Committed r228925: <https://trac.webkit.org/changeset/228925>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/37793473>