Re: Move MIME related platform files under network/mime (issue 2482653002 by kinuko@chromium.org)

0 views
Skip to first unread message

kin...@chromium.org

unread,
Dec 4, 2016, 10:39:01 AM12/4/16
to mk...@chromium.org, chromium...@chromium.org, blink-...@chromium.org, esp...@chromium.org
Mike, do you think you can review this? The change itself is pretty mechanical,
new location for the mime-related files is already discussed with Elliott in the
previous patch (https://crrev.com/2444873002) and hopefully no need to argue.

https://codereview.chromium.org/2482653002/

mk...@chromium.org

unread,
Dec 6, 2016, 8:44:35 AM12/6/16
to kin...@chromium.org, blink-...@chromium.org, chromium...@chromium.org, esp...@chromium.org
LGTM, except for the license changes. If you're just moving the file, my
understanding is that we shouldn't modify the license. *shrug* IANAL, so...


https://codereview.chromium.org/2482653002/diff/130001/third_party/WebKit/Source/platform/network/mime/ContentType.cpp
File third_party/WebKit/Source/platform/network/mime/ContentType.cpp
(left):

https://codereview.chromium.org/2482653002/diff/130001/third_party/WebKit/Source/platform/network/mime/ContentType.cpp#oldcode27
third_party/WebKit/Source/platform/network/mime/ContentType.cpp:27: */
Nit: If you're just moving the file, don't change the license.

https://codereview.chromium.org/2482653002/diff/130001/third_party/WebKit/Source/platform/network/mime/MIMETypeRegistry.h
File third_party/WebKit/Source/platform/network/mime/MIMETypeRegistry.h
(left):

https://codereview.chromium.org/2482653002/diff/130001/third_party/WebKit/Source/platform/network/mime/MIMETypeRegistry.h#oldcode24
third_party/WebKit/Source/platform/network/mime/MIMETypeRegistry.h:24:
*/
Nit: If you're just moving the file, don't change the license.

https://codereview.chromium.org/2482653002/

tyos...@chromium.org

unread,
Dec 7, 2016, 2:20:16 AM12/7/16
to kin...@chromium.org, mk...@chromium.org, blink-...@chromium.org, chromium...@chromium.org, esp...@chromium.org, toyos...@chromium.org
Ditto for the files for which move detection didn't happen.

https://codereview.chromium.org/2482653002/

tyos...@chromium.org

unread,
Dec 7, 2016, 2:20:21 AM12/7/16
to kin...@chromium.org, mk...@chromium.org, blink-...@chromium.org, chromium...@chromium.org, esp...@chromium.org, toyos...@chromium.org
lgtm


https://codereview.chromium.org/2482653002/diff/130001/third_party/WebKit/Source/core/inspector/InspectorPageAgent.cpp
File third_party/WebKit/Source/core/inspector/InspectorPageAgent.cpp
(left):

https://codereview.chromium.org/2482653002/diff/130001/third_party/WebKit/Source/core/inspector/InspectorPageAgent.cpp#oldcode62
third_party/WebKit/Source/core/inspector/InspectorPageAgent.cpp:62:
#include "core/loader/resource/CSSStyleSheetResource.h"
toCSSStyleSheetResource() used in this file is defined in this header
using the DEFINE_RESOURCE_TYPE_CASTS macro.

https://codereview.chromium.org/2482653002/diff/130001/third_party/WebKit/Source/core/inspector/InspectorPageAgent.cpp#oldcode64
third_party/WebKit/Source/core/inspector/InspectorPageAgent.cpp:64:
#include "core/loader/resource/ScriptResource.h"
ditto about toScriptResource()

https://codereview.chromium.org/2482653002/

kin...@chromium.org

unread,
Dec 7, 2016, 10:33:37 AM12/7/16
to mk...@chromium.org, tyos...@chromium.org, blink-...@chromium.org, chromium...@chromium.org, esp...@chromium.org, toyos...@chromium.org
Thanks! Restored the copyright info in the moved files.



https://codereview.chromium.org/2482653002/diff/130001/third_party/WebKit/Source/core/inspector/InspectorPageAgent.cpp
File third_party/WebKit/Source/core/inspector/InspectorPageAgent.cpp
(left):

https://codereview.chromium.org/2482653002/diff/130001/third_party/WebKit/Source/core/inspector/InspectorPageAgent.cpp#oldcode62
third_party/WebKit/Source/core/inspector/InspectorPageAgent.cpp:62:
#include "core/loader/resource/CSSStyleSheetResource.h"
On 2016/12/07 07:20:20, tyoshino wrote:
> toCSSStyleSheetResource() used in this file is defined in this header
using the
> DEFINE_RESOURCE_TYPE_CASTS macro.

Oops, there was a merge error... done.


https://codereview.chromium.org/2482653002/diff/130001/third_party/WebKit/Source/core/inspector/InspectorPageAgent.cpp#oldcode64
third_party/WebKit/Source/core/inspector/InspectorPageAgent.cpp:64:
#include "core/loader/resource/ScriptResource.h"
On 2016/12/07 07:20:20, tyoshino wrote:
> ditto about toScriptResource()

Done.
On 2016/12/06 13:44:35, Mike West (slow) wrote:
> Nit: If you're just moving the file, don't change the license.

Done.
On 2016/12/06 13:44:35, Mike West (slow) wrote:
> Nit: If you're just moving the file, don't change the license.

commit-bot@chromium.org via codereview.chromium.org

unread,
Dec 7, 2016, 10:34:07 AM12/7/16
to kin...@chromium.org, mk...@chromium.org, tyos...@chromium.org, commi...@chromium.org, blink-...@chromium.org, chromium...@chromium.org, esp...@chromium.org, toyos...@chromium.org

commit-bot@chromium.org via codereview.chromium.org

unread,
Dec 7, 2016, 11:36:17 AM12/7/16
to kin...@chromium.org, mk...@chromium.org, tyos...@chromium.org, commi...@chromium.org, blink-...@chromium.org, chromium...@chromium.org, esp...@chromium.org, toyos...@chromium.org
Committed patchset #6 (id:170001)

https://codereview.chromium.org/2482653002/

commit-bot@chromium.org via codereview.chromium.org

unread,
Dec 7, 2016, 11:39:15 AM12/7/16
to kin...@chromium.org, mk...@chromium.org, tyos...@chromium.org, commi...@chromium.org, blink-...@chromium.org, chromium...@chromium.org, esp...@chromium.org, toyos...@chromium.org
Patchset 6 (id:??) landed as
https://crrev.com/97c9fecf4a50dd69ccf162aa1c75c7f40cdbbf58
Cr-Commit-Position: refs/heads/master@{#436977}

https://codereview.chromium.org/2482653002/
Reply all
Reply to author
Forward
0 new messages