DevTools: Add ability to add and edit cookies

third_party/WebKit/Source/devtools/front_end/components_lazy/CookiesTable.js

 /*
  * Copyright (C) 2009 Apple Inc.  All rights reserved.
  * Copyright (C) 2009 Joseph Pecoraro
  * Copyright (C) 2010 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1.  Redistributions of source code must retain the above copyright
@@ skipping 18 matching lines @@
  */
 
 /**
  * @unrestricted
  */
 Components.CookiesTable = class extends UI.VBox {
   /**
    * @param {boolean} expandable
    * @param {function()=} refreshCallback
    * @param {function()=} selectedCallback
+   * @param {string=} cookieDomain
    */
-  constructor(expandable, refreshCallback, selectedCallback) {
+  constructor(expandable, refreshCallback, selectedCallback, cookieDomain) {

[allada, 2016/12/13 01:34:31]

Lets rename "expandable" to "readOnly" since that's what it does now.

     super();
 
-    var readOnly = expandable;
+    this._readOnly = expandable;
     this._refreshCallback = refreshCallback;
+    this._cookieDomain = cookieDomain;
 
     var columns = /** @type {!Array<!UI.DataGrid.ColumnDescriptor>} */ ([
       {
         id: 'name',
         title: Common.UIString('Name'),
         sortable: true,
         disclosure: expandable,
         sort: UI.DataGrid.Order.Ascending,
         longText: true,
-        weight: 24
+        weight: 24,
+        editable: true

[allada, 2016/12/13 01:34:30]

This should be based on this._readOnly

       },
-      {id: 'value', title: Common.UIString('Value'), sortable: true, longText: true, weight: 34},
-      {id: 'domain', title: Common.UIString('Domain'), sortable: true, weight: 7},
-      {id: 'path', title: Common.UIString('Path'), sortable: true, weight: 7},
-      {id: 'expires', title: Common.UIString('Expires / Max-Age'), sortable: true, weight: 7},
+      {id: 'value', title: Common.UIString('Value'), sortable: true, longText: true, weight: 34, editable: true},
+      {id: 'domain', title: Common.UIString('Domain'), sortable: true, weight: 7, editable: true},
+      {id: 'path', title: Common.UIString('Path'), sortable: true, weight: 7, editable: true},
+      {id: 'expires', title: Common.UIString('Expires / Max-Age'), sortable: true, weight: 7, editable: true},
       {id: 'size', title: Common.UIString('Size'), sortable: true, align: UI.DataGrid.Align.Right, weight: 7},
       {id: 'httpOnly', title: Common.UIString('HTTP'), sortable: true, align: UI.DataGrid.Align.Center, weight: 7},
       {id: 'secure', title: Common.UIString('Secure'), sortable: true, align: UI.DataGrid.Align.Center, weight: 7}, {
         id: 'sameSite',
         title: Common.UIString('SameSite'),
         sortable: true,
         align: UI.DataGrid.Align.Center,
         weight: 7
       }
     ]);
 
-    if (readOnly) {
+    if (this._readOnly) {
       this._dataGrid = new UI.DataGrid(columns);
     } else {
-      this._dataGrid = new UI.DataGrid(columns, undefined, this._onDeleteCookie.bind(this), refreshCallback);
+      this._dataGrid = new UI.DataGrid(columns, this._onUpdateCookie.bind(this), this._onDeleteCookie.bind(this), refreshCallback);
       this._dataGrid.setRowContextMenuCallback(this._onRowContextMenu.bind(this));
     }
 
     this._dataGrid.setName('cookiesTable');
     this._dataGrid.addEventListener(UI.DataGrid.Events.SortingChanged, this._rebuildTable, this);
 
     if (selectedCallback)
       this._dataGrid.addEventListener(UI.DataGrid.Events.SelectedNode, selectedCallback, this);
 
     this._nextSelectedCookie = /** @type {?SDK.Cookie} */ (null);
+    this._lastEditedColumnId = /** @type {?string} */ (null);

[allada, 2016/12/13 01:34:31]

nit: Rather than type casting it like his lets declare what type it is using
something like:

/** @type {?string} */
this._lastEditedColumnId = null;

[kdzwinel, 2016/12/14 01:32:36]

Makes sense! I wonder why `_nextSelectedCookie` above does type casting?

[allada, 2016/12/14 03:04:15]

It tells closure that this variable is a type... The other way implies it from
inference.

 
     this._dataGrid.asWidget().show(this.element);
     this._data = [];
   }
 
   /**
    * @param {?string} domain
    */
   _clearAndRefresh(domain) {
     this.clear(domain);
     this._refresh();
   }
 
   /**
    * @param {!UI.ContextMenu} contextMenu
    * @param {!UI.DataGridNode} node
    */
   _onRowContextMenu(contextMenu, node) {
-    if (node === this._dataGrid.creationNode)
+    if (node.isCreationNode)
       return;
-    var domain = node.cookie.domain();
+
+    const cookie = node.cookie;
+    const checkmark = '\u2713';
+    contextMenu.appendCheckboxItem(
+      Common.UIString('Secure flag'),
+      this._setCookieFlag.bind(this, node, 'secure', cookie.secure() ? '' : checkmark),
+      cookie.secure(),
+      false
+    );
+    contextMenu.appendCheckboxItem(
+      Common.UIString('HttpOnly flag'),
+      this._setCookieFlag.bind(this, node, 'httpOnly', cookie.httpOnly() ? '' : checkmark),
+      cookie.httpOnly(),
+      false
+    );
+
+    var sameSiteSubmenu = contextMenu.appendSubMenuItem(Common.UIString('SameSite flag'));
+    sameSiteSubmenu.appendCheckboxItem(

[allada, 2016/12/13 01:34:31]

Lets remove this as a checkbox item and just make it a submenuitem.

[allada, 2016/12/14 03:04:15]

Lets move it back to checkbox... I misread the hierarchy. (keep the rename
though)

+      Common.UIString('<empty>'),
+      this._setCookieFlag.bind(this, node, 'sameSite', ''),
+      !cookie.sameSite(),
+      false
+    );
+    sameSiteSubmenu.appendCheckboxItem(
+      Protocol.Network.CookieSameSite.Lax,
+      this._setCookieFlag.bind(this, node, 'sameSite', Protocol.Network.CookieSameSite.Lax),
+      cookie.sameSite() === Protocol.Network.CookieSameSite.Lax,
+      false
+    );
+    sameSiteSubmenu.appendCheckboxItem(

[allada, 2016/12/13 01:34:31]

We should add one more called something like "No Restriction". This is default
and is set by not setting anything in SameSite param in browser_protocol.

[kdzwinel, 2016/12/14 01:32:36]

Isn't that the same I called '<empty>' above? (I'll rename it)

[allada, 2016/12/14 03:04:15]

Yes, I see.

+      Protocol.Network.CookieSameSite.Strict,
+      this._setCookieFlag.bind(this, node, 'sameSite', Protocol.Network.CookieSameSite.Strict),
+      cookie.sameSite() === Protocol.Network.CookieSameSite.Strict,
+      false
+    );
+    contextMenu.appendSeparator();
+
+    var domain = cookie.domain();
     if (domain) {
       contextMenu.appendItem(
           Common.UIString.capitalize('Clear ^all from "%s"', domain), this._clearAndRefresh.bind(this, domain));
     }
     contextMenu.appendItem(Common.UIString.capitalize('Clear ^all'), this._clearAndRefresh.bind(this, null));
   }
 
+  _setCookieFlag(node, flag, value) {
+    node.data[flag] = value;
+    node.refresh();
+    this._saveNode(node);
+  }
+
   /**
    * @param {!Array.<!SDK.Cookie>} cookies
    */
   setCookies(cookies) {
     this.setCookieFolders([{cookies: cookies}]);
   }
 
   /**
    * @param {!Array.<!{folderName: ?string, cookies: !Array.<!SDK.Cookie>}>} cookieFolders
    */
@@ skipping 16 matching lines @@
   clear(domain) {
     for (var i = 0, length = this._data.length; i < length; ++i) {
       var cookies = this._data[i].cookies;
       for (var j = 0, cookieCount = cookies.length; j < cookieCount; ++j) {
         if (!domain || cookies[j].domain() === domain)
           cookies[j].remove();
       }
     }
   }
 
+  /**
+   * @override
+   */
+  willHide() {
+    this._lastEditedColumnId = null;
+  }
+
   _rebuildTable() {
     var selectedCookie = this._nextSelectedCookie || this.selectedCookie();
+    var lastEditedColumnId = this._lastEditedColumnId;
     this._nextSelectedCookie = null;
+    this._lastEditedColumnId = null;
     this._dataGrid.rootNode().removeChildren();
     for (var i = 0; i < this._data.length; ++i) {
       var item = this._data[i];
       if (item.folderName) {
         var groupData = {
           name: item.folderName,
           value: '',
           domain: '',
           path: '',
           expires: '',
           size: this._totalSize(item.cookies),
           httpOnly: '',
           secure: '',
           sameSite: ''
         };
         var groupNode = new UI.DataGridNode(groupData);
         groupNode.selectable = true;
         this._dataGrid.rootNode().appendChild(groupNode);
         groupNode.element().classList.add('row-group');
-        this._populateNode(groupNode, item.cookies, selectedCookie);
+        this._populateNode(groupNode, item.cookies, selectedCookie, lastEditedColumnId);
         groupNode.expand();
       } else {
-        this._populateNode(this._dataGrid.rootNode(), item.cookies, selectedCookie);
+        this._populateNode(this._dataGrid.rootNode(), item.cookies, selectedCookie, lastEditedColumnId);
       }
     }
+    if (!this._readOnly)
+      this._dataGrid.addCreationNode(false);
   }
 
   /**
    * @param {!UI.DataGridNode} parentNode
    * @param {?Array.<!SDK.Cookie>} cookies
    * @param {?SDK.Cookie} selectedCookie
+   * @param {?string} lastEditedColumnId
    */
-  _populateNode(parentNode, cookies, selectedCookie) {
+  _populateNode(parentNode, cookies, selectedCookie, lastEditedColumnId) {
     parentNode.removeChildren();
     if (!cookies)
       return;
 
     this._sortCookies(cookies);
     for (var i = 0; i < cookies.length; ++i) {
       var cookie = cookies[i];
       var cookieNode = this._createGridNode(cookie);
       parentNode.appendChild(cookieNode);
       if (selectedCookie && selectedCookie.name() === cookie.name() && selectedCookie.domain() === cookie.domain() &&
-          selectedCookie.path() === cookie.path())
+          selectedCookie.path() === cookie.path()) {
         cookieNode.select();
+        if (lastEditedColumnId !== null)
+          this._dataGrid.startEditingNextEditableColumnOfDataGridNode(cookieNode, lastEditedColumnId);
+      }
     }
   }
 
   _totalSize(cookies) {
     var totalSize = 0;
     for (var i = 0; cookies && i < cookies.length; ++i)
       totalSize += cookies[i].size();
     return totalSize;
   }
 
@@ skipping 94 matching lines @@
 
   _onDeleteCookie(node) {
     var cookie = node.cookie;
     var neighbour = node.traverseNextNode() || node.traversePreviousNode();
     if (neighbour)
       this._nextSelectedCookie = neighbour.cookie;
     cookie.remove();
     this._refresh();
   }
 
+  _onUpdateCookie(editingNode, columnIdentifier, oldText, newText) {

[allada, 2016/12/13 01:34:31]

nit: doctype please.

[kdzwinel, 2016/12/14 01:32:36]

Sure! I can't figure out the pattern here though. Not all methods have doctype,
I thought private ones don't, but now I see that some of them do. In other
words, why we don't need a doctype on `_totalSize` or `_onDeleteCookie`, but
want it on `_onUpdateCookie`?

[allada, 2016/12/14 03:04:15]

It's because old code was not doctyped. We are trying to get everything
doctyped, so all future code should be.

+    this._lastEditedColumnId = columnIdentifier;
+    this._setDefaults(editingNode);
+    if (this._isValidCookieData(editingNode.data))
+      this._saveNode(editingNode);
+    else
+      editingNode.unsaved = true;
+  }
+
+  _setDefaults(node) {

[allada, 2016/12/13 01:34:30]

nit: doctype please.

+    if (node.data.name === null)
+      node.data.name = '';
+    if (node.data.value === null)
+      node.data.value = '';
+    if (node.data.domain === null)
+      node.data.domain = this._cookieDomain;
+    if (node.data.path === null)
+      node.data.path = '/';
+    if (node.data.expires === null)
+      node.data.expires = Common.UIString('Session');
+  }
+
+  _saveNode(node) {

[allada, 2016/12/13 01:34:31]

nit: doctype please.

+    var oldCookie = node.cookie;
+    var newCookie = this._createCookieFromData(node.data);
+    if (oldCookie && (newCookie.name() !== oldCookie.name() || newCookie.url() !== oldCookie.url())) oldCookie.remove();

[allada, 2016/12/13 01:34:30]

nit: oldCookie.remove() should be on a new line.

+    node.cookie = newCookie;
+    newCookie.save((error, success) => {
+      if (success)
+        this._refresh();
+      else
+        node.unsaved = true;
+    });
+    this._nextSelectedCookie = newCookie;
+  }
+
+  _createCookieFromData(data) {

[allada, 2016/12/13 01:34:31]

nit: doctype please.

+    var target = SDK.targetManager.targets(SDK.Target.Capability.Network)[0];

[allada, 2016/12/13 01:34:30]

We don't want to use targetManager here we want to use
MultitargetNetworkManager. I'd suggest adding an entry here (near the line
number of the link). In multitarget file we should only dispatch to one agent
instead of all agents.

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/devtools/front...

[kdzwinel, 2016/12/14 01:32:36]

So instead of getting a first target from targetManager, I should get a first
network agent from MultitargetNetworkManager?
SDK.Cookie takes targets in the constructor, but this can be replaced with
Protocol.NetworkAgent.

[allada, 2016/12/14 03:04:15]

I mean, we should add a new function/method in multitargetNetworkAgent that we
can call and let it figure out which target to send it to, similar to how
clearBrowserCookies() works in that file.

[kdzwinel, 2016/12/14 14:26:50]

Sorry, I still do not follow :/ `clearBrowserCookies` doesn't really figure out
anything, it just calls `clearBrowserCache` on all available agents. When saving
a cookie I shouldn't call it on all targets AFAIK. So, which target is "the
right one"? target.name() === "Main"?

+    var cookie = new SDK.Cookie(target, data.name, data.value, null);
+    cookie.addAttribute('domain', data.domain);
+    cookie.addAttribute('path', data.path);
+    if (data.expires && data.expires !== Common.UIString('Session')) {
+      var secondsSinceEpoch = Date.parse(data.expires) / 1000;

[allada, 2016/12/13 01:34:31]

nit: we should floor this.

+      cookie.addAttribute('expires', secondsSinceEpoch || undefined);
+    }
+    if (data.httpOnly)
+      cookie.addAttribute('httpOnly');
+    if (data.secure)
+      cookie.addAttribute('secure');
+    if (data.sameSite)
+      cookie.addAttribute('sameSite', data.sameSite);
+    cookie.setSize(data.name.length + data.value.length);
+    return cookie;
+  }
+
+  _isValidCookieData(data) {

[allada, 2016/12/13 01:34:31]

nit: doctype please.

+    return (data.name || data.value) && this._isValidDomain(data.domain) && this._isValidPath(data.path) && this._isValidDate(data.expires);
+  }
+
+  _isValidDomain(domain) {

[allada, 2016/12/13 01:34:31]

nit: doctype please.

+    if (domain === '')

[allada, 2016/12/13 01:34:30]

nit: We prefer if (!domain)

+      return true;
+    var url = null;

[allada, 2016/12/13 01:34:31]

nit: Lets move the definition down into the try catch. This will ensure closure
compiler will know what type it is.

+    try {
+      url = new URL('http://' + domain);

[allada, 2016/12/13 01:34:30]

Lets use ParsedURL instead. We should be able to get rid of the try-catch.

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/devtools/front...

+    } catch (e) {
+      return false;
+    }
+    return url.hostname === domain;
+  }
+
+  _isValidPath(path) {

[allada, 2016/12/13 01:34:31]

nit: doctype please.

+    var url = null;

[allada, 2016/12/13 01:34:30]

nit: diddo from above.

+    try {
+      url = new URL('http://example.com' + path);

[allada, 2016/12/13 01:34:31]

see above.

+    } catch (e) {
+      return false;
+    }
+    return url.pathname === path;
+  }
+
+  _isValidDate(date) {

[allada, 2016/12/13 01:34:30]

nit: doctype please.

+    return date === '' || date === Common.UIString('Session') || !isNaN(Date.parse(date));
+  }
+
   _refresh() {
     if (this._refreshCallback)
       this._refreshCallback();
   }
 };