Add Page. TransferToPageSet

Add Page. TransferToPageSet API that allows transferring pages from one page_set to another.

BUG=394085
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=287476

[nduca, 2014-07-16 01:06:54]

sorry

https://codereview.chromium.org/397793005/diff/1/tools/telemetry/telemetry/pa...
File tools/telemetry/telemetry/page/page_unittest.py (right):

https://codereview.chromium.org/397793005/diff/1/tools/telemetry/telemetry/pa...
tools/telemetry/telemetry/page/page_unittest.py:123: 
how about a unit test in page_set_unittest that forms a pageset from a sub
pageset plus an additional page?

https://codereview.chromium.org/397793005/diff/1/tools/telemetry/telemetry/pa...
tools/telemetry/telemetry/page/page_unittest.py:123: 
how about a unit test in page_set_unittest that forms a pageset from a sub
pageset plus an additional page?

https://codereview.chromium.org/397793005/diff/20001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/page/page_set.py (right):

https://codereview.chromium.org/397793005/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/page/page_set.py:74: def TransferPageTo(self,
other_page_set, index=-1):
i think it should be page.TransferToPageSet(other_page_set).

Hide indexing.

def page.TransferToPageSet(page_set):
  assert self._page_set
  self._page_set.remove(self)
  self._page_set = None
  page_set.AddPage(self)

[nednguyen, 2014-07-16 03:29:18]

https://codereview.chromium.org/397793005/diff/20001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/page/page_set.py (right):

https://codereview.chromium.org/397793005/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/page/page_set.py:74: def TransferPageTo(self,
other_page_set, index=-1):
On 2014/07/16 01:06:54, nduca wrote:
> i think it should be page.TransferToPageSet(other_page_set).
> 
> Hide indexing.
> 
> def page.TransferToPageSet(page_set):
>   assert self._page_set
>   self._page_set.remove(self)
>   self._page_set = None
>   page_set.AddPage(self)

Done. My only concern with this is that one may need to be careful to not do:

for p in page_set_A:
   p.TransferToPageSet(page_set_B)

But python's remove() API also has the same problem, so I am fine with this.

[nednguyen, 2014-07-23 16:36:43]

On 2014/07/16 03:29:18, nednguyen wrote:
>
https://codereview.chromium.org/397793005/diff/20001/tools/telemetry/telemetr...
> File tools/telemetry/telemetry/page/page_set.py (right):
> 
>
https://codereview.chromium.org/397793005/diff/20001/tools/telemetry/telemetr...
> tools/telemetry/telemetry/page/page_set.py:74: def TransferPageTo(self,
> other_page_set, index=-1):
> On 2014/07/16 01:06:54, nduca wrote:
> > i think it should be page.TransferToPageSet(other_page_set).
> > 
> > Hide indexing.
> > 
> > def page.TransferToPageSet(page_set):
> >   assert self._page_set
> >   self._page_set.remove(self)
> >   self._page_set = None
> >   page_set.AddPage(self)
> 
> Done. My only concern with this is that one may need to be careful to not do:
> 
> for p in page_set_A:
>    p.TransferToPageSet(page_set_B)
> 
> But python's remove() API also has the same problem, so I am fine with this.

To get rid of this ugly situation, let's just keep the design that Page is
immutable and support the method p.ClonePageAndAddToPageSet(ps) instead. wdyt?

[nduca, 2014-08-04 22:13:32]

lgtm

[nednguyen, 2014-08-04 22:15:36]

The CQ bit was checked by nednguyen@google.com

[commit-bot: I haz the power, 2014-08-04 22:17:14]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/nednguyen@google.com/397793005/60001

[commit-bot: I haz the power, 2014-08-05 07:34:57]

Change committed as 287476